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

[travis] Record code coverage and display on README #703

Merged
merged 2 commits into from
Jan 8, 2019

Conversation

willprice
Copy link
Contributor

@willprice willprice commented Jan 1, 2019

This PR adds code coverage to the travis builds and adds an accompanying badge.

A few details of why I've implemented it the way I have:

To test against the installed version of the package (which is
preferably since this can catch installation file inclusion bugs) we
have to deal with quirks of coverage.py. By default the ecosystem is
set up to do development coverage tests (e.g. in src, or by using pip install -e . or python setup.py develop).

We want to test against the version installed, to do this we have to
find the install path which we'll then pass to the --cov arg added by
pytest-cov. To do this, we have to cd out of the current folder, and
import the installed version and get it's install path (if we don't cd
out, then we end up getting the existing directory in the cwd since by
default cwd is on sys.path)

Once we have the install path, we pass that to the --cov of pytest,
however we also want to rewrite the paths for codecov to pick them up on
the website, if they don't have to code coverage with local paths, they
don't register is properly.

In order to that, we have a .coveragerc file that has the contents:

[paths]
source =
    torchvision
    /**/site-packages/torchvision

This tells codecov to treat all paths with those prefixes as the same,
so anything like
/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/torchvision/models/__init__.py
would be treated the same as
torchvision/models/__init__.py after running the coverage combination
command. The first path seems to be special, in that all subsequent
paths are rewritten to that one.

Once we collect coverage, we then run coverage run to rewrite the
installation paths in the coverage file, .coverage, to those expected
by codecov.

Phew. And that's it.

N.B: Whilst adding test/init.py does solve the issue, it also results in
PWD being added to the path, and then we'll test the development version
of the package, and not the installed version (see
https://docs.pytest.org/en/latest/goodpractices.html#tests-as-part-of-application-code)

@codecov-io
Copy link

codecov-io commented Jan 1, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@84896a6). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #703   +/-   ##
========================================
  Coverage          ?   43.1%           
========================================
  Files             ?      29           
  Lines             ?    2909           
  Branches          ?     476           
========================================
  Hits              ?    1254           
  Misses            ?    1581           
  Partials          ?      74

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 84896a6...8390094. Read the comment docs.

@willprice willprice force-pushed the code-coverage branch 9 times, most recently from 17729f3 to 44b3131 Compare January 1, 2019 23:24
Test installed version of package

To test against the installed version of the package (which is
preferably since this can catch installation file inclusion bugs) we
have to deal with quirks of coverage.py. By default the ecosystem is
set up to do development coverage tests (e.g. in src, or by using `pip
install -e .` or `python setup.py develop`).

We want to test against the version installed, to do this we have to
find the install path which we'll then pass to the `--cov` arg added by
`pytest-cov`. To do this, we have to cd out of the current folder, and
import the installed version and get it's install path (if we don't cd
out, then we end up getting the existing directory in the cwd since by
default cwd is on `sys.path`)

Once we have the install path, we pass that to the `--cov` of pytest,
however we also want to rewrite the paths for codecov to pick them up on
the website, if they don't have to code coverage with local paths, they
don't register is properly.

In order to that, we have a .coveragerc file that has the contents:

```
[paths]
source =
    torchvision
    /**/site-packages/torchvision
```

This tells codecov to treat all paths with those prefixes as the same,
so anything like
`/home/travis/miniconda/envs/test-environment/lib/python2.7/site-packages/torchvision/models/__init__.py`
would be treated the same as
`torchvision/models/__init__.py` after running the coverage combination
command. The first path seems to be special, in that all subsequent
paths are rewritten to that one.

Once we collect coverage, we then run `coverage run` to rewrite the
installation paths in the coverage file, `.coverage`, to those expected
by codecov.

Phew. And that's it.

N.B: Whilst adding test/__init__.py does solve the issue, it also results in
PWD being added to the path, and then we'll test the development version
of the package, and not the installed version (see
https://docs.pytest.org/en/latest/goodpractices.html#tests-as-part-of-application-code)
@willprice
Copy link
Contributor Author

If/When this PR is merged in, someone will need to add a new config entry to travis.

  1. Visit https://codecov.io/gh/pytorch/vision
  2. Login and add vision as a repo (you can go directly to https://codecov.io/gh/pytorch/vision)
  3. Copy value of the CODECOV_TOKEN="xxxxxxxx-a3df-45fb-xxxx-114d63b284af" token
  4. Open up https://travis-ci.com/pytorch/vision and go to the settings page and add the token value with the name CODECOV_TOKEN, make sure the option for this to show up in builds is not ticked (this is a secret key)

Et voila, it should all be hunky dory.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks!

I'll let @soumith comment on this as well, as it involves a few changes that I'm not very familiar with.

@willprice
Copy link
Contributor Author

Looks like the travis python 2.7 build timed out and needs to be rerun.

@fmassa fmassa closed this Jan 8, 2019
@fmassa fmassa reopened this Jan 8, 2019
@fmassa
Copy link
Member

fmassa commented Jan 8, 2019

(Closing and reopening the PR also rerun the tests)

@willprice
Copy link
Contributor Author

@fmassa, thanks, didn't think of that!

@willprice willprice closed this Jan 8, 2019
@willprice willprice reopened this Jan 8, 2019
@willprice willprice closed this Jan 8, 2019
@willprice willprice reopened this Jan 8, 2019
@soumith soumith merged commit 98ca260 into pytorch:master Jan 8, 2019
@willprice willprice deleted the code-coverage branch January 8, 2019 23:32
@soumith
Copy link
Member

soumith commented Jan 8, 2019

thanks @willprice !

@willprice
Copy link
Contributor Author

@soumith Thanks for merging :)

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.

4 participants