Skip to content

[PhpUnitBridge] Bump PHPUnit 7+8 #32059

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
Jun 16, 2019
Merged

[PhpUnitBridge] Bump PHPUnit 7+8 #32059

merged 1 commit into from
Jun 16, 2019

Conversation

ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Jun 15, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

@Taluu
Copy link
Contributor

Taluu commented Jun 15, 2019

Shouldn't this target 2.8 ?

@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 15, 2019

i figured master because of

That should be submitted on master: we shouldn't change the version on a patch release. (#29439 (comment))

so this changes the default version; for 2.8 users can leverage SYMFONY_PHPUNIT_VERSION=8.2

@nicolas-grekas nicolas-grekas added this to the next milestone Jun 15, 2019
@nicolas-grekas
Copy link
Member

We should maybe remember to bump before the 4.4.0 if a new version is released meanwhile.

@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 15, 2019

We should definitively not forget to bump to v8 :)

Im wondering about the purpose of hardcoding the upper bound constraint for PHPUnit :/ it blocks the require phpunit/phpunit case and let Composer decide the stable version 🤔 Not doing so,
would make tools like https://github.com/jakzal/toolbox (installing simple-phpunit on a Docker image) always make simple-phpunit install the latest stable phpunit by default.

AFAIK it's not possible to do e.g. SYMFONY_PHPUNIT_VERSION=@stable due some hardcoded version checks, which maybe arent needed as php 7 (?) (#31948)

@nicolas-grekas
Copy link
Member

it's not possible to do e.g. SYMFONY_PHPUNIT_VERSION=@stable due some hardcoded version checks

did you try relaxing those? now that we use composer to download phpunit, maybe it could be made to work?

@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 16, 2019

@nicolas-grekas IIUC ideally the default for sf4.4 as of php7.2 would be @stable right? To avoid having to invoke SYMFONY_PHPUNIT_VERSION=@stable simple-phpunit all the time.

And im not sure doing it as of php7.2, if we dont need the version upfront i tend to prefer not hardcoding any upper limits 🤔

@nicolas-grekas
Copy link
Member

I'm not sure: what about BC when a new major of phpunit is released?

@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 16, 2019

Ultimately let the user decide its fixed version using SYMFONY_PHPUNIT_VERSION=x.y?

Having @stable by default seems sensible :/

@nicolas-grekas
Copy link
Member

@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 16, 2019

I see, let's leave as is. Explicit version control is nice anyway. Basically i was looking for a cheap solution to let https://hub.docker.com/r/jakzal/phpqa/ install both a wrapped v7 and v8 by default. But it's distributed from 7.1 to 7.3, so v8 should be excluded for 7.1

The toolbox also installs v7 + v8 natively under different binaries: https://github.com/jakzal/toolbox/blob/master/resources/tools.json#L618-L641

This works because it can exclude 7.1 tag-based per tool, but not per install command for a single tool. As simple-phpunit is a version manager it doesnt make sense to have e.g. simple-phpunit + simple-phpunit-7.

Therefor im leaning to exclude the whole simple-phpunit binary in the toolbox, on 7.1. (cc @jakzal), then it can be installed using SYMFONY_PHPUNIT_VERSION=8.2 simple-phpinit install without relying on any defaults in SF.

@ro0NL ro0NL changed the title [PhpUnitBridge] Bump to PHPUnit 8 [PhpUnitBridge] Bump PHPUnit 7+8 Jun 16, 2019
@ro0NL
Copy link
Contributor Author

ro0NL commented Jun 16, 2019

I figured PHPunit 7 can also be bumped.

@fabpot
Copy link
Member

fabpot commented Jun 16, 2019

Thank you @ro0NL.

@fabpot fabpot merged commit 5491d53 into symfony:4.4 Jun 16, 2019
fabpot added a commit that referenced this pull request Jun 16, 2019
This PR was squashed before being merged into the 4.4 branch (closes #32059).

Discussion
----------

[PhpUnitBridge] Bump PHPUnit 7+8

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.

Additionally (see https://symfony.com/roadmap):
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against branch 4.4.
 - Legacy code removals go to the master branch.
-->

Commits
-------

5491d53 [PhpUnitBridge] Bump PHPUnit 7+8
@ro0NL ro0NL deleted the patch-3 branch June 16, 2019 12:04
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
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