Skip to content
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

Polyfill new phpunit 9.1 assertions #37320

Closed
nicolas-grekas opened this issue Jun 17, 2020 · 11 comments · Fixed by #37567
Closed

Polyfill new phpunit 9.1 assertions #37320

nicolas-grekas opened this issue Jun 17, 2020 · 11 comments · Fixed by #37567
Labels
Feature Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. PhpUnitBridge
Projects

Comments

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jun 17, 2020

We should polyfill the new assertions added to phpunit 9.1 in our bridge, so that they can be used on lower versions of php:
https://github.com/sebastianbergmann/phpunit/blob/9.1.5/ChangeLog-9.1.md#910---2020-04-03

See links in #32922 for inspiration.

See also https://github.com/sebastianbergmann/phpunit/pull/4210/files

Help wanted!

@nicolas-grekas nicolas-grekas added Feature Help wanted Issues and PRs which are looking for volunteers to complete them. labels Jun 17, 2020
@nicolas-grekas nicolas-grekas added this to Nicolas' in Help Wanted Jun 17, 2020
@nicolas-grekas nicolas-grekas added the Good first issue Ideal for your first contribution! (some Symfony experience may be required) label Jun 17, 2020
@phpfour
Copy link
Contributor

phpfour commented Jun 17, 2020

@nicolas-grekas I'm up for it ✋

@nicolas-grekas
Copy link
Member Author

Nice thanks, can't wait! (but no hurry either :) )

@phpfour
Copy link
Contributor

phpfour commented Jun 17, 2020

Nice thanks, can't wait! (but no hurry either :) )

You're welcome :) I'll get started on this and hopefully will have an update by the weekend 🤞🏼

@derrabus
Copy link
Member

@phpfour I started working on this topic a couple of weeks ago. If this commit is helpful for you, feel free to copy over as much as you like. 😃 derrabus@a05d504

@phpfour
Copy link
Contributor

phpfour commented Jun 20, 2020

hi @derrabus, thanks for your reference...that's quite a large commit you have...I am thinking whether it would be beneficial if I contribute to your repository to get it completed rather than doing on my own? WDYT?

@wouterj
Copy link
Member

wouterj commented Jun 20, 2020

You can fetch Derrabus' fork and then cherry-pick the commit into your branch and work from that:

git remote add derrabus https://github.com/derrabus/symfony.git
git fetch derrabus
git cherry-pick a05d50

Or you can download the raw diff of that commit and apply it locally:

wget https://github.com/derrabus/symfony/commit/a05d504971bbbd6a4e2ef27fbec47d53b61a4cdd.diff
git apply a05d504971bbbd6a4e2ef27fbec47d53b61a4cdd.diff

@derrabus
Copy link
Member

The commit is rather large because I've partly migrated the Symfony test suite to the polyfilled methods already. I don't think that this is necessary just yet. The interesting part is in PolyfillAssertTrait.php.

@nicolas-grekas
Copy link
Member Author

I confirm migrating the test suite is not desired unless some methods are deprecated. This will save merge conflicts.

@phpfour
Copy link
Contributor

phpfour commented Jun 25, 2020

Thanks guys for the direction, hoping to back soon with some update.

@derrabus
Copy link
Member

@phpfour Is there anything I can do to support you?

@phpfour
Copy link
Contributor

phpfour commented Jul 13, 2020

I've just pushed the commit and created the PR, sorry this has been sitting idle for a few days 😔

@nicolas-grekas @derrabus would request your review 🙏

@fabpot fabpot closed this as completed in 488e2c9 Jul 15, 2020
fabpot added a commit that referenced this issue Aug 28, 2020
This PR was merged into the 4.4 branch.

Discussion
----------

[PhpUnitBridge] Polyfill new phpunit 9.1 assertions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #37320
| License       | MIT
| Doc PR        | -

Backport of #37567, follows the discussion on #37960.

Commits
-------

d945b88 [PhpUnitBridge] Polyfill new phpunit 9.1 assertions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. PhpUnitBridge
Projects
Help Wanted
Nicolas'
Development

Successfully merging a pull request may close this issue.

5 participants