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

Fix #3697 Respect @coversNothing at coverage driver start #3699

Merged
merged 2 commits into from May 26, 2019

Conversation

andrewnicols
Copy link
Contributor

As mentioned in #3697, the existing @coversNothing annotation works as expected for classes, but for methods the coverage data is still generated and it is later filtered out.

This change adds support for @coversNothing in the test method annotation, and attempts to address inverted assertions in the unit testing of the existing functionality.

@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

Merging #3699 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3699      +/-   ##
============================================
+ Coverage     82.79%   82.81%   +0.01%     
- Complexity     3755     3756       +1     
============================================
  Files           147      147              
  Lines          9911     9913       +2     
============================================
+ Hits           8206     8209       +3     
+ Misses         1705     1704       -1
Impacted Files Coverage Δ Complexity Δ
src/Util/Test.php 86.97% <100%> (+0.22%) 222 <0> (+1) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41a5260...bd8248b. Read the comment docs.

@andrewnicols
Copy link
Contributor Author

Note: I see that in shouldCoversAnnotationBeUsed, the @coversNothing annotation has a higher precedence than @covers on the same method. There's nothing in the documentation to say which shoudl have higher precedent and unit tests pass with it either way.

I've currently modified it for the 'new' behaviour, but I'll swap it tomorrow (late here in Australia) unless I hear otherwise.

@andrewnicols
Copy link
Contributor Author

Is this eligible for backport to 7.5? If so, do I need to prepare a patch and raise a separate pull request?

Thanks

@sebastianbergmann
Copy link
Owner

If you send a pull request for 7.5 then I will consider it, yes. I will, however, not invest time in backporting it myself. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants