Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Make tests support phpunit 8 #32842

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

Merged
merged 1 commit into from
Jul 31, 2019
Merged

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jul 31, 2019

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 -

Having void on setUp/tearDown is really painful /cc @sebastianbergmann FYI

This works around the issue by adding a new trait to the phpunit-bridge : ForwardCompatTestTrait.
Another reason why the bridge is so useful...

With this change, we should have the same codebase be able to run under phpunit 4.8 (used with PHP5.5) up to phpunit 8.2 (used with PHP7.4).

🤞 🤕

One more step towards PHP 7.4 support.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jul 31, 2019
@nicolas-grekas nicolas-grekas force-pushed the phpunit8 branch 2 times, most recently from 88e284d to 144abeb Compare July 31, 2019 21:29
nicolas-grekas added a commit that referenced this pull request Jul 31, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Make tests support phpunit 8

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

Having `void` on setUp/tearDown is **really** painful /cc @sebastianbergmann FYI

This works around the issue by adding a new trait to the phpunit-bridge: `SetUpTearDownTrait`.
Another reason why the bridge is so useful...

With this change, we should have the same codebase be able to run under phpunit 4.8 (used with PHP5.5) up to phpunit 8.2 (used with PHP7.4).

:crossed_fingers: :face_with_head_bandage:

One more step towards PHP 7.4 support.

Commits
-------

81af97f Make tests support phpunit 8
@nicolas-grekas nicolas-grekas merged commit 81af97f into symfony:3.4 Jul 31, 2019
@nicolas-grekas nicolas-grekas deleted the phpunit8 branch July 31, 2019 22:40
@nicolas-grekas nicolas-grekas mentioned this pull request Jul 31, 2019
23 tasks
@Tobion
Copy link
Contributor

Tobion commented Aug 1, 2019

This looks like a huge source of conflicts. Are we sure we want to introduce that? The alternatives would be to not support phpunit 8 or only do these changes in 4.4 and up.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Aug 1, 2019

This is merged up to master now so that conflicts are resolved once for all. We have to support phpunit 8 because that's the only way to PHP 7.4, which we want to support on branch 3.4...

@mlocati
Copy link
Contributor

mlocati commented Aug 6, 2019

The alternatives would be to not support phpunit 8

This is a recurring problem: every PHPUnit version supports just a small range of PHP versions.

@nicolas-grekas
Copy link
Member Author

@mlocati we made some progress since this PR, see #32922

@@ -32,6 +32,11 @@ abstract class KernelTestCase extends TestCase
*/
protected static $kernel;

private function doTearDown()
Copy link
Contributor

@teohhanhui teohhanhui Aug 29, 2019

Choose a reason for hiding this comment

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

Maybe it doesn't bother too many people, but the visibility of this method has been changed from v4.3.3:

protected function doTearDown(): void

It broke our builds in API Platform.

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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.