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

Integrating Coverage.py with cesium #211

Merged
merged 14 commits into from Nov 14, 2016
Merged

Conversation

arkwave
Copy link

@arkwave arkwave commented Oct 21, 2016

  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.

after_success: .drone/deploy_docs.sh
after_success:
- .drone/deploy_docs.sh
- codecov
Copy link
Contributor

Choose a reason for hiding this comment

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

Look at deploy_docs.sh to see how to only run this on one Python version

@stefanv
Copy link
Contributor

stefanv commented Nov 10, 2016

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?

if [[$COVERAGE == 1]]

then
echo "Python version: 3.4/3.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get rid of this line

set -e

if [[$COVERAGE == 1]]

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this blank line

echo "Running coverage.py"
nosetests -v --exe --with-coverage
else
echo "Python version: 2.x"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this "else" segment

@@ -7,7 +7,17 @@ source ~/envs/cesium/bin/activate

section "Tests"

make ${TEST_TARGET}
if [[$COVERAGE == 1]]

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line

if [[$COVERAGE == 1]]

then
echo "Python version: 3.4/3.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line

else
echo "Python version: 2.x"
echo "Coverage not set up for python 2.x"
make ${TEST_TARGET}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like TEST_TARGET is unused. Let's remove it from .travis.yml, and then do something like this:

if [[ $COVERAGE == 1 ]]; then
    NOSE_FLAGS="--with-coverage"
fi

nose -v $NOSE_FLAGS cesium

@stefanv
Copy link
Contributor

stefanv commented Nov 13, 2016

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

@arkwave
Copy link
Author

arkwave commented Nov 13, 2016

@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
Copy link
Contributor

stefanv commented Nov 14, 2016

It looks like they have the following:

after_success:
  - codecov

@arkwave
Copy link
Author

arkwave commented Nov 14, 2016

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
Copy link
Contributor

stefanv commented Nov 14, 2016 via email

@arkwave
Copy link
Author

arkwave commented Nov 14, 2016

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
Copy link
Contributor

stefanv commented Nov 14, 2016 via email

@stefanv
Copy link
Contributor

stefanv commented Nov 14, 2016 via email

@stefanv stefanv merged commit ef05c4e into cesium-ml:master Nov 14, 2016
@stefanv
Copy link
Contributor

stefanv commented Nov 14, 2016

Excellent, thanks @arkwave !

@arkwave arkwave deleted the codecoverage branch November 29, 2016 19:36
@bnaul
Copy link
Contributor

bnaul commented Jan 30, 2017

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

@stefanv
Copy link
Contributor

stefanv commented Jan 31, 2017

See #237

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.

None yet

3 participants