search

cesium-ml / cesium

Machine Learning Time-Series Platform
Other
670 stars 101 forks source link

Integrating Coverage.py with cesium #211

Closed arkwave closed 7 years ago

arkwave commented 7 years ago

1) updated travis.yml to run codecov after build passes. 2) added in a .coveragerc file to stop coverage.py from testing scikit-learn imports 3) updated requirements.txt to include coverage.

stefanv commented 7 years ago

Problem: the test suite will now get run twice, once without and once with coverage, under Python 3.5. Can we change it so that it only is run once?

stefanv commented 7 years ago

We probably also need a command to upload the coverage report to cov.io?

arkwave commented 7 years ago

@stefanv I'm not entirely sure, this seems to suggest that integration is handled automatically: https://github.com/codecov/codecov-python (specifically the Configuration section). Thoughts?

stefanv commented 7 years ago

It looks like they have the following:

after_success:
  - codecov
arkwave commented 7 years ago

Hmm I am a little confused in that case. Initially, that same line after_success: codecov was marked for deletion because it led to the test suite running twice. However, in the scikit-image repo, additional dependencies are tested with coverage in the same way as they're done here in travis_script.sh, but after succcess: codecov is still included in the travis.yml file. Wouldn't this also lead to coverage running twice, or is there something I'm misunderstanding?

stefanv commented 7 years ago

In most cases, that command just won't do anything, since there's no coverage information to upload: https://travis-ci.org/scikit-image/scikit-image/jobs/174340273#L7563

arkwave commented 7 years ago

Alright, I have added that line back in. Thanks for catching it. Also, does the cesium-ml repo might have to be registered at codecov.io for coverage reports to be published? Or is that more to centralize the coverage reports at a single location?

stefanv commented 7 years ago

I've registered cesium with them now, so we should be good to go!

stefanv commented 7 years ago

Looks like codecov is missing (no pip install?): https://travis-ci.org/cesium-ml/cesium/jobs/175605412#L1617

Also, code coverage is enabled for two items in the matrix, instead of just one.

stefanv commented 7 years ago

Excellent, thanks @arkwave !

bnaul commented 7 years ago

This does not appear to actually work (see: all the PRs since this was merged).

stefanv commented 7 years ago

See https://github.com/cesium-ml/cesium/pull/237