-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
PhpUnit-Bridge can't deal with @runTestsInSeparateProcesses #23003
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
Comments
Please submit a PR. The limitation should be removed! |
Splitting this up because it's complex and might require an upstream change for phpunit. First, I'm working on marking isolated tests with @expectedDeprecation as risky. Progress here: master...paul-m:mark-isolated-tests-risky Not ready for a PR because I don't know enough about the symfony phpunit script to understand how to make it pass without hanging. |
…ethod (nicolas-grekas) This PR was merged into the 3.3 branch. Discussion ---------- [Bridge\PhpUnit] Fix infinite loop when running isolated method | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - May unlock #23003 (ping @paul-m) And required to make #23711 pass. Commits ------- e07c2f1 [Bridge\PhpUnit] Fix infinite loop when running isolated method
Thanks. I'm a bit of a newbie to the Symfony way of doing things, so I wasn't sure if it was me missing a step or something. It looks like a ^3.3 fix. I'm coming at this from the Drupal 8 world, where we're stuck at phpunit-bridge 3.2.*. See https://www.drupal.org/node/2882826 |
@paul-m about https://www.drupal.org/node/2882826, the bridge should work with a recent enough version of PHPunit 4.8.* |
… processes (paul-m) This PR was merged into the 3.3 branch. Discussion ---------- [Bridge\PhpUnit] Handle deprecations triggered in separate processes | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23003, #16726 | License | MIT | Doc PR | - As reported in #23003, deprecations triggered in process-isolated test cases are not gathered. This caught us already: HttpFoundation is still using deprecated code paths, but we missed them because of that issue with the bridge. Here is the fixed output:  Credits to @paul-m for working on the issue first. Commits ------- ca0fedd [BrowserKit] Handle deprecations triggered in insulated requests ff379ef [Bridge\PhpUnit] Handle deprecations triggered in separate processes
I'm working on Drupal 8 core. Drupal has adopted symfony/phpunit-bridge as a way to catch deprecations. The codebase has a lot of
@trigger_error('', E_USER_DEPRECATED)
type code in it.The problem is that a lot of our PHPUnit-based tests are functional and need to run in isolated processes.
There are two problems:
We can't test
@expectedException
in isolated tests. They always fail.Errors triggered by
@trigger_error()
never surface in isolated tests.There are two reasons for this:
For 1 above,
SymfonyTestsListener
sets the expectation for the test run before the isolation occurs. Whatever deprecation error is emitted, it happens in another process. When it's done, the 'host' listener then finds no deprecation message so it fails the test.For 2 above, the problem is that a)
Symfony\Bridge\PhpUnit\DeprecationErrorHandler
is never registered in isolation, due to the way the isolation occurs. Also, once you solve the problem of registering the error handler, it fails the test by callingexit(1)
. As it turns out, PHPUnit ignores the return value of the child process, so we never get the fail.I've written a patch for Drupal that basically overcomes these limitations, but it's effectively a fork of symfony/phpunit-bridge, and it'd be much better not to do that.
You can see the patch here: https://www.drupal.org/node/2870194#comment-12111028
Is there a reason symfony/phpunit-bridge doesn't support isolated tests? I couldn't find a testing policy document to see if the limitation was arbitrary or intentional.
The text was updated successfully, but these errors were encountered: