Follow-up from #3613347: Re-use container in kernel tests and #1411074: Allow kernel tests to share the test environment.
We are moving towards kernel tests sharing a database/environment between methods by default, with environment-per-method as opt in.
We added a performance optimisation a long time ago to use a memory key/value in kernel tests, however a shared database requires the key/value store to use the regular database storage.
The combined performance gain of those two issues is approaching a 50% speed-up in some tests, so we can simplify things by always using the database key/value store and removing the special handling in kernel tests, which simplifies things a bit too.
#3613347: Re-use container in kernel tests does that but leaves the $keyValue property intact, in this issue, we should deprecate the property, update any tests that rely on it, and add a change record for contributed modules.
| Comment | File | Size | Author |
|---|
Show commandsStart 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:
changes
Comments
Comment #2
sapnil_biswas commentedGetting started on this
Comment #4
longwaveThe MR looks good but we need a change record, then point the deprecation to the change record instead of this issue.
Comment #5
sapnil_biswas commentedCreated the change record at change record #3629745 and updated the deprecation message and test coverage to reference it.
Comment #6
sapnil_biswas commentedComment #7
catchThis is used less than I would have expected.
Comment #8
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #9
sapnil_biswas commentedComment #10
abryantdev commentedReviewed MR !17396 on current main (the patch applies cleanly), PHP 8.5.12, SQLite.
KernelTestBaseTest passes (19 tests, 51 assertions), including the new testGetKeyValuePropertyDeprecation.
WorkspaceFormPersistenceTest passes (1 test, 14 assertions).
One side effect to consider. KernelTestBase::__get() returns NULL for every property name other than keyValue. On main, reading an undefined property in a kernel test raises PHP's "Undefined property" warning. With this MR it is silent. I checked with a scratch kernel test that does assertNull($this->typoedProperty): on main it reports 1 warning ("Undefined property: ...::$typoedProperty"), with the MR it passes with no warning. A typo in a property name in any kernel test, or in contrib, would stop being reported.
Suggestion: after the keyValue branch, trigger an E_USER_WARNING with the same text PHP uses, for example sprintf('Undefined property: %s::$%s', static::class, $name), then return NULL. That keeps the deprecation and keeps the old warning for everything else.
I did not change the issue status.
AI use: an AI assistant set up and ran these tests and drafted this comment. I reviewed the results before posting.