Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Fix KernelTestCase compatibility for PhpUnit 8#30084
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix KernelTestCase compatibility for PhpUnit 8#30084
Uh oh!
There was an error while loading.Please reload this page.
Conversation
203cf61 toddf2a25Compareddf2a25 toe216c0dCompare9850f7d to254cd15CompareUh oh!
There was an error while loading.Please reload this page.
d26f2bf toff5de16Comparenicolas-grekas commentedFeb 7, 2019
Instead of doing this change, would adding the |
ff5de16 to83a56a0Comparealexander-schranz commentedFeb 7, 2019
@nicolas-grekas sounds like a good idea! Did change it. |
| * | ||
| * Shuts the kernel down if it was used in the test. | ||
| */ | ||
| protectedstaticfunctionensureKernelShutdown() |
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.
could you please confirm it works even if this method is protected?
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.
Can confirm it was called in a simple test extending the KernelTestCase.
nicolas-grekas left a comment
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.
There is potential BC break, if ppl callparent::tearDown(). We could work around by implementing__call(). Not sure it's worth it, wdyt?
alexander-schranz commentedFeb 7, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas tried it but __call seems never be called when parent::tearDown() is called it seems directly calling the PhpUnit teardown. (https://wtools.io/php-sandbox/6y) |
nicolas-grekas commentedFeb 7, 2019
all is fine then, great! |
alexander-schranz commentedFeb 7, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas and the |
…schranz)This PR was merged into the 3.4 branch.Discussion----------Fix KernelTestCase compatibility for PhpUnit 8| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? |no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#30071| License | MIT| Doc PR |As the PhpUnit 8 Testcase has different return types as PhpUnit 7 there need to be 2 different classes to support both PhpUnit 8 and PhpUnit 7. With a class alias we can then change which version is used based on the PhpUnit Version constant. The fix is a little bit hacky but to support different major versions I see no other way.Not sure as we can't upgrade symfony/symfony to PhpUnit 8 how we can create a TestCase for this change.Commits-------83a56a0 Fix phpunit 8 compatibility
Tobion commentedFeb 8, 2019
This is a huge BC break and must be reverted. It's common practice to overwrite |
nicolas-grekas commentedFeb 9, 2019
:( thanks for the review. |
…las-grekas)This PR was squashed before being merged into the 3.4 branch (closes#30124).Discussion----------Fix KernelTestCase compatibility for PhpUnit 8 (bis)| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Follow up of#30084 and@Tobion's comment there.Fabbot failure is a false-positive.Commits-------1077df6 Fix KernelTestCase compatibility for PhpUnit 8 (bis)
This PR was merged into the 3.4 branch.Discussion----------compatibility with phpunit8This basically adds the same phpunit8 compatibility layer added in#30084 (but for other test classes)See also discussion in#30071| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#30071| License | MIT| Doc PR | noneCommits-------5ef254f compatibility with phpunit8
Uh oh!
There was an error while loading.Please reload this page.
As the PhpUnit 8 Testcase has different return types as PhpUnit 7 there need to be 2 different classes to support both PhpUnit 8 and PhpUnit 7. With a class alias we can then change which version is used based on the PhpUnit Version constant. The fix is a little bit hacky but to support different major versions I see no other way.
Not sure as we can't upgrade symfony/symfony to PhpUnit 8 how we can create a TestCase for this change.