Proposal: pytest-cov should do less

[nedbat]

I've been working in the pytest-cov code recently to get it to support coverage.py 5.0 (pull request: #319), and also reviewing #330. The problems that have come up have brought me to a conclusion: pytest-cov does too many things.

The current code attempts to be a complete UI for coverage. I think this is a mistake. Pytest-cov should limit itself to those functions that can only be done within a pytest plugin. In particular, the reporting options of pytest-cov add no value over just using coverage directly to generate reports.

Pytest-cov is no one's passion project. There are a few of us that would like to move it forward, but want to do it efficiently. Removing functionality from pytest-cov would make it easier to maintain.

If we keep to the current model of pytest-cov being a complete UI for coverage.py, then every time something is added to coverage.py, the pytest-cov plugin needs to be updated. As an example, we just added a new report type to coverage.py (JSON). Why do we need to add options to pytest-cov to support it?

Pytest is a test runner. Its job is to run tests. The pytest-cov plugin should limit itself to integrating the run phase of coverage into the test running process. Reporting is a separate step that is better separately.

The current design leads to tortured option syntax in an attempt to squeeze everything into the pytest command line. There's no need. Remove options that aren't related to running, and let some options be specified in configuration files. Sophisticated test suites already have configuration files. Let's simplify pytest-cov.

Thoughts?

[nedbat]

/cc @ionelmc @graingert

[nedbat]

