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

update tox/travis to check code coverage on py3 #1471

Merged
merged 8 commits into from Mar 5, 2015

Conversation

Projects
None yet
3 participants
@mmerickel
Member

mmerickel commented Nov 22, 2014

Coverage is combined between py2 and py3 for an aggregate coverage
metric. This means we can stop putting "no cover" gates around py3 code
and ignoring whether it's ever tested.

This is the best way I could find to do this based on several resources [1,2]. I'd be ecstatic if someone has a cleaner way. The only negative I see of this approach is running the tests one final time to dump xunit and coverage.xml files.

Fixes #1470.

[1] http://blog.ionelmc.ro/2014/05/25/python-packaging/
[2] https://github.com/tuskar/python-tuskarclient/blob/master/tox.ini

mmerickel added some commits Nov 22, 2014

update tox/travis to check code coverage on py3
coverage is combined between py2 and py3 for an aggregate coverage
metric. This means we can stop putting "no cover" gates around py3 code
and ignoring whether it's ever tested.
@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver Nov 22, 2014

Member

The report env is failing -- we aren't getting 100% coverage of all those lines previously skipped due to pragma: no cover.

Member

tseaver commented Nov 22, 2014

The report env is failing -- we aren't getting 100% coverage of all those lines previously skipped due to pragma: no cover.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Nov 22, 2014

Member

Damn it passed locally but it might be running differently on travis. Time to investigate.

Member

mmerickel commented Nov 22, 2014

Damn it passed locally but it might be running differently on travis. Time to investigate.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Nov 22, 2014

Member

Yeah so, I'm not sure how to get around this issue. Each sub-job is executed in its own sandbox on travis in parallel so they cannot share the coverage file.

Member

mmerickel commented Nov 22, 2014

Yeah so, I'm not sure how to get around this issue. Each sub-job is executed in its own sandbox on travis in parallel so they cannot share the coverage file.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Dec 1, 2014

Member

The best way I can think of to get around this is to create one final job that does this. So each individual job does not run coverage and goes quickly. Then there is a final job that runs "TOXENV=clean,py26,py32,cover" or something like that. What do you think @tseaver? This seems reasonable to me.

Member

mmerickel commented Dec 1, 2014

The best way I can think of to get around this is to create one final job that does this. So each individual job does not run coverage and goes quickly. Then there is a final job that runs "TOXENV=clean,py26,py32,cover" or something like that. What do you think @tseaver? This seems reasonable to me.

@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver Dec 1, 2014

Member

You have to have coverage enbaled during the py26 and py27 runs, too.

Member

tseaver commented Dec 1, 2014

You have to have coverage enbaled during the py26 and py27 runs, too.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Dec 1, 2014

Member

@tseaver That's correct but I could send that in as a separate env option like WITH_COVERAGE. I've seen that done on some of the originally linked templates. Or just define a py26_cover and py32_cover.

Member

mmerickel commented Dec 1, 2014

@tseaver That's correct but I could send that in as a separate env option like WITH_COVERAGE. I've seen that done on some of the originally linked templates. Or just define a py26_cover and py32_cover.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Feb 11, 2015

Member

I'm open to some alternatives but this latest push 14126ca has the following outputs:

  • coverage-py26.xml, nosetests-py26.xml
  • coverage-py27.xml, nosetests-py27.xml
  • coverage-py32.xml, nosetests-py32.xml
  • coverage.py33.xml, nosetests-py33.xml
  • coverage.py34.xml, nosetests-py34.xml
  • coverage.pypy.xml, nosetests-pypy.xml
  • coverage.pypy3.xml, nosetests-pypy3.xml
  • coverage.xml (an aggregate of the project that is at 100%)

Main change required to our jenkins box is to read nosetests-py26.xml instead of nosetests.xml because that one is not generated any longer (it was always just py26 but the name has changed).

Advantages:

  • We no longer use nosexcover. Less dependencies.
  • Uncovered some test fixtures that weren't being used due to better output.
  • Aggregate report of 100% coverage with far fewer "no cover" pragmas.
  • We are now incentivized to get coverage to 100% in modules that are not compat.py to improve specific-version output.

Disadvantages:

  • The individual coverage reports are not at 100% which can be annoying when running in a dev environment. Developers would need to learn to run tox -e py26,py32,cover to get a total. I think this is fine.
  • Coverage seems to run pretty slow on pypy.
Member

mmerickel commented Feb 11, 2015

