Fix tests on Drupal 9 and drop support for 8.7

A lot of

Unable to install modules: module 'entity_embed_test' is incompatible with this version of Drupal core.

See \Drupal\Core\Extension\InfoParserDynamic::parse() in Drupal core:

      if (!isset($parsed_info['core']) && !isset($parsed_info['core_version_requirement'])) {
        if (strpos($filename, 'core/') === 0 || strpos($filename, $this->root . '/core/') === 0) {
          // Core extensions do not need to specify core compatibility: they are
          // by definition compatible so a sensible default is used. Core
          // modules are allowed to provide these for testing purposes.
          $parsed_info['core_version_requirement'] = \Drupal::VERSION;
        }
        elseif (isset($parsed_info['package']) && $parsed_info['package'] === 'Testing') {
          // Modules in the testing package are exempt as well. This makes it 👈👈👈👈👈👈👈👈👈👈👈👈👈
          // easier for contrib to use test modules.
          $parsed_info['core_version_requirement'] = \Drupal::VERSION;
        }
        else {
          // Non-core extensions must specify core compatibility.
          throw new InfoParserException("The 'core' or the 'core_version_requirement' key must be present in " . $filename);
        }
      }

Apparently this is not working correctly!?

Ah, no, there's a problem in the test module:

name: 'Entity Embed test'
type: module
description: 'Support module for the Entity Embed module tests.'
core: 8.x
package: Testing

This should just remove the core key altogether 😄

Debugged the failures with @katherined. The good news is, they looked a lot worse than they actually were.

This should fix most of them, with the notable exception of the update path test. Due to the removal of all Drupal 8 update path testing materials in Drupal 9, we need to wait until Drupal 8.8 is the minimum supported version of core before we can fix that test.

Let's see how this does. If it passes, it means we cannot commit it until Drupal 8.8 is the oldest supported version of core.

Starting from the clean slate of https://www.drupal.org/pift-ci-job/1590206 ...let's see how many errors this fixes.

A quick experiment -- let's see if we can't have our cake and eat it too. If we can, then I can commit this right now.

Whoops, I meant theme_installer. Not theme_install.

That check doesn't work on 8.8/8.9 because both files exist. You need a less fancy version, I'd just put 8.8 in there and then after that you check if it does _not_ exist and if so, go back to 8.0

That said, the trusted callback stuff will either require 8.8 anyway or ugly hacks to dynamically define the interface.

I think at this point is pretty much given that the first scheduled 9.0 date won't happen, there's just too much left to do this week, so that also means we have more time until we have to drop 8.7 support, so my recommendation, as usual, would be to just go for ^8.8, do it cleanly and then wait for a good time to drop 8.7 :)

Thanks, @Berdir. Let's see how this does -- I just dropped backwards compatibility.

Okay, that took several hours of increasing frustration and confusion, but then with @oknate's invaluable -- dare I say heroic? -- debugging help, we realized that the test is breaking because, when we implement TrustedCallbackInterface on EntityReferenceFieldFormatter, the X-Drupal-Assertion header goes away, changing the size of the transfer.

That raises the question: why do we even care how much actual data was transferred over the wire? Don't we just care that it's non-zero? This seems plausible because, a few lines later, we specifically assert that a second preview request transferred zero bytes (since it's cached on the client side). Therefore, in theory, we should only need to assert that the first preview request transfers some data. Putting constraints on how much data it was, makes the test more brittle and doesn't seem to add much.

That raises the question: why do we even care how much actual data was transferred over the wire? Don't we just care that it's non-zero? This seems plausible because, a few lines later, we specifically assert that a second preview request transferred zero bytes (since it's cached on the client side). Therefore, in theory, we should only need to assert that the first preview request transfers some amount of bytes (non-zero).

Yes, we don't care about the specific number of bytes transfered. We care that it's non-zero vs zero. I was just being cautious about it not accepting a broken response with like a PHP fatal error. But … it doesn't materially matter — non-zero vs zero is what materially matters. (Although a one-byte response could never contain a meaningful preview, that's not the scope of this particular test, so it doesn't materially matter.)

+1 to this change :)

+++ b/tests/src/FunctionalJavascript/MediaImageTest.php
@@ -719,7 +719,11 @@ class MediaImageTest extends EntityEmbedTestBase {
+      'query' => [
+        'XDEBUG_SESSION_START' => 1,
+      ],

We definitely don't want this to get committed 😅

Fixed!

Crediting Wim and Berdir for their help.

Sorry, one more:

+++ b/tests/modules/entity_embed_test/entity_embed_test.info.yml
@@ -1,7 +1,7 @@
-core: 8.x
+core_version_requirement: ^8.7.7 || ^9

+++ b/tests/modules/entity_embed_translation_test/entity_embed_translation_test.info.yml
@@ -1,7 +1,7 @@
-core: 8.x
+core_version_requirement: ^8.7.7 || ^9

Test modules don't need to have this specified. You can just delete these lines.

Done.

On another reading, I realized that you wanted me to delete the core: 8.x lines as well. So that's done here.

test modules not needing this is only in core since 8.8.2, so you'd need to depend on that in the main file :) That said, you would need to set it to at least ^8.8 with the current patch anyway.

That said, you would need to set it to at least ^8.8 with the current patch anyway.

Exactly :)

Yeah, but the main .info.yml currently contains "core_version_requirement: ^8.7.7 || ^9", which would allow to install on 8.7 and then fatal on the test modules.

And it also contains a unecessary >=8.4 requirement that could be cleaned up.

Good points. Fixed here.

Committed and pushed to 8.x-1.x. Thanks, everyone!