Skip to content

[DO NOT MERGE] Test codecov patch#992

Closed
sroet wants to merge 5 commits into
openpathsampling:masterfrom
sroet:test_codecov
Closed

[DO NOT MERGE] Test codecov patch#992
sroet wants to merge 5 commits into
openpathsampling:masterfrom
sroet:test_codecov

Conversation

@sroet

@sroet sroet commented Mar 12, 2021

Copy link
Copy Markdown
Member

This is just a PR to make sure #991 does not alter the patch codecov requirements

@codecov

codecov Bot commented Mar 12, 2021

Copy link
Copy Markdown

Codecov Report

Merging #992 (b900a3e) into master (a09f2b3) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #992      +/-   ##
==========================================
- Coverage   80.59%   80.58%   -0.01%     
==========================================
  Files         138      139       +1     
  Lines       14671    14674       +3     
==========================================
+ Hits        11824    11825       +1     
- Misses       2847     2849       +2     
Impacted Files Coverage Δ
openpathsampling/test.py 50.00% <50.00%> (ø)
openpathsampling/__init__.py 100.00% <100.00%> (ø)
openpathsampling/netcdfplus/cache.py 61.18% <0.00%> (-0.35%) ⬇️

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 a09f2b3...b900a3e. Read the comment docs.

Comment thread openpathsampling/test.py Outdated
@@ -0,0 +1 @@
print("codecov should scream about this")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
print("codecov should scream about this")
def fake_func():
print("codecov should scream about this")

You need to import the file somewhere -- either in the main package or in a test_test.py test module. Coverage doesn't know about files unless they're imported (for example, none of the OPS visualization stuff is even included in unit tests, because it has to be imported separately.)

Also, check the .coveragerc to make sure that we don't exclude anything with "test" in it from coverage.

@dwhswenson

Copy link
Copy Markdown
Member

Py27 fail is something flaky with PyEMMA in recent days. CodeCov non-fail is because coverage.py doesn't see the file you added.

@sroet

sroet commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

Also, check the .coveragerc to make sure that we don't exclude anything with "test" in it from coverage.

Just the openpathsampling/tests/ directory, I did test if the coverage showed up in the coverage report if I ran py.test --cov=openpathsampling. However, codecov is more clever than that xD

@dwhswenson

Copy link
Copy Markdown
Member

Behaves as expected! 😄

@dwhswenson

Copy link
Copy Markdown
Member

Oh, although I should clarify: rule for patch is that it must increase total coverage, not have 100% coverage.

@sroet

sroet commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

Oh, although I should clarify: rule for patch is that it must increase total coverage, not have 100% coverage.

Yeah, I was under the impression that you expected 100% coverage on any altered line. (unless, of course, explicitly untested with pragma: no cover)

@sroet

sroet commented Mar 12, 2021

Copy link
Copy Markdown
Member Author

With that, this PR can be closed 😄

@sroet sroet closed this Mar 12, 2021
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.

2 participants