Add support for codecov in AppVeyor #3923
Conversation
tox.ini
Outdated
| deps = | ||
| {[testenv]deps} | ||
| coveralls | ||
| codecov | ||
| commands = | ||
| coverage run -m pytest testing | ||
| coverage run -m pytest testing/test_config.py |
There was a problem hiding this comment.
Temporarily run only testing/test_config.py for testing purposes.
Codecov Report
@@ Coverage Diff @@
## master #3923 +/- ##
========================================
- Coverage 94.99% 93.2% -1.8%
========================================
Files 108 108
Lines 23437 23356 -81
Branches 2332 2321 -11
========================================
- Hits 22265 21769 -496
- Misses 881 1254 +373
- Partials 291 333 +42
Continue to review full report at Codecov.
|
|
Finally, I think it is now working properly. 😅 |
nicoddemus
changed the title
|
Not sure why py27 and xdist jobs are failing... any hints @blueyed? |
scripts/prepare-coverage.bat
Outdated
| @@ -0,0 +1,7 @@ | |||
| if not defined PYTEST_NO_COVERAGE ( | |||
| set "_PYTEST_TOX_COVERAGE_RUN=coverage run --source {envsitepackagesdir}/_pytest/,%CD%/testing -m" | |||
| set "_PYTEST_TOX_EXTRA_DEP=coverage-enable-subprocess" | |||
There was a problem hiding this comment.
i beleivethis should add coverage in any case
There was a problem hiding this comment.
What do you mean @RonnyPfannschmidt? That we should add coverage even for environments like linting and docs?
There was a problem hiding this comment.
no, the requirement, i beleive i saw a few errors about the coverage binary being not in the env
There was a problem hiding this comment.
also i believe coverage integration should be a tox pugin - @tox-dev might have an oppinion there
There was a problem hiding this comment.
Are you sure it was coverage? I see tox warning about env:
WARNING: test command found but not installed in testenv
cmd: /usr/bin/env
env: /home/travis/build/pytest-dev/pytest/.tox/py36
Maybe you forgot to specify a dependency? See also the whitelist_externals envconfig setting.
I've tried to fix this and the py27-xdist job by explicitly setting COVERAGE_FILE and COVERAGE_PROCESS_START outside tox in a60cc25, but this ended up breaking other things so I removed this commit from this branch for the moment.
Let's see what @blueyed advises here. 👍
There was a problem hiding this comment.
This will also collect coverage for tox then already - I have seen errors that source for /tmp/pip-req… was missing. It would need explicit includes then probably.
Instead of using coverage we could have a wrapper script instead that sets it then?
There was a problem hiding this comment.
Re tox-coverage: I think that would be nice to have, and could look similar to https://github.com/tarpas/pytest-testmon/blob/master/testmon/tox_testmon.py - i.e. it would automatically install codecov and set up env, if tox --coverage was used.
There was a problem hiding this comment.
Instead of using coverage we could have a wrapper script instead that sets it then?
See #3923 (comment), I don't think that's necessary, but would indeed be a good solution if that was not the case. 👍
There was a problem hiding this comment.
Also it is a mystery to me why py27-xdist fails to collect coverage, while py36-xdist works...
There was a problem hiding this comment.
i find it acceptable to skip coverage there and recording a followup issue, given the pending EOL of python2.7 im inclined to declare it as acceptable loss
| - C:\Python36\python -m pip install --upgrade --pre tox | ||
|
|
||
| build: false # Not a C# project, build stuff at the test step instead. | ||
|
|
||
| before_test: | ||
| - call scripts\prepare-coverage.bat |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
tox.ini
Outdated
| coverage: _PYTEST_TOX_EXTRA_DEP=coverage-enable-subprocess | ||
| coverage: COVERAGE_FILE={toxinidir}/.coverage | ||
| coverage: COVERAGE_PROCESS_START={toxinidir}/.coveragerc | ||
| {env:_PYTEST_TOX_COVERAGE_RUN:} pytest --lsof -ra {posargs:testing} |
There was a problem hiding this comment.
Please keep the "coverage" factor?!
There was a problem hiding this comment.
Unless I'm missing something, the "coverage" factor will only be considered when executing tox -e *coverage* no? If that's the case, we don't execute any test env with "coverage" in the name, that's why I thought this could be removed.
There was a problem hiding this comment.
Not on CI, but you can do tox -e py36-coverage manually.
There was a problem hiding this comment.
Oh I see, did not thought about that use case. Will add it back. 👍
|
py27-xdist and py27-nobyte (the failing ones) do not appear to generate coverage ("no data to combine"). |
|
py36-xdist works however. |
I can reproduce the issue locally: With |
Here's the output of |
|
@nicoddemus |
|
The debug output appears to have the "coverage combine" problem though?! It says "source_match: C:\pytest\testing" - but should probably also have "_pytest" there?! |
scripts/prepare-coverage.bat
Outdated
| @@ -0,0 +1,7 @@ | |||
| if not defined PYTEST_NO_COVERAGE ( | |||
| set "_PYTEST_TOX_COVERAGE_RUN=coverage run --source {envsitepackagesdir}/_pytest/,%CD%/testing -m" | |||
There was a problem hiding this comment.
We could use here {envinidir} instead of %CD% probably.. (also with Travis then)
There was a problem hiding this comment.
Oh good catch. Will do.
Done, let's see how it goes. |
|
Build finally finished. |
|
Pushed a commit to use |
|
Interestingly this fails py27-xdist on Travis now, too - SyntaxErrors and such: https://travis-ci.org/pytest-dev/pytest/jobs/424201724 |
Oh my that doesn't make sense. Let's hope that was some fluke. I've pushed a new change, I noticed |
|
Oh look at that, it now passed. But the coverage seems fishy on Windows: Pushed a new commit which also sets |
|
Strange, any idea why tox fails with: Given that all I changed on diff --git a/.travis.yml b/.travis.yml
index 17cceb84..0efb2b1f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -82,7 +82,9 @@ env:
before_script:
- |
if [[ "$PYTEST_NO_COVERAGE" != 1 ]]; then
- export _PYTEST_TOX_COVERAGE_RUN="env COVERAGE_FILE={toxinidir}/.coverage COVERAGE_PROCESS_START={toxinidir}/.coveragerc coverage run --source {envsitepackagesdir}/_pytest/,{toxinidir}/testing -m"
+ export COVERAGE_FILE="{toxinidir}/.coverage"
+ export COVERAGE_PROCESS_START="{toxinidir}/.coveragerc"
+ export _PYTEST_TOX_COVERAGE_RUN="coverage run --source {envsitepackagesdir}/_pytest/,{toxinidir}/testing -m"
export _PYTEST_TOX_EXTRA_DEP=coverage-enable-subprocess
fi |
|
Duh of course, because now we don't go through |
|
Strange, why is |
I think this is a directory pip uses to install pytest or something like that. |
We currently appear to collect coverage from pip-install, where then the source is not available anymore. Ref: pytest-dev#3923 (comment)
|
Oh, actually suggestion is |
|
No dice 😞 |
Given that I'm out of ideas, I would be OK with this as well. After all having coverage for all the other environments is already a large improvement. |
|
OK, to me it is time to admit defeat. 😅 I'm skipping the two failing envs from coverage in AppVeyor. If @blueyed or someone else wants to investigate more this can be done in a future PR, as adding coverage for the other environments will already be a net gain. 👍 |
|
|
Oh OK, I thought that was part of the previous effort to get py27-xdist working. Reverted and pushed. |
|
@nicoddemus |
|
Oh! Could you push that then please? I'm away from my computer |
Yes, it is - somehow missed that you are skipping it now. I would say that we should retry it here, instead of in a separate PR. |
|
What I mean is going back to #3923 (comment), and use |
We currently appear to collect coverage from pip-install, where then the source is not available anymore. Ref: pytest-dev#3923 (comment)
|
OK, I will cleanup history and re-enable all the envs once 3.8.0 is out. Thanks a lot @blueyed for all the help here! 🤗 |
|
Squashed the long commit history! Hopefully now everything is working. 👍 |
| - TOXENV: "docs" | ||
| PYTEST_NO_COVERAGE: "1" |
There was a problem hiding this comment.
@nicoddemus
Was the removal of pexpect intentional here?
There was a problem hiding this comment.
Yes, those jobs don't run on Windows:
(.env36) λ tox -e py27-pexpect
GLOB sdist-make: c:\pytest\setup.py
_____________________________________________ summary _____________________________________________
SKIPPED: py27-pexpect: platform mismatch
congratulations :)
So it doesn't make sense to queue the job just to skip it right at the beginning. 👍
No description provided.