Problem/Motivation
A new contender on the random test failure stage is:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode with data set "without alignment" (false)
Behat\Mink\Exception\ElementNotFoundException: Element matching css "article.media--view-mode-_2222" not found.
/var/www/html/vendor/behat/mink/src/WebAssert.php:418
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:1554
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
Seen in the QA jobs (https://www.drupal.org/node/3060/qa):
https://www.drupal.org/pift-ci-job/2693933
https://www.drupal.org/pift-ci-job/2694177
Whilst researching this, another, less frequent random fail in the same method was detected:
1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode with data set "without alignment" (false)
Behat\Mink\Exception\ElementNotFoundException: Element matching css "article.media--view-mode-view-mode-1" not found.
/var/www/html/vendor/behat/mink/src/WebAssert.php:418
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:361
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
https://www.drupal.org/pift-ci-job/2694785
Since both random fails occur in the same test method, it's hard/impossible to get ~10.000 clean runs, needed to proof the patch is working, when solving only one of them.
Let's solve them both in this issue.
Steps to reproduce
The failing $assert_session->elementExists('css', 'article.media--view-mode-view-mode-X');s seem to not have enough waiting time before them to have the switching of the media view mode fully processed.
Note that there are 4 of these assertions and only 2 fail randomly. The ones that _don't_ fail randomly have a wait-assert of somekind in the lines before them.
Since there seem to be no visual nor non-visual (think HTML attribute change somewhere) change, there's no "nice way" to wait for the switching of the media view mode being fully processed.
Although I'm not a big fan of whacking waitForElement() on tests, it seems to me this is the only way to remove these random fails.
Proposed resolution
Replace the two randomly failing $assert_session->elementExists('css', 'article.media--view-mode-view-mode-X');s with similiar $assert_session->waitForElement('css', 'article.media--view-mode-X')s.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3367433-8.patch | 1.41 KB | spokje |
Comments
Comment #2
spokjeComment #3
spokjenvm
Comment #4
spokjeComment #5
spokjeLet's first proof that running
\Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode()and\Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode()alone produces both random test failures.Here's a patch that runs only
\Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testViewMode()500 times.Comment #6
spokjeThe normal routine to prove a random failure is fixed is to run the failing patch and the patch with the fix at the same time, whilst the latter has to have ~8000 - 10.000 failure free runs to prove it's credibility.
Comment #7
spokjeComment #8
spokje12 * 750 = 9000 failure free runs. Also, since
testViewModeis fed by a dataProvider with 2 cases, the actual method is run twice in each run.Here's the actual patch.
Comment #9
spokjeComment #10
smustgrave commentedLGTM
Comment #11
catchCommitted/pushed to 11.x and cherry-picked back through to 9.5.x, thanks!