tox: guard against parallel pytest coverage execution

[davvid]

An usual problem users have is that pytest-cov will erase the
previous coverage data by default, thus if you run tox with
multiple environments you’ll get incomplete coverage at the end.

To prevent this problem you need to use --cov-append. It’s still
recommended to clean the previous coverage data to have
consistent output.

Cf. https://pytest-cov.readthedocs.io/en/latest/tox.html

[davvid]

The tox docs are only mention "incomplete coverage", but in practice it's possible to trigger test errors. I've observed the following without these changes:

INTERNALERROR> coverage.misc.CoverageException:
INTERNALERROR> Couldn't use data file '.../.coverage': no such table: meta
...
ERROR: InvocationError for command pytest (exited with code 3)

[jaraco]

Thanks for this. I'd very much like to support evaluating coverage across runs of different Python. I've historically suppressed or ignored coverage on files that had selective execution.

I have some reservations, but I'm happy to explore options.

[jaraco]

My biggest reservation comes from this change, which

(a) adds an extra tox environment to the default argument,
(b) doesn't run under explicit environment invocations such as 'tox -e py38' or 'tox -e python' (which I have configured in my environment),
(c) corrects for lingering state at the beginning, and
(d) even with all of this, the coverage details aren't going to be readily appended in CI environments or other environments that provide isolation between runs.

I think I'd prefer to keep a simple 'tox' environment to simply run the tests, but add more complicated configurations to wrapper scripts or CI invocations or user-local settings.

Do you have a place where you're already using this approach? Does coverage recommend this approach?

[jaraco]

I see this feature was added in tox 3.7, so that would imply a minversion bump to 3.7.

[jaraco]

Nit: I'd prefer this to appear at the end of the list - that's where I typically add them.

[jaraco]

More importantly, what's this line about? Is it to add coverage? To limit coverage? I do try to avoid pinning dependencies without a reason to pin (and a comment to explain why). Looking quickly at coverage, this is a downgrade from what's currently used. Is it necessary to support this feature? If so, there should be a comment explaining that.

[jaraco]

Is this change generally applicable?

[davvid]

Thanks for the review notes. I agree, this isn't ready yet.

The coverage < 5 tweaks are workarounds mentioned for stuff related to pytest-dev/pytest-cov#386 and pytest-dev/pytest-cov#398 (comment) which could be helpful if we add pre-canned coverage environments.

That said, the recent pytest release notes mention changes that may have already addressed these issues, so it's worth trying newer versions these days. https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst#290-2020-05-22

As you noted, this would be best mentioned in a code comment in addition to the commit message so that it doesn't get lost in this random github thread. I have to revisit the coverage < 5 specs.

pytest.ini contains --cov, and that precludes someone from running tox tests in parallel without also having parallel = True in .coveragerc. The original changes were prompted by parallel tox runs stomping over the shared data file, which resulted in failed test runs that were confusing to diagnose.

In order to provide a simple tox test environment where users won't stumble into these edge cases then what do you think about this?

1. Remove --cov from pytest.ini so that the simple tox run doesn't have to worry about coverage. Users that want parallel coverage can inject --cov using {args} via the pytest {args} template.
2. Add parallel = True to .coveragerc, which current seems to be needed for parallel execution.
3. Add a separate coverage environment for doing coverage, which won't have to concern itself with parallel tox execution, and can handle the clean/run/combine/collect steps into a single run.
4. It might also make sense to provide an explicit coverage-clean and coverage-combine environments so that a user can choose to run the clean and collect/combine steps independently. For example, if they ran a parallel run themselves by injecting --cov then this would be helpful for them.

The most useful feature for me is parallel tox running of tests; coverage is secondary and not as important in my perspective, but 1. does not shut the door to that use case.

Supporting all the various test dependency permutations with parallel tox running and coverage can be tricky, which is why it seems like punting and making them explicit separate steps seems helpful.

Of course, this is something that would be best documented a bit, so perhaps the best first commit/patch would be to come up with a patch against skeleton.md to propose the workflow.

What do you think?