I'm open to some alternatives but this latest push 14126ca has the following outputs:

  • coverage-py26.xml, nosetests-py26.xml
  • coverage-py27.xml, nosetests-py27.xml
  • coverage-py32.xml, nosetests-py32.xml
  • coverage.py33.xml, nosetests-py33.xml
  • coverage.py34.xml, nosetests-py34.xml
  • coverage.pypy.xml, nosetests-pypy.xml
  • coverage.pypy3.xml, nosetests-pypy3.xml
  • coverage.xml (an aggregate of the project that is at 100%)

Main change required to our jenkins box is to read nosetests-py26.xml instead of nosetests.xml because that one is not generated any longer (it was always just py26 but the name has changed).

Advantages:

  • We no longer use nosexcover. Less dependencies.
  • Uncovered some test fixtures that weren't being used due to better output.
  • Aggregate report of 100% coverage with far fewer "no cover" pragmas.
  • We are now incentivized to get coverage to 100% in modules that are not compat.py to improve specific-version output.

Disadvantages:

  • The individual coverage reports are not at 100% which can be annoying when running in a dev environment. Developers would need to learn to run tox -e py26,py32,cover to get a total. I think this is fine.
  • Coverage seems to run pretty slow on pypy.
@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver Feb 19, 2015

Member

LGTM overall. I think I would prefer to make py27 the "canonical" PY2 version at this point, but we can tweak that later.

Member

tseaver commented Feb 19, 2015

LGTM overall. I think I would prefer to make py27 the "canonical" PY2 version at this point, but we can tweak that later.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Feb 19, 2015

Member

I've thought about this a bit and the main change I want to make before this is merged is that the tests run dog-damn slow on travis when running coverage. I don't have a good idea how to fix it though. Tentative idea is:

  • Turn off tests for most envs.
  • Run final (slow) test on just py26,py32,cover with an env variable to turn on coverage.
Member

mmerickel commented Feb 19, 2015

I've thought about this a bit and the main change I want to make before this is merged is that the tests run dog-damn slow on travis when running coverage. I don't have a good idea how to fix it though. Tentative idea is:

  • Turn off tests for most envs.
  • Run final (slow) test on just py26,py32,cover with an env variable to turn on coverage.
@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Feb 20, 2015

Member

@tseaver can you please re-evaluate this based on my latest changes? Minus some quirks on tox syntax I'm pretty happy with this configuration. The outputs are the following:

  • nosetests-py26.xml
  • nosetests-py27.xml
  • nosetests-py32.xml
  • nosetests-py33.xml
  • nosetests-py34.xml
  • nosetests-pypy.xml
  • nosetests-pypy3.xml
  • coverage-py2.xml
  • coverage-py3.xml
  • coverage.xml (an aggregate of the project that is at 100%)
Member

mmerickel commented Feb 20, 2015

@tseaver can you please re-evaluate this based on my latest changes? Minus some quirks on tox syntax I'm pretty happy with this configuration. The outputs are the following:

  • nosetests-py26.xml
  • nosetests-py27.xml
  • nosetests-py32.xml
  • nosetests-py33.xml
  • nosetests-py34.xml
  • nosetests-pypy.xml
  • nosetests-pypy3.xml
  • coverage-py2.xml
  • coverage-py3.xml
  • coverage.xml (an aggregate of the project that is at 100%)
@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mcdonc

This comment has been minimized.

Show comment
Hide comment
@mcdonc

mcdonc Mar 4, 2015

Member

I have no problem with any of this.

Member

mcdonc commented Mar 4, 2015

I have no problem with any of this.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Mar 4, 2015

Member

@mcdonc jenkins will have to be fixed to use nosetests-py27.xml

From what I have read at some point you can actually have jenkins use multiple xml files to combine the results, but I cannot verify this.

Member

mmerickel commented Mar 4, 2015

@mcdonc jenkins will have to be fixed to use nosetests-py27.xml

From what I have read at some point you can actually have jenkins use multiple xml files to combine the results, but I cannot verify this.

mmerickel added a commit that referenced this pull request Mar 5, 2015

Merge pull request #1471 from Pylons/feature.py3-coverage
update tox/travis to check code coverage on py3

@mmerickel mmerickel merged commit 6c1a1c6 into master Mar 5, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mmerickel mmerickel deleted the feature.py3-coverage branch Mar 5, 2015

@tseaver

This comment has been minimized.

Show comment
Hide comment
@tseaver

tseaver Mar 9, 2015

Member

Jenkins did break. I tweaked its config to look for nosetests-py27.xml, and re-ran to ensure it succeeds.

Member

tseaver commented Mar 9, 2015

Jenkins did break. I tweaked its config to look for nosetests-py27.xml, and re-ran to ensure it succeeds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment