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

Check DISABLE_AMEND_COVERAGE_FROM_SRC env var #208

Merged

Conversation

fingolfin
Copy link
Member

If the environment variable DISABLE_AMEND_COVERAGE_FROM_SRC is set to yes, do not call amend_coverage_from_src!. This is an optional mitigation for issue #187, and in general makes it easier to test the impact of amending the coverage data.

@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch 3 times, most recently from 9ecaaff to 6cc4d61 Compare March 6, 2019 17:21
@codecov-io
Copy link

codecov-io commented Mar 6, 2019

Codecov Report

Merging #208 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
+ Coverage   80.18%   80.23%   +0.05%     
==========================================
  Files           6        6              
  Lines         333      334       +1     
==========================================
+ Hits          267      268       +1     
  Misses         66       66
Impacted Files Coverage Δ
src/Coverage.jl 86.91% <100%> (+0.12%) ⬆️

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 6c31888...bf6a68b. Read the comment docs.

@ararslan
Copy link
Member

ararslan commented Mar 6, 2019

Seems okay, but could you add some documentation in the README or whatever for how to properly use this and what the consequences of doing so are?

@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch from 6cc4d61 to 019121b Compare March 7, 2019 09:03
@fingolfin fingolfin mentioned this pull request Mar 7, 2019
@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch 4 times, most recently from 60c973e to 7457973 Compare March 7, 2019 13:49
@fingolfin
Copy link
Member Author

Enabled this for testing, effect can be see on e.g. https://codecov.io/gh/JuliaCI/Coverage.jl/src/60c973e831cfa9f9e9da48ef07d3b8f35fbbd770/src/lcov.jl where function writefile is not marked as code at all; it really is a dead function.

@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch from 7457973 to f5c0b8f Compare March 7, 2019 13:57
@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch 2 times, most recently from 1d8b7cd to 817c8b6 Compare March 7, 2019 14:47
Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

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

Incredibly thorough and well-explained documentation for this, very nice work!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
If the environment variable DISABLE_AMEND_COVERAGE_FROM_SRC is set to `yes`,
do not call `amend_coverage_from_src!`. This is an optional mitigation for
issue JuliaCI#187, and in general makes it easier to test the impact of amending the
coverage data.
@fingolfin fingolfin force-pushed the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch from 4611663 to bf6a68b Compare March 7, 2019 23:04
@fingolfin fingolfin merged commit 944e8a0 into JuliaCI:master Mar 8, 2019
@fingolfin fingolfin deleted the mh/DISABLE_AMEND_COVERAGE_FROM_SRC branch March 8, 2019 08:12
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.

3 participants