ci: revisit coverage reporting

[blueyed]

This should bring coverage back that got missing with 9dcd6f2.

Continuation of #4839 / #4846.

[codecov]

Codecov Report

Merging #4865 into master will increase coverage by 1.23%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4865      +/-   ##
==========================================
+ Coverage   94.48%   95.71%   +1.23%     
==========================================
  Files         113      113              
  Lines       25173    25172       -1     
  Branches     2499     2498       -1     
==========================================
+ Hits        23784    24093     +309     
+ Misses       1059      764     -295     
+ Partials      330      315      -15

Impacted Files	Coverage Δ
testing/test_assertrewrite.py	83.53% <100%> (-0.03%)	⬇️
testing/test_terminal.py	99.81% <0%> (+0.56%)	⬆️
src/_pytest/pytester.py	87.05% <0%> (+1.13%)	⬆️
src/_pytest/cacheprovider.py	97.16% <0%> (+1.41%)	⬆️
src/_pytest/terminal.py	91.74% <0%> (+2.64%)	⬆️
src/_pytest/doctest.py	96.37% <0%> (+2.89%)	⬆️
src/_pytest/assertion/util.py	97.63% <0%> (+4.33%)	⬆️
src/_pytest/unittest.py	94.17% <0%> (+6.87%)	⬆️
src/_pytest/debugging.py	77.98% <0%> (+17.61%)	⬆️
testing/test_pdb.py	99.07% <0%> (+50.92%)	⬆️

---

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 bd2c9be...8481e43. Read the comment docs.

[blueyed]

For reference: https://codecov.io/gh/pytest-dev/pytest/compare/ee626743...revisit-cov-new/changes to see what is still missing (ee62674 is before the coverage dropped).

[blueyed]

ok with 6942a2a: https://travis-ci.org/pytest-dev/pytest/builds/500432749#L320:

src/_pytest/debugging.py 159 28 46 7 78% 106-109, 112-126, 131-132, 140-146, 189-191, 194, 258, 87->89, 91->101, 165->170, 188->189, 193->194, 250->252, 257->258

And bad again when using "include" again:

src/_pytest/debugging.py 159 84 46 5 42% 46, 83-152, 157-159, 173-174, 180-181, 185-198, 216-219, 234, 240-243, 249-252, 258, 45->46, 165->170, 215->216, 231->234, 257->258

So apparently #4841 broke it.

[blueyed]

Still good now with #4867 included here:

src/_pytest/debugging.py 159 28 46 7 78%

[blueyed]

@nicoddemus
While this is coming to an end finally: what's your opinion on keeping the jobs minimal like here?
I.e. due to the coverage report(s) we can see that with only those jobs everything is covered for.

(I plan to push a test commit next to run all jobs from master with coverage to see what the difference would be.)

[nicoddemus]

While this is coming to an end finally: what's your opinion on keeping the jobs minimal like here?

I think keeping them to a minimal is good, coverage does increase the job runtime significantly.

[blueyed]

I think keeping them to a minimal is good, coverage does increase the job runtime significantly.

I've meant to keep the actual jobs at a minimum, i.e. run them all with coverage, but only the necessary ones (to have full/max coverage) - very much like it is here currently.

[nicoddemus]

I've meant to keep the actual jobs at a minimum, i.e. run them all with coverage, but only the necessary ones (to have full/max coverage) - very much like it is here currently.

As long as we run over all Python versions we are supposed to support, the relevant variations with pluggy and xdist in both Python 2 and 3, I think it is perfectly fine to merge a few jobs for example numpy+nobyte for example.

(The last time I took a look I missed pluggy and py36 on azure, not sure if it was an interim commit or what btw).

[blueyed]

So this is back on track according to https://codecov.io/gh/pytest-dev/pytest/compare/ee626743...38ce5fa/changes (comparing ee62674 to 38ce5fa):

• Changes in src/_pytest/cacheprovider.py is fixed in cacheprovider: _ensure_supporting_files: remove unused branches #4872
• Changes in testing/test_parseopt.py: improved

TODO:

• Changes in src/_pytest/capture.py fails to display in the UI, but appears to be missed coverage in _attempt_to_close_capture_file - will add a todo comment to make it show up in the diff.

[blueyed]

As long as we run over all Python versions we are supposed to support, the relevant variations with pluggy and xdist in both Python 2 and 3, I think it is perfectly fine to merge a few jobs for example numpy+nobyte for example.

Also across platforms?

Since Azure runs 10 jobs in parallel I would focus on putting most of them there.

[nicoddemus]

Also across platforms?

Yes, I think that's important; we have often caught bugs this way.

[blueyed]

Here are the changes for full coverage: https://codecov.io/gh/pytest-dev/pytest/compare/38ce5fa...eb54cd4/changes.
(comparing last two commits, before I've just pushed again)

Two are about pypy, which we do not want in general - for this it might make sense to have single job with coverage only targeting those.

The other diffs should be fixed by what I've just pushed, i.e. there is no (coverage) benefit from adding more jobs.

[blueyed]

@nicoddemus

Also across platforms?

Yes, I think that's important; we have often caught bugs this way.

How so? I would assume that py35 fails both on Windows and Linux if it is a non-platform issue, and if it is a platform related bug it would fail on either of them (maybe with a different Python then).

But I agree that it is good to have a good/clear test matrix in general - I think my motivation is mostly based on if doing multiple PRs in parallel and having to wait on them. But to be honest, it does not really make sense to run py37 with/without xdist on windows/linux for example.

[nicoddemus]

How so? I would assume that py35 fails both on Windows and Linux if it is a non-platform issue, and if it is a platform related bug it would fail on either of them (maybe with a different Python then).

Hmm yes, that's why it is important to run py35 in both Windows and Linux. Not sure, I think we are agreeing in the end. 😁

[blueyed]

Not really.. what I've meant that the platform related bug would not be tight to a Python version, and that a single failure is OK/enough - i.e. a py35 issue does not have to fail on Travis and Azure.
But I am OK with it, and it helps to trigger/verify e.g. a os.path related issue in a certain Python version.

Will have to bring the jobs back - currently wondering why the codecov status is missing.

[blueyed]

@nicoddemus
Please review.
Also compare the resulting diff matrix on Travis and Azure between this and master carefully.

I think it is good, and would like to squash and rebase it after #4878, and get this soon into features then also.

[blueyed]

Squashed.

[blueyed]

Travis is very similar: T=Travis, TC=Travis with coverage

TOXENV	master	new
py27-pexpect,py27-trial	T	TC
py36-xdist	T	T
linting,docs,doctesting	T	C
py27-xdist (osx)	T	TC
py37-xdist (osx)	T	T
py27	TC	X
py37	T	T
pypy-xdist	T	T
pypy3-xdist	T	T
py34-xdist	T	T
py35-xdist	T	T
py37-lsof-numpy-xdist	TC	TC
py27-nobyte-numpy-xdist	TC	T
py27-pluggymaster-xdist	T	T
py37-pexpect,py37-trial	T	TC
py37-pluggymaster-xdist	T	T
py37-freeze	T	T

Full table with Azure, A=Azure, AC=Azure with coverage:

TOXENV	master	new
py27	TC, AC	T, A
py27-nobyte-lsof-numpy		AC
py27-nobyte-numpy-xdist	TC, AC	T
py27-pexpect,py27-trial	T	TC
py27-pluggymaster-xdist	T, A	T, AC
py27-trial	AC
py27-xdist (osx)	T	TC
py34	AC
py34-xdist	T	T, AC
py35	AC
py35-xdist	T	T, AC
py36	AC
py36-xdist	T	T
py36-xdist		AC
py37	AC
py37	T	T, AC
py37-freeze	T	T
py37-linting,docs,doctesting	T	C, A
py37-lsof-numpy-xdist	TC	TC
py37-pexpect,py37-trial	T	TC
py37-pluggymaster-xdist	T, A	T, A
py37-trial/numpy	AC	A
py37-xdist (osx)	T	T
pypy	A	A
pypy-xdist	T	T
pypy3-xdist	T	T

[blueyed]

For reference: https://codecov.io/gh/pytest-dev/pytest/compare/ee626743...revisit-cov-new/changes to see what is still missing (ee62674 is before the coverage dropped).

[blueyed]

\o/