Fix KernelTestCase compatibility for PhpUnit 8 #30084
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexander-schranz
force-pushed
the
feature/phpunit-8-compatibility
branch
2 times, most recently
from
203cf61
to
ddf2a25
Compare
alexander-schranz
force-pushed
the
feature/phpunit-8-compatibility
branch
from
ddf2a25
to
e216c0d
Compare
alexander-schranz
changed the title
alexander-schranz
force-pushed
the
feature/phpunit-8-compatibility
branch
5 times, most recently
from
9850f7d
to
254cd15
Compare
alexander-schranz
changed the title
alexander-schranz
changed the title
alexander-schranz
changed the title
alexander-schranz
force-pushed
the
feature/phpunit-8-compatibility
branch
2 times, most recently
from
d26f2bf
to
ff5de16
Compare
|
Instead of doing this change, would adding the |
alexander-schranz
force-pushed
the
feature/phpunit-8-compatibility
branch
from
ff5de16
to
83a56a0
Compare
|
@nicolas-grekas sounds like a good idea! Did change it. |
| @@ -208,6 +208,8 @@ protected static function createKernel(array $options = []) | |||
| } | |||
|
|
|||
| /** | |||
| * @after | |||
| * | |||
| * Shuts the kernel down if it was used in the test. | |||
| */ | |||
| protected static function ensureKernelShutdown() | |||
There was a problem hiding this comment.
could you please confirm it works even if this method is protected?
There was a problem hiding this comment.
Can confirm it was called in a simple test extending the KernelTestCase.
nicolas-grekas
approved these changes
Feb 7, 2019
|
@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) |
all is fine then, great! |
|
@nicolas-grekas and the |
nicolas-grekas
added a commit
that referenced
this pull request
Feb 8, 2019
…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
|
This is a huge BC break and must be reverted. It's common practice to overwrite |
nicolas-grekas
added a commit
to nicolas-grekas/symfony
that referenced
this pull request
Feb 9, 2019
|
:( thanks for the review. |
fabpot
added a commit
that referenced
this pull request
Feb 12, 2019
…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)
fabpot
added a commit
that referenced
this pull request
Mar 9, 2019
This PR was merged into the 3.4 branch. Discussion ---------- compatibility with phpunit8 This 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 | none Commits ------- 5ef254f compatibility with phpunit8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.