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

Comments

Spokje created an issue. See original summary.

spokje’s picture

Issue summary: View changes
spokje’s picture

StatusFileSize
new2 KB

nvm

spokje’s picture

Issue summary: View changes
spokje’s picture

StatusFileSize
new58.67 KB

Let'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.

spokje’s picture

StatusFileSize
new58.67 KB
new59.83 KB

The 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.

spokje’s picture

Issue summary: View changes
spokje’s picture

StatusFileSize
new1.41 KB

12 * 750 = 9000 failure free runs. Also, since testViewMode is fed by a dataProvider with 2 cases, the actual method is run twice in each run.

Here's the actual patch.

spokje’s picture

Assigned: spokje » Unassigned
Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

LGTM

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked back through to 9.5.x, thanks!

  • catch committed f1e9d3fe on 10.0.x
    Issue #3367433 by Spokje: [random test failure] \Drupal\Tests\ckeditor5\...

  • catch committed 26e04469 on 10.1.x
    Issue #3367433 by Spokje: [random test failure] \Drupal\Tests\ckeditor5\...

  • catch committed 8053a61b on 11.x
    Issue #3367433 by Spokje: [random test failure] \Drupal\Tests\ckeditor5\...

  • catch committed b7af923e on 9.5.x
    Issue #3367433 by Spokje: [random test failure] \Drupal\Tests\ckeditor5\...

Status: Fixed » Closed (fixed)

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