Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Coverage config#8003

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 3 commits intomatplotlib:masterfromdopplershift:coverage-config
Feb 4, 2017
Merged

Conversation

dopplershift
Copy link
Contributor

  • Cleaning up codecov config to remove commenting
  • Remove coveralls
  • Trying to set up codecov to report coverage in test code separately
  • Also enabled coverage on AppVeyor

@dopplershift
Copy link
ContributorAuthor

If this works first time through I'll be amazed...

@tacaswell
Copy link
Member

Not liking something about the configuration on windows.

    config_read = config.from_file(fname, section_prefix=prefix)  File "C:\Miniconda35-x64\envs\test-environment\lib\site-packages\coverage\config.py", line 248, in from_file    section, unknown, filename

@tacaswelltacaswell added this to the2.1 (next point release) milestoneFeb 1, 2017
@QuLogic
Copy link
Member

Adding tests to coverage fixes#6902.

Copy link
Member

@NelleVNelleV left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@NelleVNelleV changed the titleCoverage config[MRG+1] Coverage configFeb 1, 2017
@QuLogic
Copy link
Member

QuLogic commentedFeb 1, 2017
edited
Loading

So if I understand this correctly; codecov should pass if test files are 100% covered. How is this true, if we aren't technically fully running on Python 2.7 at the moment? Surely there's a skip for Py 2 in at least one test?

@dopplershiftdopplershift changed the title[MRG+1] Coverage config[WIP] Coverage configFeb 1, 2017
@dopplershift
Copy link
ContributorAuthor

It's not working properly yet. There should be 3 separate statuses:

  • Overall library coverage
  • % of the diff covered
  • Lines covered in the tests

Not sure why it didn't work, since this was verbatim from my own project with just some paths changed...

@dopplershift
Copy link
ContributorAuthor

The way this works is that codecov merges coverage runs...so if every line is run at least once, you'll get 100% coverage. We won't be able to see if we're not running things on py2.

It looks like there is a feature called flags where we could mark each run as python 2 or 3. Wecould then segment that way, but then if we have version-specific test code, we would never get to 100%...

@QuLogic
Copy link
Member

If it's anything like the PR to stop codecov from commenting, we might need to actually merge it to see any effect.

@QuLogic
Copy link
Member

Oh wait, maybe it's because we've already got aci/codecov.yml?

@dopplershift
Copy link
ContributorAuthor

Yeah, the duplication is probably the problem. Let's see how this one goes.

@dopplershift
Copy link
ContributorAuthor

dopplershift commentedFeb 2, 2017
edited
Loading

Still didn't take. I've rebased on master (squashed some commits) and fixed a couple minor validation problems with the yaml file I made. If this doesn't seem to take, my only thoughts are to try merging with master and hope for the best.

@dopplershift
Copy link
ContributorAuthor

Bingo! And somehow we only have 97% coverage of test files...

Should we commit (maybe with a different target) and try to fix it elsewhere?

Copy link
Member

@dstansbydstansby left a comment

Choose a reason for hiding this comment

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

👍

@dstansbydstansby changed the title[WIP] Coverage config[MRG+1] Coverage configFeb 2, 2017
@tacaswell
Copy link
Member

Can we push the test coverage limit down before merging?

@NelleV
Copy link
Member

We've got 3% of our tests not running on travis? That's interesting to know.

@QuLogic
Copy link
Member

It's mostly backends (e.g., Qt4/Qt5 can't be run in same process or installed in the same conda environment), some pickle and animation stuff, and everything else seems small.

But when looking at the file on codecov, it says 'File not found in report', so I can't see what the actual problematic lines are.@dopplershift any ideas why it's not coming up properly?

@dopplershift
Copy link
ContributorAuthor

@NelleV Well, 2.2% of the lines of code in our tests are not being executed anyways.

@QuLogic I think it's failing when it tries to compare against previous coverage on the matplotlib repo, which was excluding the tests.

I've rebased to address the conflicts in.travis.yml andappveyor.yml. I also dropped the limit for the tests to 97.7%. I'm almost more interested in the files that are a few tenths below 100% since Ireally don't understand why in those cases.

@dopplershift
Copy link
ContributorAuthor

Rebased now that#7974 is merged. Hopefully everything will work as expected.

@dopplershift
Copy link
ContributorAuthor

I think we're good now. I did notice at least one coverage change caused by the fact that we have a wait loop (for locks incbook.py) that changes our coverage depending on whether we actually wait.

@QuLogic
Copy link
Member

All AppVeyor builds show "No coverage report found".

This way we can get coverage for platform-dependant lines.
This sets it to report even if CI fails (just to get to see numbers).Separate out coverage for lines in tests (which should be 100%, but iscurrently ~97.7%), versus those in the library itself.
@dopplershift
Copy link
ContributorAuthor

Was submitting the coverageafter we cleaned the repos for building packages--which nuked the coverage results. I've now added the upload to the testing phase. (I'm open to other suggestions for fixing this.)

Maybe now everything will work.

@dopplershift
Copy link
ContributorAuthor

Ok, it looks to be properly running on AppVeyor now--though only for pytest=True. I thought we were entirely using pytest now? (Irrelevant for this PR either way.)

@@ -140,6 +140,8 @@ test_script:
- if x%USE_PYTEST% == xno python tests.py %PYTEST_ARGS%
# Generate a html for visual tests
- python visual_tests.py
- if x%USE_PYTEST% == xyes pip install codecov
- if x%USE_PYTEST% == xyes codecov -e PYTHON_VERSION PLATFORM
Copy link
Member

@dstansbydstansbyFeb 4, 2017
edited
Loading

Choose a reason for hiding this comment

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

These lines mean that codecov only happens ifUSE_PYTEST is true, which is only for the one build. pytest is still being run in the other builds, just throughtests.py in the root which callstest() inmatplotlib/__init__.py.

I don't know if running the tests usingtest.py collects coverage, but if it does I guess these if statements can just go.

Copy link
Member

Choose a reason for hiding this comment

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

Yea, these conditions could be dropped.

@tacaswelltacaswell merged commit6b6adb4 intomatplotlib:masterFeb 4, 2017
@dopplershiftdopplershift deleted the coverage-config branchFebruary 4, 2017 23:57
@QuLogicQuLogic changed the title[MRG+1] Coverage configCoverage configFeb 5, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic left review comments

@dstansbydstansbydstansby approved these changes

@NelleVNelleVNelleV approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v2.1
Development

Successfully merging this pull request may close these issues.

5 participants
@dopplershift@tacaswell@QuLogic@NelleV@dstansby

[8]ページ先頭

©2009-2025 Movatter.jp