Media Tests should not rely on Classy

Some notes for reviewers...

1. In MediaAccessTest I had to make a change to the .view-media selector. But I couldn't pass up an opportunity to make the assertions below that considerably more meaningful. In a sense this was "required" because I was no longer able to select the view in a very convincing way.

2. In MediaJavascriptTestBase there's a helper method to assert links in a status message. This cannot use JsWebAssert::statusMessageContainsAfterWait because that method just checks text, not markup.

3. I put a @todo in CKEditorIntegrationTest and left it with Classy until Starterkit is ready. This one has tons of apparently important selection of classes.

4. In MediaSourceOEmbedVideoTest I remove a hunk of code that was added in #2982307: Misnamed template can cause fatal error in themes that do not extend Stable. No that the default theme is Stark, I believe that the hunk of code is no longer needed. It only makes sense if the test begins by running something that extends stable. This test also has one of those sneaky negative assertions with no positive counterpart that needed to be updated even though there wasn't a failure triggered moving from classy to stark.

5. MediaStandardProfileTest uses the standard profile, so it doesn't even need to declare a default theme.

6. This module has a ton of changes so I did not make any nice-to-have changes such as changing pageTextContains to statusMessageContains.

Another note. In MediaDisplayTest, the following hunk is improved in addition to being refactored for stark. The final assertion was originally added in #2934850: Media Images should be rendered at a reasonable size by default. But that assertion doesn't actually test that at all. It just tested if there was an img tag, with no regard for the image style. That's why the assertion now checks that the image src corresponds to the appropriate image style.

+++ b/core/modules/media/tests/src/FunctionalJavascript/MediaDisplayTest.php
@@ -193,14 +194,13 @@ public function testMediaDisplay() {
-    $assert_session->elementExists('css', '.field--name-field-related-media');
+    $assert_session->pageTextContains('Related media');
     // Media name element is not there.
-    $assert_session->elementNotExists('css', '.field--name-name');
     $assert_session->pageTextNotContains($image_media_name);
-    // Only one element is present inside the media container.
-    $this->assertCount(1, $page->findAll('css', '.field--name-field-related-media article.media--type-image > div'));
-    // Assert the image is present.
-    $assert_session->elementExists('css', '.field--name-field-related-media article.media--type-image img');
+    // Only one image is present.
+    $assert_session->elementsCount('xpath', '//img', 1);
+    // The image has the correct image style.
+    $assert_session->elementAttributeContains('xpath', '//img', 'src', '/styles/large/');
   }

Thanks for the review @larowlan. I have attached a new patch. Here are some notes.

4.1: Updated. Not sure how the patch even ran given this violation.

4.2: The presence of the label is used to assert the presence of the field since we no longer have a good css class to assert the presence of the field. I have updated a comment to perhaps clarify this slightly.

4.4: I have updated this so that an image is being found within the image field output. We're viewing a media at its "view" route, so this should be sufficient since any field on the page is a field from the media.

4.5: Very brittle! I have updated.

4.6: We are losing precision, but it's a negative assertion so I think it's ok to lose some precision. In fact, the lost precision may enhance the effectiveness of the test. We don't really care what the image style is. We just want to make sure the the oembed media is not being rendered as a media thumbnail (i.e. image). Rather, we only want it to be rendered as an oembed iframe, which is what the assertion before the img thing checks.

Let me know if I am unpersuasive about 4.6!

Or if anything else isn't quite right. Cheers!

That looks like a real failure related to the Standard profile recently being changed to use Olivero:

1) Drupal\Tests\media\FunctionalJavascript\MediaStandardProfileTest::testMediaSources
Behat\Mink\Exception\ExpectationException: 0 elements matching css "article.media--type-audio > *" found on the page, but should be 1.

I'll dig in.

Here's the story. Olivero uses a div instead of an article to render media. So I made that little swap in MediaStandardProfileTest.

But why didn't this fail everywhere else once standard started using Olivero? It's because elsewhere MediaStandardProfileTest still declares Classy to be the defaultTheme and that value is honored even when the standard profile is used. But our patch here is removing the defaultTheme declaration such that we are relying on the theme in the standard profile. Which is now Olivero.

Thanks for the review, @nod_!

And you beat me to noting the need for a reroll. That's because of this issue: #3271057: Move Media Library CKEditor 4 integrations from Media into CKEditor

That was fixed in 9.5.x, so I suspect the patch won't apply to D9.5.x either. I'll queue that up to confirm but I'm pretty sure. Adding the Need reroll tag and setting to NW.

I queued the patch in #11 up with 9.5. Back to NR. Sorry for the simul-posting confusion.