Test that all core content+config entity types have functional REST test coverage

@vaplas: thanks so much for creating this issue! And then on top of that, you also created a patch! Awesome :)

I think this looks ready. We now need to get it to green… which means that we need to fix every issue mentioned in #2824572: Write EntityResourceTestBase subclasses for every other entity type. :) So postponing it on that!

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,119 @@
+      ________________________
+     /           Hi!          \\
+    | What about a few of cool |
+    |        REST tests?       |
+    |                          |
+    |     Progress: $g/$a.     |
+    | ________________________/
+    |/
+//  o
+l'>
+ll
+llama
+|| ||
+'' ''
+";

HAHAHAHHAHAHAHAHAHAHAHHAHAHAHAHAHAHAHAHAHAHAH

THANK YOU :D

Now that #2824572: Write EntityResourceTestBase subclasses for every other entity type. is finally reaching the end, let's drive this patch to completion.

content_moderation_rest_resource_alter() ensures the ContentModerationState entity type is never exposed via REST. We need to take removed REST resource plugin definitions into account.

Additionally, REST test coverage for entity types being added by experimental modules don't live in \Drupal\Tests\rest, but in Drupal\Tests\experimental_module_name\Functional\Rest.

This updates the test coverage to look in multiple places for the test coverage.

#13 is the same as #9, just with added comments. But there's >5 months of progress with REST test coverage going from #9's test result to #13's.

#9:

Drupal\Tests\rest\Functional\EntityResource\EntityResourceRestTestCoverageTest::testEntityTypeRestTestCoverage
☼
      ________________________
     /           Hi!          \
    | What about a few of cool |
    |        REST tests?       |
    |                          |
    |     Progress: 19/42.     |
    | ________________________/
    |/
//  o
l'>
ll
llama
|| ||
'' ''

Failed asserting that Array &0 (
    0 => 'Feed (Drupal\aggregator\Entity\Feed)'
    1 => 'BlockContentType (Drupal\block_content\Entity\BlockContentType)'
    2 => 'BlockContent (Drupal\block_content\Entity\BlockContent)'
    3 => 'Message (Drupal\contact\Entity\Message)'
    4 => 'ContactForm (Drupal\contact\Entity\ContactForm)'
    5 => 'ContentModerationState (Drupal\content_moderation\Entity\ContentModerationState)'
    6 => 'FieldStorageConfig (Drupal\field\Entity\FieldStorageConfig)'
    7 => 'FieldConfig (Drupal\field\Entity\FieldConfig)'
    8 => 'File (Drupal\file\Entity\File)'
    9 => 'ContentLanguageSettings (Drupal\language\Entity\ContentLanguageSettings)'
    10 => 'RdfMapping (Drupal\rdf\Entity\RdfMapping)'
    11 => 'ResponsiveImageStyle (Drupal\responsive_image\Entity\ResponsiveImageStyle)'
    12 => 'RestResourceConfig (Drupal\rest\Entity\RestResourceConfig)'
    13 => 'Menu (Drupal\system\Entity\Menu)'
    14 => 'Tour (Drupal\tour\Entity\Tour)'
    15 => 'View (Drupal\views\Entity\View)'
    16 => 'Workflow (Drupal\workflows\Entity\Workflow)'
    17 => 'BaseFieldOverride (Drupal\Core\Field\Entity\BaseFieldOverride)'
    18 => 'EntityViewMode (Drupal\Core\Entity\Entity\EntityViewMode)'
    19 => 'FieldLayoutEntityViewDisplay (Drupal\field_layout\Entity\FieldLayoutEntityViewDisplay)'
    20 => 'EntityFormMode (Drupal\Core\Entity\Entity\EntityFormMode)'
    21 => 'FieldLayoutEntityFormDisplay (Drupal\field_layout\Entity\FieldLayoutEntityFormDisplay)'
    22 => 'DateFormat (Drupal\Core\Datetime\Entity\DateFormat)'
) is identical to Array &0 ().

#13:

Drupal\Tests\rest\Functional\EntityResource\EntityResourceRestTestCoverageTest::testEntityTypeRestTestCoverage
☼
      ________________________
     /           Hi!          \
    | What about a few of cool |
    |        REST tests?       |
    |                          |
    |     Progress: 34/42.     |
    | ________________________/
    |/
//  o
l'>
ll
llama
|| ||
'' ''

Failed asserting that Array &0 (
    0 => 'Message (Drupal\contact\Entity\Message)'
    1 => 'ContentModerationState (Drupal\content_moderation\Entity\ContentModerationState)'
    2 => 'File (Drupal\file\Entity\File)'
    3 => 'Workflow (Drupal\workflows\Entity\Workflow)'
    4 => 'EntityViewMode (Drupal\Core\Entity\Entity\EntityViewMode)'
    5 => 'FieldLayoutEntityViewDisplay (Drupal\field_layout\Entity\FieldLayoutEntityViewDisplay)'
    6 => 'EntityFormMode (Drupal\Core\Entity\Entity\EntityFormMode)'
    7 => 'FieldLayoutEntityFormDisplay (Drupal\field_layout\Entity\FieldLayoutEntityFormDisplay)'
) is identical to Array &0 ().

