Problem/Motivation

Functional tests do a full site installation, which means installing config for the install profile and any modules for each method.

During install, when $strictConfigSchema is set to TRUE, ConfigSchemaChecker takes around 600ms per install (with xhprof overhead), so in a test with ten methods, that can be 6 seconds.

During a test run, we only really need to check configuration schema for config shipped with modules/profiles once, not every method of every test in which they're installed, which can be literally thousands in the case of system and user module.

Additionally, we automatically check strict config for functional tests, but there's a @todo to apply this to contrib because at the moment checking is either on or off for all of contrib.

Steps to reproduce

Proposed resolution

Don't automatically set ::$strictConfigSchema() in functional tests, default to FALSE and let individual tests set it where needed. This will also allow it to be used in contrib tests on an opt-in basis.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3615984

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

catch created an issue. See original summary.

catch’s picture

Status: Active » Needs work

Pushed a commit.

- removes the property from TestSetupTrait
- sets it to TRUE on kernel tests for now, although it may be a performance issue for kernel tests too - will push a separate commit setting it to false there later to see how that looks.
- sets it to FALSE on browser tests
- sets it back to TRUE on GenericModuleTestBase - this ensures every module's default config gets strict schema checked.
- also sets it to TRUE in the umami, standard, and minimal functional tests.

catch’s picture

Confirmed that this is taking a decent amount of time in kernel tests - e.g. for NodeFieldAccessTest just under 300ms is spent checking config schema for about 30 config saves. That's a single-method test that otherwise does a lot of config entity saving. This will be more variable between kernel tests because the amount of config they save can be vastly different.

borisson_’s picture

While the performance benefit of this is clear. I'm not sure about this. During the development of a lot of the schema strictness improvements, we found a lot of tests with broken config, even where the config was incorrect in some test modules or where it was created in an incorrect way inside a test method.
I think making this change can lead to places is some tests where there keeps being incorrect config.

catch’s picture

@borisson_ yes it's not clear cut, but it feels like overkill applying this to every config save in every config test too. If we had a way to only do it once per object that would be great but not how our tests work.

We could potentially have an additional pipeline on push/daily that overrides the behaviour via an environment variable?

Generic tests should be enough to catch issues in actual core modules for pipeline/on-commit tests, for test module config we're really 'testing the tests' so getting feedback slightly later would be annoying, but should be rare compared to how many pipelines we run in a year.

borisson_’s picture

Generic tests should be enough to catch issues in actual core modules for pipeline/on-commit tests, for test module config we're really 'testing the tests' so getting feedback slightly later would be annoying, but should be rare compared to how many pipelines we run in a year.

I think I agree that the benefits outweigh the negatives here. I haven't looked at the MR yet, so no other feedback yet.