10000 Read PHPUnit configuration to check for listeners too by alexpott · Pull Request #31649 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

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

Closed
wants to merge 3 commits into from

Conversation

alexpott
Copy link
Contributor
Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? todo
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

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.

$configuration = Configuration::getInstance($this->arguments['configuration']);
}
foreach ($configuration->getListenerConfiguration() as $registeredListener) {
if (is_subclass_of($registeredListener['class'], SymfonyTestsListenerForV5::class)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ForV6?

Copy link
Member

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?

Copy link
Contributor Author
@alexpott alexpott May 28, 2019

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 ?!?!?

@alexpott alexpott force-pushed the phpunit-listeners branch from dce0147 to 4226b90 Compare May 28, 2019 15:09
@alexpott alexpott force-pushed the phpunit-listeners branch from 23e9dba to 80cd0c3 Compare May 28, 2019 15:21
@nicolas-grekas nicolas-grekas modified the milestones: 3.4, next May 29, 2019
@nicolas-grekas
Copy link
Member

Should target 3.4 since it's a bug fix?

@nicolas-grekas nicolas-grekas modified the milestones: next, 3.4 May 29, 2019
@nicolas-grekas
Copy link
Member

@greg0ire maybe you'd have some time to take over this PR? It'd be great :)

@greg0ire
Copy link
Contributor

I'll give it a look :)

@greg0ire
Copy link
Contributor
greg0ire commented Aug 3, 2019

greg0ire pushed a commit to greg0ire/symfony that referenced this pull request Aug 3, 2019
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
greg0ire pushed a commit to greg0ire/symfony that referenced this pull request Aug 3, 2019
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
greg0ire pushed a commit to greg0ire/symfony that referenced this pull request Aug 3, 2019
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
greg0ire pushed a commit to greg0ire/symfony that referenced this pull request Aug 8, 2019
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
@fabpot
Copy link
Member
fabpot commented Aug 9, 2019

closing in favor of #32903

@fabpot fabpot closed this Aug 9, 2019
greg0ire pushed a commit to greg0ire/symfony that referenced this pull request Sep 2, 2019
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
nicolas-grekas added a commit that referenced this pull request Sep 2, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0