[random test failure] MediaTest::testLinkManualDecorator()

Ugh! Thanks for reporting this! 🙏😊

Per #3325336-13: Add explicit test coverage to prove multiple CKEditor 5 instances on the same page can co-exist, in this DrupalCI run.

Updating title per special issue titles.

Does this need two issues or is the same underlying problem?

This happened on core 9.5 today. I know that we know this now, but adding the link for future reference
https://www.drupal.org/pift-ci-job/2626370

Does this need two issues or is the same underlying problem?

@quietone in #6

I _think_ I found the problem with MediaTest::testLinkManualDecorator() and that seems unrelated to MediaTest:: testEditableCaption(), so repurposing this issue for the first failure and creating a new issue for the second one.

Let's first proof that running MediaTest::testLinkManualDecorator() alone, without any other tests in \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest still produces a random test failure.

Here's a patch that runs MediaTest::testLinkManualDecorator() and MediaTest::testLinkManualDecorator() alone 1500 times.

The actual fix should be back-portable (is that even a word?) to 9.4.x if needed, but since I'm needing to run the patches multiple times to proof both the "random" and the "failure" bit in random failure are solved, putting this to 11.x-dev, so I don't need to manually change the branch manually a gazillion times.

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.

So let's do that here (and do a write up in the IS to explain why this is the fix).

Hmmm,

promising, but the lack of a roll of dried tobacco leaves that people smoke, like a cigarette but bigger and without paper around it is disheartening.
At the very least the failure rate has dropped dramatically.

The failures in Sqlite are table locks, which most probably happen because I started 5 1500x runs at the same time.

The single test failure in PHP 8.1 & MySQL 5.7 is most probably cause by the fact that after I click the "Toggle caption off"-balloon-button (or rather, let TestBot click it 1500 times), I have no waiting period for the text-box to disappear and the "Drupal Media toolbar" being updated.

Let's add that wait and see what happens with the attached patches.

So there are 2 remaining random test failures:

1. 1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testLinkManualDecorator with data set "unrestricted" (true)
   Behat\Mink\Exception\ElementNotFoundException: Element matching css ".ck-balloon-panel_visible .ck-balloon-rotator__content > .ck.ck-link-actions" not found.
   
   /var/www/html/vendor/behat/mink/src/WebAssert.php:418
   /var/www/html/core/modules/ckeditor5/tests/src/Traits/CKEditor5TestTrait.php:116
   /var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:284
   /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
     

and

2. 1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testLinkManualDecorator with data set "unrestricted" (true)
   Failed asserting that an object is not empty.
   
   /var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
   /var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
   /var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:287
   /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
     

Both are "wierd" (Well, wadda you expect from random JS failures...).

Looking at patch [3362864-60], which adds some screenshots and logging when those failures occur:

1. The screenshot shows that the link was _NOT_ created and the expected balloon is _NOT_ present.

Weird stuff since it has passed all assertions on buttons being present, so the only thing I can come up with is that every once in a blue moon the actual CKEditor 5 (so not the Drupal code around it) fails to create a link.

2. The screenshot shows that the link was created and the expected balloon is present.

However the contents of getEditorDataAsHtmlString is  , which seems "odd".

1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest::testLinkManualDecorator with data set "unrestricted" (true)
getEditorDataAsHtmlString is:  
Failed asserting that an object is not empty.

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaTest.php:291
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

My gut feeling is that 1) could be an upstream problem and 2) being a result of 1).

I'm a bit unsure if it's worth to open a new follow-up issue on these, since the failure rate seems in the range of /shrug-js-tests-fail-once-every-blue-moon.

Seeing the fact that this change drops the failure rate from about 10 in 1500 down to 6 in 45.000, and that there's a (IMHO) valid root cause identified, I'm going to post the actual patch for review.

The decision if we need a follow-up for the remaining (very less frequent) test-failures remains open.

Think it if it significantly helps the fail rate that's better then what's currently there. I'm +1 for opening a follow up for the other failures (sticking to the agile thought) but will let the committer decide if it's worth it.

Committed 3b6179f and pushed to 11.x. Thanks! Cherry-picked all the way down to 9.5.x.