Intermittent fails in Drupal\views_ui\Tests\DisplayTest

I ran this test locally and it took 13 minutes 37 seconds...

Did some tests with xhprof and I think it only showed me bogus memory usage, because the numbers were very low. Note that it's also possible that we have random endless loop here, those also result in oom fatal errors. Doesn't seem that likely, though.

As usual when I'm profiling tests, found a few other issues, posted my general findings in #2006434-16: [meta] Speed up web tests.

Maybe this was #2190421: Fix some services that shouldn't be serialized. We had a more or less persistent similar fail in UserCancelTest and PHP 5.4+ completely broken due to that.

Please report any failures about this that you see here, otherwise we can close it in a week or so?

Actually, what I think will help is #2190643: Serializing the container is a very very bad idea, let's prevent it?, because earlier test fails there showed that this class is also affected by that.

Faced with that issue 3-4 times in #2191709-7: Remove the "configurable" flag on field types

WE could split this into two tests? > 10 mins for one test is not really that cool.

Yes, I guess that would help, but I haven't seen this fail in quite some time, the only random fail that I've seen recently is the image field one.

So maybe change to a non-major task?

I split the tests into two classes:

- one class which handles adding and removing displays (DisplayTestCRUD)
- another class which handles the finer-grained actions in the UI (reordering, link displays, etc.) - DisplayTest

To cut down on duplicated code I used a trait to share code between them.

Test results:

Before patch:

$ php core/scripts/run-tests.sh --url http://local.drupal.org --color --class "Drupal\views_ui\Tests\DisplayTest"
Display tests 344 passes, 0 fails, 0 exceptions, 86 debug messages
Test run duration: 4 min 54 sec

After:

$ php core/scripts/run-tests.sh --url http://local.drupal.org --color --class "Drupal\views_ui\Tests\DisplayTest"
Display element tests 212 passes, 0 fails, 0 exceptions
Test run duration: 2 min 44 sec

$ php core/scripts/run-tests.sh --url http://local.drupal.org --color --class "Drupal\views_ui\Tests\DisplayTestCRUD"
Display CRUD UI tests 132 passes, 0 fails, 0 exceptions
Test run duration: 1 min 26 sec

What's the best way to instrument memory usage on run-scripts.sh?

You could add a call to memory_get_peak_usage() at the end, print it with format_size().

Edit: Probably better to do it right after the test was executed the end is a bit complicated in case of that script ;)

o_O

 $ gn diff 1
git diff core/scripts/run-tests.sh
diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh
index 5594dd5..4cbab7b 100755
--- a/core/scripts/run-tests.sh
+++ b/core/scripts/run-tests.sh
@@ -98,6 +98,8 @@
   simpletest_clean_results_table();
 }
 
+echo 'Memory peak usage: ' . round(memory_get_peak_usage(TRUE) / 1024 / 1024, 2) . ' MB' . PHP_EOL;
+
 // Test complete, exit.
 exit;

Before patch:

$ php core/scripts/run-tests.sh --url http://local.drupal.org --class "Drupal\views_ui\Tests\DisplayTest"
Display tests 344 passes, 0 fails, 0 exceptions
Test run duration: 4 min 11 sec
Memory peak usage: 14.75 MB

After:

$ php core/scripts/run-tests.sh --url http://local.drupal.org  --class "Drupal\views_ui\Tests\DisplayTest"
Display element tests 212 passes, 0 fails, 0 exceptions
Test run duration: 2 min 42 sec
Memory peak usage: 15.25 MB

$ php core/scripts/run-tests.sh --url http://local.drupal.org  --class "Drupal\views_ui\Tests\DisplayTestCRUD"
Display CRUD UI tests 132 passes, 0 fails, 0 exceptions
Test run duration: 1 min 28 sec
Memory peak usage: 15.25 MB

I love the idea of not pack everything into one big test!

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayTestTrait.php
@@ -0,0 +1,37 @@
+
+trait DisplayTestTrait {

A bit of documentation would be maybe helpful.

Boop

There's a lot of weird cruft in that patch...must have forgot to rebase.

+++ b/core/modules/views_ui/lib/Drupal/views_ui/Tests/DisplayTestTrait.php
@@ -0,0 +1,40 @@
+trait DisplayTestTrait {
...
+    $this->drupalPostForm('admin/structure/views/add', $view, t('Save and edit'));

Sorry, this just feels like using a trait for the sake of it. This is not really a trait-like case IMO. It also has a dependency on WebTestBase.

I think randomView() should just move into the UITestBase class as this is generally a handy method - being able to create arbitrary views via the UI.

Good call. There might have been a touch of "cool, PHP 5.4" tbh :P

In general I consider traits as just a solution to not require base-classes for things which are technically just a copy&paste, so not sure whether dependencies actually matter ...

Sure, but a base class usually has its own dependencies in order :) I think it is maybe not as useful in test classes tbh.

Great! Looks good here, apart from:

+/**
+ * @file
+ * Contains \Drupal\views_ui\Tests\DisplayTest2.
+ */

Fixed that on commit, and committed/pushed to 8.x. Thanks!

This just failed again in https://qa.drupal.org/pifr/test/742998

The test did not complete due to a fatal error. Completion check DisplayTestCRUD.php 71 Drupal\views_ui\Tests\DisplayTestCRUD->testRemoveDisplay()

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 536870912 bytes) in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php on line 336
FATAL Drupal\views_ui\Tests\DisplayTestCRUD: test runner returned a non-zero error code (255).

Haven't seen this in a very long time.