@jezdez Is there something i can do to clear up the confusion (if that's what the emoji means!)? I may have written too fast and left out some context.

[jezdez]

I absolutely understand the "simple is better than complex" idea in this proposal and I support you in making maintenance easier. 👍🏻

I definitely worry about having to know the coverage CLI tool as well as setting up pytest-cov to generate terminal output though. At the moment it's one step in a very pytest-typical config setup (updating addopts in pytest.ini) and you're proposing two steps to get the same result, addopts and another way to run coverage report. Given how difficult some CI environments are (e.g. path problems, incompatible dependencies etc) I think this change would basically trade lower maintenance friction on your side with a higher barrier of ease of use. Not saying that's a bad trade, just that it would make sense to maybe find a way to alleviate that higher barrier for users.

Alternatively to your suggestion would there be a way to instead implement a simplified version of the coverage API in pytest-cov? Only cover terminal reporting in pytest-cov for example and document how to do more complex reporting via the coverage CLI?

[graingert]

I'd be in favour of the coverage API providing a method like .summary() such that this code would pass the tests: https://github.com/pytest-dev/pytest-cov/commit/5493d822db91ecca715daa79c41fff512a9ce478

[graingert]

would be some wiggle room in how cov_fail_under would be passed to summary of course, or it could handle the cov_fail_under entirely and raise an exception after having finished all the writing

[nedbat]

1. A few people have mentioned the convenience of having one command (pytest --cov-etc) that will run tests and produce coverage reports. I understand that convenience, but a simple shell script would do the same job. I don't think it makes sense for pytest-cov to implement a new but worse UI for coverage.py to get that convenience.

I could compromise on @jezdez's idea of keeping text reporting, and leaving other reporting to the coverage.py command line. How would other people feel about that?

2. I don't understand how a new API method in coverage.py will solve the problem? The UI would still be a set of baroque options in pytest-cov, that would need to be maintained, extended, and tested.

BTW: I'm very glad for the discussion! :)

[somacdivad]

As a frequent user of Pytest-cov, I'm strongly in favor of reducing its features.

And I agree with @jezdez that keeping the console reports is worth thinking about. It's literally the only reporting feature I use with Pytest-cov, if I use it for reporting at all, and I can see how it's handy enough to keep around.

The benefits of the Pytest-cov reporting UI always felt marginal to me, at best. At worst they just add bloat to my Pytest commands.

[okken]

simplifying the plugin would be great. Allowing text reporting would be preferable to not.

[gaborbernat]

You get my vote to go down this path with pytest cov 3 👍

[maurobaraldi]

What about two "versions" of plugin: pytest-cov and pytest-cov (enhanced).Ok, it does not much sense for me too, an enhanced version from a plugin, but the version could be like a branch from vcs project.

Maybe, this would be like a experiment, to analyse how many people miss this "enhanced features".

[nedbat]

@maurobaraldi I'm not sure who would maintain the enhanced one. If I only contribute to the plain version (to add support for new coverage.py features), then the enhanced won't be a superset of the plain version. That sounds extra-confusing. :(

[loechel]

It may be good for the discussion to talk about the affected API and options and common use cases and configurations.

I think it should be differentiated between three major useages:

• direct call of pytest
• usage of tox
• usage of CI/CD with coveralls

The direct useage of pytest is for simplicity and speedup of concurrent testing / subprocesses

Usage of tox may follow two mayor config styles:

as described in coverage docs:

[testenv]
...
commands =
    pytest --cov=src --cov=tests --cov-report=xml

setenv =
  COVERAGE_FILE=.coverage.{envname}

deps =
    pytest
    pytest-cov

[testenv:coverage]
skip_install = true

deps =
    coverage

setenv =
  COVERAGE_FILE=.coverage

commands =
    coverage erase
    coverage combine
    coverage html
    coverage xml
    coverage report --fail-under=100.0

or as documentet in pytest-cov docs:

[testenv]
commands = pytest --cov --cov-append --cov-report=term-missing
deps =
    pytest
    pytest-cov
depends =
    {py27,py36}: clean
    report: py27,py36

[testenv:report]
deps = coverage
skip_install = true
commands =
    coverage report
    coverage html

[testenv:clean]
deps = coverage
skip_install = true
commands = coverage erase

My question is, if cov-report options are removed, would both approches still work. Are other options as terminal and xml actually used? May it be possible to delegate the whole reporting to the coverage framework itself?

Could it be an option make pytest-cov just a commad wrapper around coverage and do noting itself anymore?

[nedbat]

Could it be an option make pytest-cov just a commad wrapper around coverage and do noting itself anymore?

@loechel, there are important things pytest-cov does that only it can do: managing the distribution and re-collection of coverage data under xdist; signaling when tests begin and end for who-tests-what; probably a few other things. It should continue to do those things. I don't see the value in pytest-cov being a wrapper around coverage.py's other functionality.

[nedbat]

There were 26 +1's on the top description. I don't know how to see the names in the GitHub UI. I had to use the API to get them: blueyed, davidism, sethmlarson, gvoysey, styvane, boxed, CodeMouse92, yeraydiazdiaz, ionelmc, RazerM, timofurrer, ivoflipse, somacdivad, obestwalter, okken, dcoles, otrenav, Stranger6667, gaborbernat, mblayman, eddieantonio, sivy, merwok, fschulze, jab, gkapfham.

[ionelmc]

Mkay so I'll add my feedback:

• If we do a pytest-cov 3.0 that removes shitton of features then we'd have more things to do: spending time on arguing what to remove, explaining why was it removed and ensuring that removal doesn't make the plugin completely unusable. So the questions is who offers time to fight the bulk of the user complaints.
• It's a good idea architecturally to avoid implementing features that should be, could be or are in coveragepy.
• I think we should start with something less radical: drop compatibility with weird or old combos or dependencies: only latest pytest, only latest coveragepy, only latest python and so on.

I do have the same "things are not optimal" feeling, that's why I +1.

[nedbat]

I hear what you are saying about dealing with the fallout.

I think probably everyone is on-board with dropping support for older everything (though I would like to keep Python 2.7 in the mix for now). Let's assume that is happening no matter what.

Then we have to decide what else to do. Here are some possibilities, a gradient of severity: A. Nothing more than that. (This is your third bullet.) B. Keep everything working as-is, but add warnings to features that will be dropped. So if you use --cov-report=html, you will get a warning, and an HTML report. C. Drop features we don't want any more, but print helpful messages about what happened. If you use --cov-report=html, you get a message saying, "This feature has been removed, use 'coverage html' instead." D. Drop features completely. If you use --cov-report=html, you get an error message, "--cov-report: unrecognized option"

(Note: in the above, I used --cov-report=html as a proxy for any feature that has been dropped.)

While D would mean the leanest resultant code, I would lean toward option C, to help people transition.

[fschulze]

+1 on C

[offbyone]

I'd personally want the current reporting to be available as an API then; I use pytest to drive coverage extensively for a variety of reasons, and if pytest-cov loses those features I'll need to reimplement them. Wrapping a command line when I'm already in Python is not ideal; I'd like to be able to invoke them as code.

If so, that'd be fine; I'd be happy to have consistent coverage reporting control between test frameworks.

[nedbat]

@offbyone I'm not sure what you mean by an API. Coverage.py has an API. Would that work? Keeping all the code in pytest-cov but exposing it in an API from the plugin only adds to the complexity, it doesn't reduce it.

[meejah]

coverage should inherit pytest-cov's ability to get coverage out of sub-proceses easily. Every time I (attempt) to do subprocess coverage, I re-discover a bunch of gotcha's with atexit, signals, environment and .pth files ;) that mean it doesn't "just work".

[blueyed]

@meejah re subprocesses see: https://pypi.org/project/coverage_enable_subprocess/ - https://github.com/nedbat/coveragepy/issues/367

[nedbat]

@meejah I am in favor of exploring the parts of pytest-cov that could be moved into coverage.py itself. To keep the process a little simpler now, I'm only talking here about dropping code from pytest-cov that is already in coverage.py.

The .pth/subprocess/etc stuff is gross and scary however it is done, but I understand that people find it frustrating.

[rixx]

I'm a frequent user of pytest and pytest-cov. I use the various reporting options (both terminal in various versions, and HTML) through pytest-cov, currently, as a matter of my regular development cycle. Thank you for soliciting opinions and making the discussion so open for everybody.

I, as a user, would be happy enough to deal with option C. I frequently work offline or on very bad internet, so being confronted with an error after an already tedious update woud be unpleasant, whereas having explicit instructions available would be an unexpectedly positive experience. (I assume I'm not speaking for a majority here, but maybe for a subset of devs that tend to be forgotten, occasionally.)

I'd understand if you choose option D instead, but I'd feel less happy about it. If adding instructions for the different flags is hard, then please make that fact very clear in the changelog – it would definitely reduce my annoyance if I ran into it by accident.

[offbyone]

@nedbat yes, probably. I wasn't saying it didn't, only that I need one to replace pytest-cov's reporting for in-process work :)

[ionelmc]

@meejah re subprocesses see: https://pypi.org/project/coverage_enable_subprocess/ - nedbat/coveragepy#367

pytest-cov does much more than that, and much better. It supports all packaging and installation forms (coverage_enable_subprocess does not).

Someone has suggested that there's no need for pytest-cov (ignoring subprocess stuff) since you can just run coverage run -mpytest, however that has some problems:

• slower runs since coverage is active during pytest's initialization (and not really needed there, since you don't care about coverage of pytest as a user of pytest)
• lack of API to control coverage from within hooks (conftest) or tests (https://pytest-cov.readthedocs.io/en/latest/markers-fixtures.html)

[merwok]

FWIW: I always use coverage run form, because I knew it before I started using pytest, I have the (incorrect?) notion that coverage needs to run first to setup its tracing hooks, and I like memorizing a generic way that works with any test runner or other command. I also visit the coverage doc regularly to see settings and news, so I guess in my mind coverage is its own tool, not just a test runner plugin.

I thought pytest-cov was sugar (only a shortcut) until I read https://github.com/pytest-dev/pytest-cov/issues/337#issuecomment-530598528, and tox already gives me a shortcut for coverage run + coverage report. Now I see that it’s useful for test suites with subprocesses or pytest-xdist.

Also, I also really enjoy the possibility of running bare pytest without virtualenv or coverage for maximum feedback speed; on client projects configured with pytest-cov it’s a little cumbersome to type pytest --no-cov=project to disable coverage report when you’re in a «many tests broken, fix one at a time» phase.

@rixx If you have pytest-cov installed, you also have coverage, and coverage help gives offline help about commands and options. Does that address your needs?

[rixx]

@rixx If you have pytest-cov installed, you also have coverage, and coverage help gives offline help about commands and options. Does that address your needs?

Not exactly. My use case is that I'm used to running pytest with --cov-report flags of various kinds. If those suddenly just throw errors at me, I won't know where to look. (Please note that I'd understand if that option is the one that wins out, I just think the other one will make a bunch of people pretty happy when upgrading.)

[merwok]

Throwing an idea out there: what if pytest-cov was not changed, and the new ideas implemented in a new plugin with another name? That way no automated upgrade can break command lines, maintainers will have to see warnings or announcements and decide to switch (the docs could help with translation of pytest-cov options to coverage.py commands).

[nedbat]

@merwok I think I like the idea of changing the name. We could make one more release of pytest-cov, where it prints a warning that there will be no more updates to it, and that you should switch to pytest-coverage. Then pytest-coverage can simply remove all the obsolete options.

This has advantages: people have to explicitly switch over, and we don't need an awkward middle phase where the code is still complex, but doesn't do what people want.

[gaborbernat]

I think I like the proposal minus the warning part 👍