Skip to content

[PhpUnitBridge] CoverageListenerTrait update for PHPUnit 8.5/9.x #38054

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
Sep 4, 2020
Merged

[PhpUnitBridge] CoverageListenerTrait update for PHPUnit 8.5/9.x #38054

merged 1 commit into from
Sep 4, 2020

Conversation

sanmai
Copy link
Contributor

@sanmai sanmai commented Sep 4, 2020

Updated to use PHPUnit\Util\Annotation\Registry and related classes.

Fixes #37637

Q A
Branch? 4.4
Bug fix? yes
New feature? yes
Deprecations? no
Tickets Fix #37637
License MIT
Doc PR n/a
  • Fixes the issue under PHPUnit 8.5+
  • Tests TBD

If this is a sensible approach, I will try to add a test for this addition.

@sanmai sanmai changed the title CoverageListenerTrait update for PHPUnit 8.5/9.x [PhpUnitBridge] CoverageListenerTrait update for PHPUnit 8.5/9.x Sep 4, 2020
@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Sep 4, 2020
@sanmai
Copy link
Contributor Author

sanmai commented Sep 4, 2020

I think I addressed all comments. How should I test this change?

I see there's a test, but it assumes a currently used version of PHPUnit. Should this be enough?

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I see there's a test, but it assumes a currently used version of PHPUnit. Should this be enough?

Yes, we're not going to test with multiple versions of phpunit.

@fabpot
Copy link
Member

fabpot commented Sep 4, 2020

Thank you @sanmai.

@fabpot fabpot merged commit a19f91a into symfony:4.4 Sep 4, 2020
@derrabus
Copy link
Member

derrabus commented Sep 4, 2020

Thank you for working on this issue, @sanmai! 🚀

@sanmai sanmai deleted the pr/SymfonyTestsListenerTrait branch September 4, 2020 11:53
fabpot added a commit that referenced this pull request Sep 5, 2020
…rait for PHPUnit 9.3 (sanmai, derrabus)

This PR was merged into the 3.4 branch.

Discussion
----------

[PhpUnitBridge] Adjust output parsing of CoverageListenerTrait for PHPUnit 9.3

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #37637, #37564
| License       | MIT
| Doc PR        | N/A

This PR makes `CoverageListenerTrait` pass with PHPUnit 9.3. It contains a backport of #38054.

The problem that is addressed here is that a one of the fixture classes is now included in the output despite having a coverage of zero. The test now accepts both cases:

* The class is omitted from the output
* The class appears with 0.00% coverage.

Commits
-------

a3831dc [PhpUnitBridge] Adjust output parsing for PHPUnit 9.3.
99c98bd [PhpUnitBridge] CoverageListenerTrait update for PHPUnit 8.5/9.x
nicolas-grekas added a commit that referenced this pull request Sep 8, 2020
…rait (sanmai)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[PhpUnitBridge] Skip internal classes in CoverageListenerTrait

PHPUnit 9+ is picky about test covering, say, a `\RuntimeException`. Fails with the likes:

    "@Covers RuntimeException" is invalid

Judging by the commit sebastianbergmann/phpunit@e06850c this change is required for PHPUnit 9.1 and up. [Here's it being tested.](https://github.com/sebastianbergmann/code-unit/blob/60c6fb972b3262e423be3000e7b8c67cbc477580/tests/unit/ClassMethodUnitTest.php#L49-L54)

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Follow-up to #38054
| License       | MIT

Commits
-------

a0dedb9 [PhpUnitBridge] Skip internal classes in CoverageListenerTrait
This was referenced Sep 27, 2020
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