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.

Sign up for GitHub

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

Jump to bottom

TST: Enable coveralls/codecov code coverage #4678

Merged
merged 14 commits into from Nov 16, 2015
Merged

TST: Enable coveralls/codecov code coverage #4678

merged 14 commits into from Nov 16, 2015

Conversation

jenshnielsen
Copy link
Member

This is a rebase of #4367 on top of #4668 / #4188
#4668 should be merges before this one.

Changes are:

@jenshnielsen
Copy link
Member Author

Forget it. The relative paths only work with the pep8 tests that we don't want

@jenshnielsen jenshnielsen force-pushed the tst_coveralls branch 2 times, most recently from 6e82294 to 6d08a2e Compare July 16, 2015 00:38
@jenshnielsen
Copy link
Member Author

Looks like codecov.io works much better. For those interested it is running on my master branch here https://codecov.io/github/jenshnielsen/matplotlib

I still have to do some fine tuning before this is ready to merge/review

@jenshnielsen jenshnielsen changed the title Enable coveralls code coverage [WIP] Enable coveralls/codecov code coverage Jul 16, 2015
@mdboom
Copy link
Member

mdboom commented Jul 16, 2015

Just curious -- I've only ever used coveralls, but have found it to be unresponsive at times. Any other advantages to codecov.io?

@jenshnielsen
Copy link
Member Author

I spent a significant amount of time at the Scipy sprint trying to get the relative/absolute links to the source working correctly with coveralls. The answer is probably burried somewhere in our setup/installation but with codecov it just seems to work.

There are other issues at the moment. I had to manually exclude the source code of numpy etc. which should already be excluded.

@tacaswell tacaswell modified the milestone: proposed next point release Jul 17, 2015
@jenshnielsen jenshnielsen force-pushed the tst_coveralls branch 2 times, most recently from 6332a8d to 7f5e5a2 Compare July 28, 2015 16:44
@mdboom
Copy link
Member

mdboom commented Oct 6, 2015

@jenshnielsen: This is cool -- I'd like to start improving coverage, and this is a logical first step. What do you think remains to be done here? I have no problem going with codecov over coveralls if that's working better.

@jenshnielsen
Copy link
Member Author

AFAIK it's basically working. I decided to postpone it till after 1.5 but I should be ready to get back to it. Let me see if I can rebase it over the weekend.

@jenshnielsen jenshnielsen mentioned this pull request Nov 1, 2015
Closed
mdboom
mdboom reviewed Nov 2, 2015
View reviewed changes
tests.py
@@ -23,6 +23,10 @@
def run(extra_args):
from nose.plugins import multiprocess

env = {"NOSE_WITH_COVERAGE": 1,
'NOSE_COVER_PACKAGE': 'matplotlib',
'NOSE_COVER_HTML': 1}
Copy link
Member

Choose a reason for hiding this comment

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

This should add to, not replace, os.environ.

Copy link
Member

Choose a reason for hiding this comment

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

Since code coverage can slow down runtimes, and it's not compatible with multiprocessing tests, we should probably have this be an option controlled by a commandline flag. Then on Travis-CI, we can do this just on one configuration (probably Python 3.5), and keep all the others running with multiple processes.

@jenshnielsen jenshnielsen force-pushed the tst_coveralls branch 2 times, most recently from 691aeec to 759aac0 Compare November 3, 2015 14:09
@mdboom
Copy link
Member

mdboom commented Nov 12, 2015

This is looking quite cool. I assume when all is said and done, there is a way to not print the coverage report to the console on Travis? It makes it hard to scroll up to find any actual failures or errors.

@jenshnielsen jenshnielsen force-pushed the tst_coveralls branch 2 times, most recently from a97a061 to 979763c Compare November 13, 2015 08:24
@jenshnielsen jenshnielsen force-pushed the tst_coveralls branch 3 times, most recently from a7a8ec8 to ba87bcc Compare November 15, 2015 12:36
@jenshnielsen
Run coverage only when activated
2ce262e 
Run one of the jobs without multiprocesses and run coverage with this. The covarage plugin does not work well with multiprocessing
@jenshnielsen
Copy link
Member Author

I think this should finally be ready to go. I guess the main question is if we want to do this or wait for the py.test work which will change this and hopefully make it a lot simpler.

I have moved the environment to init.py and made it possible to run coverage from matplotlib.test() via a function argument. I have reverted to coveralls which seems more stabile than codecov.

The only way have been able to make either of codecov and coveralls able to figure out the files is to do a develop install. This also means that the log is a lot shorted because al the files are not copied.
I have done that via pip install -e . which means that no log at all is shown on a successful build.

Furthermore this is running my fork of nose to be able to suppress the output to std out.

According to the nose docs and various issues you only get reliable coverage output when you disable multiprocessing so I have done that on one of the jobs and only run coverage on that.

@jenshnielsen jenshnielsen mentioned this pull request Nov 15, 2015
Merged
@mdboom
Copy link
Member

mdboom commented Nov 16, 2015

With respect for waiting for py.test -- I don't see a strong reason to wait given that you've already done the work here. But I'll leave this up for a while in case there are differing opinions. I agree it should hopefully be easier under py.test in the long run.

@jenshnielsen jenshnielsen changed the title [WIP] Enable coveralls/codecov code coverage TST: Enable coveralls/codecov code coverage Nov 16, 2015
@tacaswell
Copy link
Member

I am 👍 on merging this now.

@WeatherGod
Copy link
Member

Just noticed that the mpl_toolkits aren't "covered".

@WeatherGod
Copy link
Member

I should clarify that what I mean is that the mpl_toolkits, while tested (somewhat), aren't included in any metrics or reporting.

@tacaswell
Copy link
Member

cbook.py also has some really funny looking coverage results.

@jenshnielsen
Copy link
Member Author

@WeatherGod I don't think mpl_toolkits are included in @tacaswell original work. I think we just need to add it to the list of modules covered.

@jenshnielsen
Copy link
Member Author

Yes there is something odd in cbook and other places. Functions that are not recorded as called but have lines within them registered as called. I will try to reproduce locally.

@tacaswell
Copy link
Member

I am going to merge this now and iterate on it as we go.

tacaswell added a commit that referenced this pull request Nov 16, 2015
@tacaswell
Merge pull request #4678 from jenshnielsen/tst_coveralls
ab100f4 
TST: Enable coveralls/codecov code coverage
@tacaswell tacaswell merged commit ab100f4 into matplotlib:master Nov 16, 2015
@jenshnielsen jenshnielsen mentioned this pull request Nov 19, 2015
Closed
@jenshnielsen jenshnielsen deleted the tst_coveralls branch January 5, 2016 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jenshnielsen @mdboom @tacaswell @WeatherGod