All hail the ASCII llama. We need more test messages like that! Also I like the idea of test coverage covering test coverage.

#17 showed that we are at 34/42. #18's correction makes this 34/41 (ContentModerationState is not exposed via REST). #19's correction makes this 35/41 (the Workflow entity type's test coverage lives in the workflows module, not in the rest module).

So we're down to 6:

Failed asserting that Array &0 (
    0 => 'Message (Drupal\contact\Entity\Message)'
    1 => 'File (Drupal\file\Entity\File)'
    2 => 'EntityViewMode (Drupal\Core\Entity\Entity\EntityViewMode)'
    3 => 'FieldLayoutEntityViewDisplay (Drupal\field_layout\Entity\FieldLayoutEntityViewDisplay)'
    4 => 'EntityFormMode (Drupal\Core\Entity\Entity\EntityFormMode)'
    5 => 'FieldLayoutEntityFormDisplay (Drupal\field_layout\Entity\FieldLayoutEntityFormDisplay)'
) is identical to Array &0 ().

But that lists FieldLayoutEntityViewDisplay rather than EntityViewDisplay and FieldLayoutEntityFormDisplay rather than EntityFormDisplay. This is because the experimental field_layout module does this:

function field_layout_entity_type_alter(array &$entity_types) {
  /** @var $entity_types \Drupal\Core\Entity\EntityTypeInterface[] */
  $entity_types['entity_view_display']->setClass(FieldLayoutEntityViewDisplay::class);
  $entity_types['entity_form_display']->setClass(FieldLayoutEntityFormDisplay::class);

Therefore the 6 remaining issues in #2824572: Write EntityResourceTestBase subclasses for every other entity type. do in fact match! (This patch adds the entity type ID to make the output a bit clearer.)

All hail the ASCII llama.

:P

We need more test messages like that!

I'm not sure core committers will be willing to commit it like this… although I agree that'd be very cool.

Also I like the idea of test coverage covering test coverage.

Not my idea! The Help module pioneered this, to ensure that every module has a hook_help() implementation :)

I'm not sure core committers will be willing to commit it like this… although I agree that'd be very cool.

We need more funny stuff like this to be honest :(

Not my idea! The Help module pioneered this, to ensure that every module has a hook_help() implementation :)

Cool, you learn something every day.

Great, now the failed test output is crystal clear:

Drupal\Tests\rest\Functional\EntityResource\EntityResourceRestTestCoverageTest::testEntityTypeRestTestCoverage
☼
      ________________________
     /           Hi!          \
    | What about a few of cool |
    |        REST tests?       |
    |                          |
    |     Progress: 35/41.     |
    | ________________________/
    |/
//  o
l'>
ll
llama
|| ||
'' ''

Failed asserting that Array &0 (
    0 => 'contact_message: Message (Drupal\contact\Entity\Message)'
    1 => 'file: File (Drupal\file\Entity\File)'
    2 => 'entity_view_mode: EntityViewMode (Drupal\Core\Entity\Entity\EntityViewMode)'
    3 => 'entity_view_display: FieldLayoutEntityViewDisplay (Drupal\field_layout\Entity\FieldLayoutEntityViewDisplay)'
    4 => 'entity_form_mode: EntityFormMode (Drupal\Core\Entity\Entity\EntityFormMode)'
    5 => 'entity_form_display: FieldLayoutEntityFormDisplay (Drupal\field_layout\Entity\FieldLayoutEntityFormDisplay)'
) is identical to Array &0 ().

Let's show the actual number of blockers.

This blocks #2910883: Move all entity type REST tests to the providing modules.

Okay, back to postponed until #2824572: Write EntityResourceTestBase subclasses for every other entity type. is done!

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,160 @@
+    $all_modules = array_filter($all_modules, function ($module) {
+      // Filter contrib, hidden, already enabled modules and modules in the
+      // Testing package.
+      if ($module->origin !== 'core' || !empty($module->info['hidden']) || $module->status == TRUE || $module->info['package'] == 'Testing') {
+        return FALSE;
+      }
+      return TRUE;
+    });

Let's give $all_modules a differnet name as its no longer all modules :)

#2843780: EntityResource: Provide comprehensive test coverage for EntityFormMode entity landed!

Once #2800873: Add XML GET REST test coverage, work around XML encoder quirks lands, we'll need to update this to verify there's XML test coverage too!

#2843781: EntityResource: Provide comprehensive test coverage for EntityViewMode entity landed!

#2843755: EntityResource: Provide comprehensive test coverage for Message entity landed!

#2843764: EntityResource: Provide comprehensive test coverage for EntityFormDisplay entity and #2843765: EntityResource: Provide comprehensive test coverage for EntityViewDisplay entity landed!

Let's get this patch tested again. We should be down to one failure now.

Also addressing @dawehner's feedback in #29 and my own nits:

1. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,160 @@
   +      // Filter contrib, hidden, already enabled modules and modules in the
   +      // Testing package.
   +      if ($module->origin !== 'core' || !empty($module->info['hidden']) || $module->status == TRUE || $module->info['package'] == 'Testing') {
   +        return FALSE;
   +      }
   

   We should also exclude experimental modules — it's okay for experimental modules to not yet have full REST test coverage.

2. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,160 @@
   +    \Drupal::service('module_installer')->install(array_keys($all_modules));
   ...
   +    $this->definitions = \Drupal::entityTypeManager()->getDefinitions();
   ...
   +    $resource_plugin_ids = array_keys(\Drupal::service('plugin.manager.rest')->getDefinitions());
   

   These should all use $this->container->get(…)

3. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,160 @@
   +  protected function getLlamaMessage($good, $all) {
   +
   +    $a = $all;
   

   Extraneous newline.

4. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,160 @@
   +    | What about a few of cool |
   +    |        REST tests?       |
   

   We should fix the language here.

Assuming this comes back with the expected failure for the sole remaining REST test coverage issue (#2843139: EntityResource: Provide comprehensive test coverage for File entity, and tighten access control handler), I think we should add a work-around for that for now so that this can already be committed. Then at least we know we won't be adding more entity types without REST test coverage: this patch will ensure we'll never regress!

Which means this issue would no longer be blocked! (Updating issue title accordingly.)

Thoughts?

@Wim Leers I think the work around is good idea, the file issue could take a while and in the meantime we won't have test to force REST coverage for any new entities.

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,166 @@
+     /           Hi!          \\
+    |  It's llame to not have  |
+    |   complete REST tests!   |

All my test messages are now llame in comparison.

#38: Great! Does that mean this becomes RTBC? :)

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,166 @@
+    $a = $all;
+    $g = $good;
+
+    if ($g < 10) {
+      $g = "0$g";
+    }

This is weird, why do we do that. This makes it harder to read IMHO

1. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,165 @@
   +      if ($module->origin !== 'core' || !empty($module->info['hidden']) || $module->status == TRUE || $module->info['package'] == 'Testing' || $module->info['package'] == 'Core (Experimental)') {
   +        return FALSE;
   +      }
   +      return TRUE;
   

   🔧 You could get rid of the if by negating the entire statement and return it.

2. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
   @@ -0,0 +1,165 @@
   +    $a = $all;
   +    $g = $good;
   

   Now that we just assign a variable I see even less of a reason to assign them to some new variable locally.

$module->status === 0, which is why the === FALSE check didn't work, but == FALSE does work.

👍

90% of the work here was done by @vaplas. This has been ready for about a week now, with no one else stepping up to RTBC it. So I will.

This has been RTBC for a week, but I figured another review would help the committers be more certain that this works as expected.

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,163 @@
+    elseif ($problems === []) {
+      $this->fail('Drupal\file\Entity\File now supports REST test coverage. We need remove @todo with work-around for pass it.');
+    }

This is awesome, it will automatically alert us when the rest coverage for files is fixed!

RTBC++

+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -0,0 +1,163 @@
+      $this->fail('Drupal\file\Entity\File now supports REST test coverage. We need remove @todo with work-around for pass it.');

the english is off here

nice work though

Fixed.

This is epic!

I found a few more docs nitpicks, but I was so excited about this patch that I fixed them on commit:

diff --git a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
index 613d327dbb..6b7bc6fd92 100644
--- a/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
+++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceRestTestCoverageTest.php
@@ -11,14 +11,14 @@
  * - every format in core (json + hal_json)
  * - every authentication provider in core (anon, cookie, basic_auth)
  *
- * @todo also require xml after https://www.drupal.org/node/2800873 lands
+ * @todo Also require XML after https://www.drupal.org/node/2800873 lands.
  *
  * @group rest
  */
 class EntityResourceRestTestCoverageTest extends BrowserTestBase {
 
   /**
-   * Entities definitions array.
+   * Entity definitions array.
    *
    * @var array
    */
@@ -32,8 +32,8 @@ protected function setUp() {
 
     $all_modules = system_rebuild_module_data();
     $stable_core_modules = array_filter($all_modules, function ($module) {
-      // Filter contrib, hidden, already enabled modules and modules in the
-      // Testing and experimental packages.
+      // Filter out contrib, hidden, testing, and experimental modules. We also
+      // don't need to enable modules that are already enabled.
       return
         $module->origin === 'core' &&
         empty($module->info['hidden']) &&

...That almost went badly because I struggle to type "XML" correctly since it's so close to my username.

Committed and pushed to 8.5.x!! 🎆🎆🎆

That is the most amazing thing I have seen on the entire internet.

#57: 🎉 😁

#58: 😵 🤣

WHERE DO YOU EVEN FIND THOSE THINGS VAPLAS???!?!?!?!?!?!??!!?!??!!!!!!

@vaplas already updated #2843139, to remove the @todo that this introduced: #2843139-96: EntityResource: Provide comprehensive test coverage for File entity, and tighten access control handler.

And I just updated #2800873 to ensure that the "test coverage test" that this issue introduced also verifies that there is XML REST test coverage for every entity type: #2800873-163: Add XML GET REST test coverage, work around XML encoder quirks.