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

CodeCov Tweaks #9650

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Conversation

PrivatePuffin
Copy link
Contributor

@PrivatePuffin PrivatePuffin commented Nov 28, 2019

Codecov has been hit-or-mis for a time now

It seems to have 2 flaws:

  • Tests are actually taken into account when comparing coverage
  • There is a random variance in coverage leading in the 0.0-0.4 range.

Motivation, Context and Description

  • As far as I know there is no plan to write tests to cover tests, so this folder could just as well be excluded.
  • Precision has been limited to whole percents only, but will round to nearest. This means 0.0-0.49 will round to zero (no change) and 0.51 will round to 1%.

This might also lead to a more realistic and stable codecov report.

How Has This Been Tested?

Automated tests for as far as thats even possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@PrivatePuffin PrivatePuffin force-pushed the Remove-Testfolder-CodeCov branch from e055c40 to 7a1673e Compare November 28, 2019 21:50
@PrivatePuffin PrivatePuffin changed the title Remove zfstests folder from CodeCov Remove zfstests/tests folder from CodeCov Nov 28, 2019
@codecov
Copy link

codecov bot commented Nov 29, 2019

Codecov Report

Merging #9650 into master will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #9650    +/-   ##
========================================
- Coverage      79%      79%   -<1%     
========================================
  Files         418      418            
  Lines      123544   123531    -13     
========================================
- Hits        97889    97702   -187     
- Misses      25655    25829   +174
Flag Coverage Δ
#kernel 80% <ø> (ø) ⬇️
#user 67% <ø> (ø) ⬇️

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 a7c3588...bf5b5ad. Read the comment docs.

@PrivatePuffin PrivatePuffin marked this pull request as ready for review November 29, 2019 07:25
- Remove zfstests folder from codecov
- Limit precision to whole percents

Signed-off-by: Kjeld Schouten-Lebbing <[email protected]>
@PrivatePuffin PrivatePuffin force-pushed the Remove-Testfolder-CodeCov branch from 7a1673e to bf5b5ad Compare November 30, 2019 20:35
@PrivatePuffin PrivatePuffin changed the title Remove zfstests/tests folder from CodeCov CodeCov Tweaks Nov 30, 2019
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 1, 2019
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks! This appears to work as advertised.

Limiting the precision to whole percentage changes if a good idea, I hope should help eliminate some of the reported variation. Though due to the semi-random nature of several test cases (ztest) some variability is unavoidable.

From the codecov summary it wasn't clear to me that "tests/zfs-tests" was really excluded from the report. But the yaml looks right to me based on the documentation, so let's give it a try.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 3, 2019
@PrivatePuffin
Copy link
Contributor Author

@behlendorf Thank for your review.
I triple checked it also, it looks like codecov doesn't really accept directory ignore in a PR... But the documentation is clear indeed.

@behlendorf behlendorf merged commit 7af7286 into openzfs:master Dec 3, 2019
@PrivatePuffin PrivatePuffin mentioned this pull request Dec 4, 2019
12 tasks
@PrivatePuffin PrivatePuffin deleted the Remove-Testfolder-CodeCov branch December 19, 2019 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants