Problem/Motivation

Discovered in #3361534: KernelTestBase::$strictConfigSchema = TRUE and BrowserTestBase::$strictConfigSchema = TRUE do not actually strictly validate.

For example:

Drupal\Tests\big_pipe\Kernel\BigPipeInterfacePreviewThemeSuggestionsTest::testBigPipeThemeHookSuggestions
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for block.block.test_block1 with the following errors: 0 [plugin] The 'test_html' plugin does not exist.

Steps to reproduce

Run Drupal core's test suite with #3361534: KernelTestBase::$strictConfigSchema = TRUE and BrowserTestBase::$strictConfigSchema = TRUE do not actually strictly validate applied, and with the lines

      // @see \Drupal\Core\Plugin\Plugin\Validation\Constraint\PluginExistsConstraint::$unknownPluginMessage
      // @todo Remove this in https://www.drupal.org/project/drupal/issues/3362457
      "The '.*' plugin does not exist.",

removed from \Drupal\Core\Config\Schema\SchemaCheckTrait::checkConfigSchema().

Proposed resolution

Solution for the example above:

diff --git a/core/modules/big_pipe/tests/src/Kernel/BigPipeInterfacePreviewThemeSuggestionsTest.php b/core/modules/big_pipe/tests/src/Kernel/BigPipeInterfacePreviewThemeSuggestionsTest.php
index 9064a6074a..928692d006 100644
--- a/core/modules/big_pipe/tests/src/Kernel/BigPipeInterfacePreviewThemeSuggestionsTest.php
+++ b/core/modules/big_pipe/tests/src/Kernel/BigPipeInterfacePreviewThemeSuggestionsTest.php
@@ -60,7 +60,7 @@ public function testBigPipeThemeHookSuggestions() {
     $entity = $this->controller->create([
       'id' => 'test_block1',
       'theme' => 'stark',
-      'plugin' => 'test_html',
+      'plugin' => 'system_powered_by_block',
     ]);
     $entity->save();
 

Remaining tasks

Fix all of these failures.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

Issue fork drupal-3362457

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

Issue summary: View changes
Issue tags: +DrupalCon Pittsburgh 2023
wim leers’s picture

Title: [PP-1] Fix all ExtensionExistsConstraint constraint violations in tests » [PP-1] Fix all PluginExistsConstraint constraint violations in tests
smustgrave’s picture

Issue tags: -DrupalCon Pittsburgh 2023 +Pittsburgh 2023
chrisdarke’s picture

Issue tags: -Pittsburgh 2023 +Pittsburgh2023

Migrating Pittsburgh 2023 to Pittsburgh2023 tag for cleanup

NickDickinsonWilde made their first commit to this issue’s fork.

nickdickinsonwilde’s picture

Assigned: Unassigned » nickdickinsonwilde

heh this one had more challenges in the functional than kernel/unit. Okay, will work on this later

borisson_’s picture

Assigned: nickdickinsonwilde » Unassigned
Status: Postponed » Needs work

No longer postponed, can be rerolled.

borisson_’s picture

Title: [PP-1] Fix all PluginExistsConstraint constraint violations in tests » Fix all PluginExistsConstraint constraint violations in tests
wim leers’s picture

Status: Needs work » Needs review
borisson_’s picture

This looks great, probably should be mentioned in the CR for #3362453: Fix all ConfigExists constraint violations in tests as well. Waiting for rtbc on the testbot status.

wim leers’s picture

Assigned: Unassigned » wim leers
Status: Needs review » Needs work

32 failures, on it!

wim leers’s picture

AFAICT 100% of test failures are related to block config entities pointing to block_content content entities.

Related: #2249303: Implement fallback plugin for Block plugins, #2248369: [meta] Deploying configuration that depends on content.

We just need to support FallbackPluginManagerInterface in PluginExists and all should be well. Not doing that would also be hugely disruptive.

wim leers’s picture

borisson_’s picture

Status: Needs review » Needs work

Still one fail

wim leers’s picture

It's unfortunate, but due to the way #2249303: Implement fallback plugin for Block plugins was implemented, we must expect NO validation error when an invalid block plugin ID is specified. Even if it's not pointing to a block_content:<uuid> plugin ID.

It'd have been better if #2249303 had special-cased block_content:* plugin IDs. OTOH, this means that other content-like block plugins that would otherwise run into #2248369: [meta] Deploying configuration that depends on content, also fall back automatically.

Improving that by narrowing when it falls back is the responsibility of the Block module, and would have a significant disruption risk, so it's out of scope here. The aim of this issue is to ensure all config is valid. And referencing a non-existing block plugin ID in a block config entity is valid ever since #2248369, unfortunately.

Created #3377709: BlockManager is too eager in providing fallback to "Broken" block: it obscures genuine problems for this.

wim leers’s picture

Status: Needs work » Needs review

Addressed the last failure, with expanded test coverage: one specifically for block_content:* that should not trigger a validation error now nor after #3377709: BlockManager is too eager in providing fallback to "Broken" block: it obscures genuine problems, and one test case that should trigger a validation error after #3377709.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Change looks good to me and all green.

lauriii’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

The MR needs to be rebased

wim leers’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll

Done. Trivial conflict in core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.php thanks to #3372972: Fix all type: machine_name constraint violations in tests having landed.

  • lauriii committed 4cb10842 on 11.x
    Issue #3362457 by Wim Leers, NickDickinsonWilde: Fix all...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4cb1084 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.