Hey cool, I've just submitted my first PR against GH, and it's *so* nice to be able to use git, I might have to do this more often <wink>. https://github.com/python/cpython/pull/138 But I have questions about the coverage reports. In bbdef4c, coverage failed because my diff wasn't 100% covered, and the module itself was missing some coverage in code I didn't touch. If you click on the red X next to that commit, you can see the codecov/patch report 94.73% of diff hit. It's expected that some lines may be missed, due to underlying platform support, so the natural thing would be to say so in the code. I.e. suppress some misses so coverage doesn't false positive. This leads to questions: * How is codecov.io actually performing coverage? It seems difficult to find out exactly based on their documentation, and python/cpython's .codecov.yml doesn't reveal any details. Over in #python-dev, nedbat guessed it would be coverage.py (big surprise! :), and that would both make sense and be great, since it's a well-known and well-loved tool. * On that guess, I figured I'd add a few #pragmas for lines which are okay not to be covered. I pushed the new commit and didn't see a codecov.io failure so I thought that did the trick, but I'm actually not so sure. If you look at the above PR, you can see that commit bbdef4c has a red X, clicking on which gives you details that codecov/patch failed but travis-ci/pr succeeded. However, if you hover over the green check next to ccefb3a, all you see is that the travis-ci/pr check succeeded. There's no indication that coverage actually ran at all on ccefb3a. That doesn't make sense to me. * How can you run the same coverage tests locally? It's hard to know, given the apparent dearth of details from codecov.io. The top-level Makefile does have a `coverage` target, but that's a different thing. It would be nice if you didn't have to push a new commit just to see if you've fixed your coverage issues. There needs to be a local way to run the same tests. As I'm writing this, suddenly ccefb3a gained another check! So it seems like codecov.io is quite delayed in reporting its coverage, and the PR results can be misleading in the meantime, because it only reports that there is 1 check until codecov.io completes. I would prefer to see a yellow dot (i.e. the test is still running). Cheers, -Barry
On Thu, Feb 16, 2017 at 9:35 AM, Barry Warsaw
Hey cool, I've just submitted my first PR against GH, and it's *so* nice to be able to use git, I might have to do this more often <wink>.
https://github.com/python/cpython/pull/138
But I have questions about the coverage reports. In bbdef4c, coverage failed because my diff wasn't 100% covered, and the module itself was missing some coverage in code I didn't touch. If you click on the red X next to that commit, you can see the codecov/patch report 94.73% of diff hit.
It's expected that some lines may be missed, due to underlying platform support, so the natural thing would be to say so in the code. I.e. suppress some misses so coverage doesn't false positive. This leads to questions:
* How is codecov.io actually performing coverage? It seems difficult to find out exactly based on their documentation, and python/cpython's .codecov.yml doesn't reveal any details. Over in #python-dev, nedbat guessed it would be coverage.py (big surprise! :), and that would both make sense and be great, since it's a well-known and well-loved tool.
Travis CI is doing coverage, and it pushes coverage.xml to codecov.io. Codecov will "just" aggregate coverage report from different travis-ci matrix entries to get the complete coverage. You can push coverage to codecov.io from your local machine, but that's painful.
* On that guess, I figured I'd add a few #pragmas for lines which are okay not to be covered. I pushed the new commit and didn't see a codecov.io failure so I thought that did the trick, but I'm actually not so sure. If you look at the above PR, you can see that commit bbdef4c has a red X, clicking on which gives you details that codecov/patch failed but travis-ci/pr succeeded.
However, if you hover over the green check next to ccefb3a, all you see is that the travis-ci/pr check succeeded. There's no indication that coverage actually ran at all on ccefb3a. That doesn't make sense to me.
that's because Codecov is triggered and set the pending/failed/success status only once travis push the first coverage report. GitHub does not trigger codecov. But I think you figured it out with your bottom comment. I agree I got confused as well. I think that might be due to coverage being only ran on "Allow failure" of travis, so the travis green check appears before travis is actually done computing coverage.
* How can you run the same coverage tests locally? It's hard to know, given the apparent dearth of details from codecov.io. The top-level Makefile does have a `coverage` target, but that's a different thing. It would be nice if you didn't have to push a new commit just to see if you've fixed your coverage issues. There needs to be a local way to run the same tests.
That I'm usure. I would just strongly recommend codecov browser extension: https://github.com/codecov/support/wiki/Browser-Extension They add an overlay in the github UI that color the line numbers red/green depending on coverage. (PR diff and file browser) so you basically never have to go to the codecov website. [1]
As I'm writing this, suddenly ccefb3a gained another check! So it seems like codecov.io is quite delayed in reporting its coverage, and the PR results can be misleading in the meantime, because it only reports that there is 1 check until codecov.io completes.
I would prefer to see a yellow dot (i.e. the test is still running).
Agreed, I think we have to figure out how to have tests working with COVERAGE=True. One possibility might be to push an empty coverage report on a fast branch. Docs ? Cheers, -- M [1]: I'd love a similar plugin for vim if one of you know one.
On Feb 16, 2017, at 10:09 AM, Matthias Bussonnier wrote:
Travis CI is doing coverage, and it pushes coverage.xml to codecov.io. Codecov will "just" aggregate coverage report from different travis-ci matrix entries to get the complete coverage. You can push coverage to codecov.io from your local machine, but that's painful.
Thanks! This was the missing ingredient. The .travis.hml encapsulates exactly how the coverage integration works. It probably wouldn't be too hard to throw the appropriate before_script/script commands in a script or Makefile target so folks could locally just do "make codecov" or some such (there's already a "make coverage" but it doesn't do what you think it does).
that's because Codecov is triggered and set the pending/failed/success status only once travis push the first coverage report. GitHub does not trigger codecov. But I think you figured it out with your bottom comment.
Yep. As it turns out, the Developers Guide does talk about coverage: https://docs.python.org/devguide/coverage.html but the .travis.yml gets around some of the problems by using a venv.
That I'm usure. I would just strongly recommend codecov browser extension: https://github.com/codecov/support/wiki/Browser-Extension
They add an overlay in the github UI that color the line numbers red/green depending on coverage. (PR diff and file browser) so you basically never have to go to the codecov website. [1]
That's fine. What I really want is to be able to run coverage locally to see what the results would be once I push my branch so I can fix anything obvious beforehand. I'm okay with going to codecov.io if there are any unexpected coverage regressions. One of the packages that I've grown very fond of is diff_cover. It provides a very nice way to see coverage of just your delta. In my projects, I fail CI if diff_cover reports < 100% even if the overall project isn't at 100% yet. That way, I know all new code is covered, and I can implement a ratchet for existing code. codecov.io does something diff_cover like.
I would prefer to see a yellow dot (i.e. the test is still running).
Agreed, I think we have to figure out how to have tests working with COVERAGE=True. One possibility might be to push an empty coverage report on a fast branch. Docs ?
It would be worth experimenting with. Thanks for the useful information! Cheers, -Barry
participants (2)
-
Barry Warsaw
-
Matthias Bussonnier