Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Currently when adding services as properties in tests you need to declare the properties and initialize them in ::setUp. This is unnecessary boilerplate.
Proposed resolution
- Add a trait similar to AutowireTrait to initialize the services in setUp.
- Add symfony/expression-language as a dev dependency. This allows adding autowiring based on expressions rather than pure service or parameter, like for example
#[AutowireProperty(expression: "service('entity_type.manager').getStorage('node')")]. - symfony/expression-language itself requires two more Symfony components through its dependency chain: symfony/cache and symfony/cache-contracts
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3464357
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:
- 3464357-autowire-properties
changes, plain diff MR !8978 - 3464357-autosetup changes, plain diff MR !8965
Comments
Comment #3
mstrelan commentedComment #4
smustgrave commentedLeft a comment on the MR.
Comment #5
mondrakeLike the idea; IMHO we should always explicitly add the
#[Autowire()]attribute to the property.If we do this, then I think we should consider adding the trait to
KernelTestBaseandBrowserTestBase.Comment #6
mstrelan commentedThanks for review, responding to both. Have also split up the setUp method in the trait so classes that have their own setUp method can just call autoSetUp without having parent::setUp called twice.
Comment #8
mstrelan commentedAdded another MR based on suggestions from @mondrake in #5
Comment #9
mondrakeI tried to use Symfony's
#[Autowire]attribute to make things more explicit - PoC so did not change everything.Ideally we could mark
autowireProperties()with#[Before]and let PHPUnit call it prior to setUp, but the container is not yet defined at that point.Comment #10
mstrelan commented@mondrake I accidentally force pushed the wrong branch, can you push your branch back?
Comment #11
mondrake@mstrelan done!
Comment #12
mondrakeComment #14
mondrakeDrupal\Tests\content_moderation\Kernel\WorkspacesContentModerationStateTest::testNonLangcodeEntityTypeModerationfails because it inherits fromDrupal\Tests\content_moderation\Kernel\ContentModerationStateTest, but the inherited properties are not scanned for autowiring - we need to take care of inheritance especially for base test classes.Comment #15
mondrake#14 is wrong. The inherited properties inherit the attributes too, https://3v4l.org/XMdhhg. The problem was in mixing \Drupal::xxx calls with injected services and usage of traits. Maybe to be addressed but not here.
Comment #16
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 #17
mondrakeRebased
Comment #18
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 #19
mondrakerebased
Comment #20
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 #21
mondrakeComment #22
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 #23
mondrakerebased
Comment #24
mondrakeComment #25
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 #26
mondrakeComment #27
smustgrave commentedSorry if this is wrong review
But see we are adding 3 new dependencies or 1? Not sure what the PinnedDevDependencies are for or why it has 3 changes but other composer changes have 1. But summary doesn't mention that. Not sure if it needs a dependency evaluation being it's symfony
Comment #28
mondrakeUpdated IS. There is one new direct dependency, on symfony/expression-language. This indirectly requires symfony/cache itself, which requires symfony/cache-contracts itself.
Comment #29
mondrakeComment #30
smustgrave commentedSo tried to give a dependency evaluation based on the question I had in #core-development https://drupal.slack.com/archives/C079NQPQUEN/p1736797368660389
So looking at https://github.com/symfony/expression-language
I see they did releases pretty regularly (1-2 a month) but last one was v7.2.0 on Nov 29th, 2024. May be nothing just worth noting
I then went to symfony issue queue and searched for 'expression-language' and only found 7 https://github.com/symfony/symfony/issues?q=is%3Aissue%20state%3Aopen%20...
So believe this could be a safe addition.
Comment #31
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 #32
mondrakemerged with 11.x, no conflicts
Comment #33
catchThis shouldn't be used in functional tests, see #2066993: Use magic methods to sync container property to \Drupal::getContainer in functional tests. Didn't review the rest of the MR properly yet, that was the first thing I checked.
Comment #34
mondrake#33 is the reasoning that
\Drupal::serviceshould be used instead?If so, would changing
AutowirePropertyTraitfrom using$this->containerto using\Drupal::xxbe a correct solution?#2066993: Use magic methods to sync container property to \Drupal::getContainer in functional tests does not differentiate between kernel and functional, so is #33 valid for kernel ones too?
Comment #35
catch@mondrake the problem is that if you have a property at all, whether for the container itself or an individual service, it can get out of sync with the container in the site-under-test because that site is in a different php process.
If you use \Drupal:: directly in the test (after e.g. installing a module or something else that rebuilds the container), then it stays in sync because it's newly requested.
Ideally we would not be calling Drupal APIs from within phpunit in functional tests once we start making requests to the Drupal site (fine beforehand in setup etc.), but that ideal is very, very far from the current reality.
Comment #36
mondrakethat sounds like this should be won't fix, then.
The assumption of this issue is that properties referring to services in the container are ok to have, and so they can be autowired. But if there's a problem with that, this does not hold (and btw it also means there is a massive debt of usage to be cleaned up).