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
Comments
Comment #2
wim leersComment #3
wim leersComment #4
smustgrave commentedComment #5
chrisdarke commentedMigrating Pittsburgh 2023 to Pittsburgh2023 tag for cleanup
Comment #8
nickdickinsonwildeheh this one had more challenges in the functional than kernel/unit. Okay, will work on this later
Comment #9
borisson_No longer postponed, can be rerolled.
Comment #10
borisson_Comment #11
wim leersComment #12
borisson_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.
Comment #13
wim leers32 failures, on it!
Comment #14
wim leersAFAICT 100% of test failures are related to
blockconfig entities pointing toblock_contentcontent entities.Related: #2249303: Implement fallback plugin for Block plugins, #2248369: [meta] Deploying configuration that depends on content.
We just need to support
FallbackPluginManagerInterfaceinPluginExistsand all should be well. Not doing that would also be hugely disruptive.Comment #15
wim leersThis conflicted with #3376794: `type: mapping` should use `ValidKeys: <infer>` constraint by default! Should be good again now :)
Comment #16
borisson_Still one fail
Comment #17
wim leersIt'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.
Comment #18
wim leersAddressed 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.Comment #19
smustgrave commentedChange looks good to me and all green.
Comment #20
lauriiiThe MR needs to be rebased
Comment #21
wim leersDone. Trivial conflict in
core/lib/Drupal/Core/Config/Schema/SchemaCheckTrait.phpthanks to #3372972: Fix all type: machine_name constraint violations in tests having landed.Comment #23
lauriiiCommitted 4cb1084 and pushed to 11.x. Thanks!