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

config tool: not compatible with repos which need coverage combine #33

Open
jugmac00 opened this issue May 15, 2020 · 7 comments
Open

Comments

@jugmac00
Copy link
Member

eg. https://github.com/zopefoundation/zc.zodbrecipes

In order to get correct coverage, @mgedmin pointed me to
https://github.com/buildout/buildout/blob/0185dc6a87f417160378547c776fb6da0c016e88/src/zc/buildout/testing.txt#L123-L152

After applying the fixes, you can't apply the config again, otherwise it deletes the fix again.

 [meta]
 template = pure-python
-commit-id = 4f6e46d28989b99f6dd6ee49829060a6720b79b9
+commit-id = 1e477452ac5420330d79b0a88bacc46abd2c858b
 fail-under = 86
 
diff --git a/tox.ini b/tox.ini
index ebcd810..ecacc26 100644
--- a/tox.ini
+++ b/tox.ini
@@ -38,19 +38,15 @@ deps =
     coverage
     coverage-python-version
     zope.testrunner
-setenv =
-    COVERAGE_PROCESS_START={toxinidir}/tox.ini
 commands =
     coverage run -m zope.testrunner --test-path=src []
     coverage html
-    coverage combine
     coverage report -m --fail-under=86
 
 [coverage:run]
 branch = True
 plugins = coverage_python_version
-source = zc.zodbrecipes
-parallel = true
+source = src
 
 [coverage:report]
 precision = 2
@mgedmin
Copy link
Member

mgedmin commented May 16, 2020

In theory it should be safe to use parallel coverage tracing everywhere. At least I'm not aware of any downsides, other than a slightly more complicated config file.

BTW does COVERAGE_PROCESS_START=tox.ini work? I wasn't sure coverage would look at the filename and realize it needs to check [coverage:run] rather than [run].

@mgedmin
Copy link
Member

mgedmin commented May 16, 2020

One advantage of always having parallel = true and calling coverage combine is you can drop usedevelop = true and rewrite all the .tox/lib/python*/ paths into src/..., if you add a [coverage:paths] section like this:

[coverage:paths]
source =
    src/
    .tox/*/lib/python*/site-packages/
    .tox/pypy*/site-packages/

I'm not arguing that we should drop usedevelop = true now, but this would give us an option of doing so.

@jugmac00
Copy link
Member Author

BTW does COVERAGE_PROCESS_START=tox.ini work? I wasn't sure coverage would look at the filename and realize it needs to check [coverage:run] rather than [run].

Yes, it works like a charm - for the moment only on my machine, but not on Travis - but maybe your suggested change will work.
zopefoundation/zc.zodbrecipes#3

@mgedmin
Copy link
Member

mgedmin commented May 16, 2020

It was using a stale .coverage file from the previous run on your machine to produce the HTML reports. ;) If you did a coverage erase before the coverage run ..., you'd've seen the same error as on Travis.

@jugmac00
Copy link
Member Author

Thanks for the explanation - that makes sense!

@mgedmin
Copy link
Member

mgedmin commented May 16, 2020

I just had a realization: we should be calling coverage erase before coverage run, if we set parallel = true.

The reason is this: parallel mode makes each coverage run write a new .coverage.$pid file. These get combined into a single .coverage and then erased by coverage combine.

If the tests fail, tox will stop after running coverage run, and you'll have a .coverage.$pid from the failed run. When you next run tox after fixing the problem, it'll create a new .coverage.$pid2 file, and then coverage combine will combine the results of the failed run with the results of the new successful run.

I don't think we want that, so it would be good to coverage erase explicitly to remove all .coverage.* files before each run.

@jugmac00
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants