[PhpUnitBridge] do not register the test listener twice #21787
Conversation
|
As far as I remember correctly, the listener must be registered as soon as possible so that namespaced function injection work all the time (for Clock/DnsMock). If the double registration doesn't have any adverse side effect, I'd prefer be careful here, that can be tricky :) |
|
I noticed this behaviour when working on #21786 ( |
|
What we could do is creating the object before calling the parent method. This should trigger the registration of our |
If the listener is already configured through the PHPUnit config, there is no need to also enable it explicitly in the test runner.
5a27b3c to
cee435f
Compare
|
So, the listener will now still be built before calling the parent method. However, it will only be actually registered if that did not happen through the PHPUnit config. |
|
Thank you @xabbuh. |
…abbuh) This PR was merged into the 2.8 branch. Discussion ---------- [PhpUnitBridge] do not register the test listener twice | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | If the listener is already configured through the PHPUnit config, there is no need to also enable it explicitly in the test runner. Commits ------- cee435f do not register the test listener twice
|
|
||
| return parent::handleConfiguration($arguments); | ||
| if (!array_filter($arguments['listeners'], function ($listener) { return $listener instanceof SymfonyTestsListener; })) { |
There was a problem hiding this comment.
no need to filter the whole array. We should stop as soon as we find a SymfonyTestsListener even if it is the first one.
There was a problem hiding this comment.
you're right - yet this is a non issue in practice, isn't it? this array is usually almost empty
If the listener is already configured through the PHPUnit config, there
is no need to also enable it explicitly in the test runner.