This is currently postponed on #1820414: CHANGE NOTICE: Move views_ui.module directly into /core/modules/. When Views UI moves we can re unite the tests.

CommentFileSizeAuthor
#16 1862352-16.patch28.78 KBdamiankloip
#13 1862352-13.patch27.98 KBdamiankloip
#9 1862352-9.patch27.29 KBdamiankloip
#7 1862352-7.patch27.25 KBdamiankloip
#7 interdiff-1862352-7.txt519 bytesdamiankloip
#6 1862352-6.patch27.25 KBdamiankloip
#2 1862352.patch6.18 KBdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Component: configuration system » views_ui.module
damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Postponed » Needs review
FileSize
1862352.patch6.18 KB

We can do this now. I think to start with, just move the UI tests directory from views tests into the views_ui/lib/Drupal/views_ui/Tests

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Seems ok.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1862352.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: Unassigned » damiankloip

Working on this, not sure what I was thinking! We need to change namespaces. Might also decouple a few things while I'm there.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Assigned: damiankloip » Unassigned
Status: Needs work » Needs review
FileSize
1862352-6.patch27.25 KB

This is more what I was after, separating out the exposed form tests, as the Views plugin exposed form test is currently extending UITestBase, which is a bit on the grubby side.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
interdiff-1862352-7.txt519 bytes
1862352-7.patch27.25 KB

With the correct Contains ... in the new Exposed form test file.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work 
+++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/ExposedFormTest.phpundefined
@@ -104,125 +103,4 @@ public function testExposedFormRender() {
-  function testExposedAdminUi() {

Yeah for improving the performance of the total test suite! But no for removing the test coverage.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DefaultViewsTest.phpundefined
@@ -2,10 +2,10 @@
+ * Definition of Drupal\views_ui\Tests\DefaultViewsTest.

Definition should be contains.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayAttachmentTest.phpundefined
@@ -2,10 +2,10 @@
   /**
    * @file
-   * Contains \Drupal\views\Tests\UI\DisplayAttachmentTest.
+   * Contains \Drupal\views_ui\Tests\DisplayAttachmentTest.
    */

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayExtenderUITest.phpundefined
@@ -2,10 +2,10 @@
   /**
    * @file
-   * Definition of Drupal\views\Tests\UI\DisplayExtenderUITest.
+   * Contains \Drupal\views_ui\Tests\DisplayExtenderUITest.
    */

This hurts, can we place remove the spaces at the front?

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/ExposedFormUITest.phpundefined
@@ -49,62 +40,6 @@ protected function setUp() {
-   * Tests whether the reset button works on an exposed form.
-   */
-  public function testResetButton() {
...
-  public function testRenameResetButton() {
...
-    $view = views_get_view('test_reset_button');

I can't spot this old tests in the new patch. This feels wrong :)

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
1862352-9.patch27.29 KB

Yeah for improving the performance of the total test suite! But no for removing the test coverage.

I think the interdiff is confusing, but the coverage has not been removed just moved the views_ui/Tests/ExposedFormUITest. Same for testResetButton() etc.. they are still there. I only moved stuff, didn't remove any coverage.

Updated patch to fix that crazy @file indentation in those files and the 'Contains..'.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Oh I always run into this trap.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Issue tags: -VDC

#9: 1862352-9.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, 1862352-9.patch, failed testing.

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs work » Needs review
FileSize
1862352-13.patch27.98 KB

Drupal\translation_entity\Tests\Views\TranslationEntityViewsUITest got added too.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I just realized that /var/www/d8/core/modules/views/tests/Drupal/views/Tests/ViewsUI/ViewUIObjectTest.php could be moved as well.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

The rest looks perfect!

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
FileSize
1862352-16.patch28.78 KB

YES, good idea!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Nice!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 4889cf6 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.