[Random test failure] Random failure in CKEditor5AllowedTagsTest::testMediaElementAllowedTags

Let's first proof that running Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5AllowedTagsTest::testMediaElementAllowedTags and Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5AllowedTagsTest::testMediaElementAllowedTags alone produces the random test failure.

Here's a patch that runs only Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5AllowedTagsTest::testMediaElementAllowedTags 1500 times.

Then again … even a Chrome version update can change this, so I guess it's not too surprising that we're able to detect new random failures later? 😔

JS random test failures are the latest and the greatest, most commonly due to changes in the speed of the testrunners.
Surprisingly not because they got so much faster, but since they get way slower...

That's … fascinating. We're literally first waiting for the field to appear, and only then checking the fields.

Even more fascinating, when the wait doesn't find the element, it returns NULL, when that's not checked anywhere, like in this test, since there's no execption thrown, the test continues (and finds out 2 lines below the element we were waiting on isn't present.

My suspicion: the checking of the first field triggers a new AJAX request, which makes the checkbox briefly uncheckable. So we need to explicitly wait before each checkField().

Sadly it isn't, looking into it during today/tomorrow.

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.

Turns out 5000x doesn't finish within about an hour, after which the results get "wonky".

Let's do it in 2500x runs.

That's 12 * 2500 = 30.000 failure free runs.

Good enough for me, here's the actual patch.

This part I do not understand. 🤔 Why does this need to be moved? 🤯
AFAICT this should not be necessary.

Did you see this from the IS?

As it turns out, we start the test by going to the 'admin/config/content/formats/add' page in \Drupal\Tests\ckeditor5\FunctionalJavascript\CKEditor5TestBase::createNewTextFormat, do some stuff and then create two new EntityViewModes.

This can be appearantly to slow for the page to pick up on it, or at least sometimes to pick up on the last of the two, which then does not get displayed when we enable the embed media option.

Was hoping that would explain things, if not, am happy to (try and) be more explicit.

The key here. IMHO. is the fact that in the current situation we're already on the 'admin/config/content/formats/add' page, before we create the EntityViewModes.
That somehow sometimes doesn't give us/the page enough time to "realize" it has two rather than one EntityViewMode to show when we click he embed media option.

This part I get: after enabling a filter, rather than just waiting for a field to appear via AJAX, wait for everything to have finished for said AJAX response 👍

Yep, I really dislike the use of waitFor[Stuff]($timePeriod).

For me it's like when you have to pick somebody up from the train station and you go there and wait for a certain amount of time, then no matter what, you continue with your other business. Even if the train arrives one second later.
So waitForPerson(20 minutes)

Whilst when you know _what_ you're waiting for, it's less random. In the above anology: If the person doesn't come out of the train which you expected them to be on, move on.
If the train's early or late, no problem, because you're not tied to a set amount of time.
So

waitForTrain($certainTrain)

is much less prone to random test failures when there's outside stuff messing with the trainschedule (or in our case, testrunners).