Robustify and restore \Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButton

+++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaLibraryTest.php
@@ -194,7 +193,8 @@ public function testButton() {
-    $assert_session->waitForElement('css', '.js-media-library-item')->click();
+    $this->assertNotEmpty($assert_session->waitForId('drupal-modal'));
+    $assert_session->elementExists('css', '.js-media-library-item')->click();

Ohhhh — so it used to fail mostly because the modal dialog didn't ever appear, and hence the other selector surely cannot exist either?

But … how is that different than just using the original selector? 🤔

Ah … we could've kept the original selector too. The key thing is that we first check that the element exists and only then click() it.

@lauriii chose to just first assert to wait for the modal, because that is conceptually easier to understand: it documents in the test what we are waiting for. 👍

Ohhhh, I hadn't even thought about repositioning potentially causing clicks to fail! 🤯

To clarify: definitely RTBC now! 🚀

This undoes one of the skips in #3268070: Temporarily skip even more failing tests.

We need to run this many more times, and compare it to the baseline each time. I will post patches that do that.

So the baseline isn't failing under current conditions. We should retest these when there are more jobs (around when there are commits pushed or RTBC retesting is happening), or come up with a 100% fail patch that proves a race condition that is fixed by the fix.

Unfortunately the fails above are:

There was 1 error:

1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testAllowedMediaTypes
Behat\Mink\Exception\ElementTextException: The text "Fear is the mind-killer" was not found in the text of the element matching css ".media-library-item__name".

/var/www/html/vendor/behat/mink/src/WebAssert.php:821
/var/www/html/vendor/behat/mink/src/WebAssert.php:467
/var/www/html/core/tests/Drupal/Tests/WebAssert.php:1005
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaLibraryTest.php:252
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

--

There was 1 failure:

1) Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButton
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'Show Image media (selected)'
+''

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:90
/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaLibraryTest.php:164
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

....Whereas the fail we expect from the random fail is:

Testing Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest
  F..                                                                 3 / 3
(100%)

  Time: 00:39.943, Memory: 4.00 MB

  There was 1 failure:

  1)Drupal\Tests\ckeditor5\FunctionalJavascript\MediaLibraryTest::testButton
  Failed asserting that a NULL is not empty.


/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55

/var/www/html/core/modules/ckeditor5/tests/src/FunctionalJavascript/MediaLibraryTest.php:198
  /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

So, I'll try queuing more of #8 in a few hours.

Thanks @lauriii, that makes sense. This is also an identical fix to the one we've used for other race conditions in the past, so hopefully it will harden this test against other failures in the future.

Committed to 10.0.x, and cherry-picked to 9.4.x and 9.3.x. Thanks!

Forgot to mention that this one doesn't get 9.2.x backport because no CKE5 in 9.2.x. :)