-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Read PHPUnit configuration to check for listeners too #31649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
$configuration = Configuration::getInstance($this->arguments['configuration']); | ||
} | ||
foreach ($configuration->getListenerConfiguration() as $registeredListener) { | ||
if (is_subclass_of($registeredListener['class'], SymfonyTestsListenerForV5::class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ForV6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or Symfony\Bridge\PhpUnit\SymfonyTestsListener
in both classes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that was a mistake. Interestingly whilst debugging this locally doing
is_subclass_of($registeredListener['class'], SymfonyTestsListener::class)
was not working even though
$registeredListener['class'] was Symfony\Bridge\PhpUnit\SymfonyTestsListener
?!?!?
dce0147
to
4226b90
Compare
23e9dba
to
80cd0c3
Compare
Should target 3.4 since it's a bug fix? |
@greg0ire maybe you'd have some time to take over this PR? It'd be great :) |
I'll give it a look :) |
@alexpott https://www.drupal.org/project/drupal/issues/303934 gives me a 404 |
The listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes symfony#31649
The listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes symfony#31649
The bridge listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes symfony#31649
The bridge listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes symfony#31649
closing in favor of #32903 |
The bridge listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes symfony#31649
This PR was merged into the 3.4 branch. Discussion ---------- [PHPUnit Bridge] Avoid registering listener twice The listener can be registered via configuration by the user. In that case, we do not want to add it again to the list of listeners. Closes #31649 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- b190536 Check phpunit configuration for listeners
Trying to use simple-phpunit to get around phpunit dependency fun and support many versions of PHP in Drupal. See https://www.drupal.org/project/drupal/issues/303934. We're running into an issue that we register the Symfony event listener in our phpunit.xml file. We need to do this because we wrap your listener and intercept some deprecations. Without this fix the symfony tests listener gets registered twice.