#1698 closed defect (fixed)
the preferred cmdline to run tests under coverage is pretty huge, and comes in two flavors due to Debian renaming of the script
Reported by: | davidsarah | Owned by: | warner |
---|---|---|---|
Priority: | normal | Milestone: | 1.10.1 |
Component: | code | Version: | 1.9.1 |
Keywords: | usability test coverage | Cc: | |
Launchpad Bug: |
Description (last modified by warner)
The preferred command line to run tests recording coverage information is:
bin/tahoe @coverage run --branch @tahoe debug trial [ARGS]
but Debian/Ubuntu? renamed the "coverage" script to "python-coverage", in which case it is:
bin/tahoe @python-coverage run --branch @tahoe debug trial [ARGS]
PYTHONPATH=. python bin/tahoe debug trial --reporter=bwverbose-coverage [ARGS]
or it is also possible to use:
bin/tahoe debug trial --coverage [ARGS]
However, these seem to cover fewer lines than the "coverage run" approach, in particular, top-level declarations in some files (for which their omission is clearly a bug... there's no way to import those files without executing those lines). Also, --reporter=bwverbose-coverage and --coverage do not allow passing additional options to 'coverage', therefore they do not support obtaining branch coverage, which is more useful.
To fix this ticket, add and document a way to run tests with correct branch coverage that works on all platforms using a short command line.
Change History (17)
comment:1 follow-up: ↓ 8 Changed at 2012-03-30T23:49:32Z by davidsarah
comment:2 Changed at 2012-03-30T23:52:22Z by davidsarah
It should also be documented how to run tests with coverage for the Tahoe installed from an OS package (/usr/bin/tahoe).
comment:3 Changed at 2012-11-02T01:04:02Z by davidsarah
- Owner changed from somebody to davidsarah
- Status changed from new to assigned
comment:4 Changed at 2012-11-02T01:07:55Z by davidsarah
- Milestone changed from 1.11.0 to eventually
comment:5 Changed at 2014-09-02T17:02:18Z by warner
- Description modified (diff)
also see #2289 (--branch coverage)
comment:6 Changed at 2014-09-02T20:54:08Z by daira
- Milestone changed from eventually to 1.12.0
- Owner changed from davidsarah to daira
- Status changed from assigned to new
Actually I'd forgotten about this ticket; #2289 is a duplicate. Its description was:
The correct command to run tests with branch coverage is:
bin/tahoe @coverage run --branch --source=src/allmydata @tahoe --version-and-path debug trial --rterrors allmydata.testexcept that @coverage sometimes needs to be @python-coverage depending on how coverage is installed.
This is impossible to remember (and the @coverage vs @python-coverage thing is annoying). Let's add a --coverage option to python setup.py trial and python setup.py test. Also let's have it print a nice error message saying how to install coverage if it's missing.
comment:7 Changed at 2014-09-02T20:54:26Z by daira
- Status changed from new to assigned
comment:8 in reply to: ↑ 1 Changed at 2014-09-02T20:58:36Z by daira
Replying to davidsarah:
... and preferably that allows us to delete the bwverbose-coverage reporter plugin.
That is #2281.
comment:9 Changed at 2014-09-02T20:59:39Z by daira
- Priority changed from major to normal
comment:10 Changed at 2014-09-05T05:45:45Z by warner
I've confirmed that running under the coverage executable gets us good data. When we first wrote this coverage code, that executable wasn't so easy to use (or at least I couldn't figure out how to use it correctly), so we had to use that bwverbose-coverage plugin (and several related ones), to execute the coverage.start() and .stop() calls at the right times. In general, it's awesome that the coverage tool is so much easier to use now.
But, if the executable doesn't have a predictable name, and if we want a "tahoe debug trial --coverage" to be responsible for locating it, then we're back to needing to run code at startup time, which locates the executable, and then shells out to the executable (which, we've learned, is usually a bad idea: One Process Is Best). It looks like setup.py Trial is currently doing that anyways, so maybe it wouldn't be too bad.
What would the command be? We want to make sure we run tests with the same Python that was used to invoke bin/tahoe, which means we can't use coverage as the argv0. Hm, so the function would need to scan $PATH for "coverage" or "python-coverage", then use it as argv[1] in ["python", "coverage", ARGS..].
I'm not super-comfortable about having one python program hunt around in $PATH to figure out which other python program to run.. it feels weird to do that at this level. Maybe this is something the Makefile should do? Maybe it could use which coverage and which python-coverage to decide upon the executable, then build up the bin/tahoe command to run underneath.
BTW, why does your preferred command run bin/tahoe @coverage run instead of coverage run bin/tahoe? Are you assuming that coverage will be installed in support/, and therefore requires that sys.path and $PYTHONPATH and $PATH and whatnot be provided by a bin/tahoe parent process? If that's really a requirement, then support/bin/coverage will exist, and we can just use it instead of an OS-supplied coverage/python-coverage executable.
comment:11 Changed at 2014-09-07T07:47:43Z by warner
Actually, I take that back, I think coverage run is *not* providing good data yet. I see make __init__.py files being covered, but nothing else. I suspect that something is spawning a subprocess, or somehow the bulk of the test code is not executing while coverage is watching for it.
(I vaguely remember something like this being the reason that I wrote that plugin in the first place).
I'll investigate more.
comment:12 Changed at 2014-09-07T07:52:36Z by warner
Huh. coverage run bin/tahoe debug trial has the not-really-covered problem. bin/tahoe @coverage run @tahoe debug trial seems to get the right data. I didn't think those two would behave differently.
incidentally, my plan is to change the Makefile to let "make test-coverage" work (and let you set COVERAGE=python-coverage" if you're running on ubuntu), and make travis get coverage, but put off changing the buildbot to get coverage for a while. I think we can let travis+coveralls take responsibility for rendering and archiving coverage data for public consumption, and remove that stuff from the buildbot (which is currently disabled now anyways)
comment:13 Changed at 2014-09-07T20:54:44Z by daira
The Makefile isn't usable (for most users) on Windows. Therefore, I would like to avoid adding new stuff to the Makefile if it needs to work on Windows, which this does.
Note that the argument to "coverage run" is a Python script, not an arbitrary executable. It will always be run in-process, i.e. in the same Python interpreter instance as coverage.
[Edit: corrected the following paragraph.]
The reason why "coverage run bin/tahoe debug trial" does not work is that bin/tahoe runs the support script that does the tests in a subprocess, in order to pass the correct PYTHONPATH. So the coverage that is captured is only that of the bin/tahoe script, since coverage isn't capable of capturing coverage information for subprocesses. (Running it directly wouldn't help, because coverage does not have the right PYTHONPATH and so neither does any script it runs.)
When coverage is run from bin/tahoe using @, it gets the correct PYTHONPATH, and so does the script that it runs. This was one of the main motivations for adding @. Importantly, bin/tahoe will expand @tahoe to the absolute path of the support script --not its own path-- before invoking coverage. If it expanded @tahoe to its own absolute path, then the tests would still be running in a subprocess of coverage which wouldn't work for the same reason as above.
Which Python interpreter is used to run coverage (and therefore the test suite) will be determined by the value of sys.executable in bin/tahoe.
Note that tests that the test suite runs in a subprocess, e.g. test_system.SystemTest.test_filesystem_with_cli_in_subprocess, will not contribute to coverage. I don't know how to fix this. trialcoverage also had that problem.
comment:14 Changed at 2014-09-07T21:15:30Z by daira
Ah, the Makefile already has test-coverage and quicktest-coverage targets. Well, I don't object to making those work, but I really want something that works cross-platform and regardless of whether the command is coverage or python-coverage. Otherwise there is an annoying documentation problem.
comment:15 Changed at 2014-09-08T15:45:19Z by daira
- Keywords review-needed added
- Milestone changed from 1.12.0 to 1.11.0
- Owner changed from daira to warner
- Status changed from assigned to new
comment:16 Changed at 2014-09-09T00:14:28Z by warner
- Keywords review-needed removed
- Resolution set to fixed
- Status changed from new to closed
Looks good, landed in [53dc723184].
I think this is enough for 1.11 . Yeah, Makefiles aren't particularly windows-friendly, but I use them as a shortcut, and I think they can serve as a useful reminder for what commands can be typed. Travis and other automation can use the setup.py test --coverage incantation.
At the start of the 1.12 cycle, I'd like to merge our work on replacing the whole (bin/tahoe, PATH/PYTHONPATH, support/lib, subprocess.call()) mess with something based on virtualenv. Once that's in place, I'd kind of like to remove all knowledge of coverage from tahoe (setup.py, bin/tahoe debug trial --coverage, etc). Then our instructions for doing testing with coverage can be:
- install coverage into the virtualenv
- run venv/bin/coverage run bin/tahoe debug trial, or venv/bin/coverage run setup.py test
Or maybe use setup.py test --coverage but have the implementation depend upon the virtualenv being in place. Anyways, we should have plenty of time to figure out what's the best approach after we get 1.11 done.
comment:17 Changed at 2014-09-10T08:41:36Z by daira
See #1303 for another Windows-only wrinkle (that I'd completely forgotten about) when using coverage run bin/tahoe debug trial.
... and preferably that allows us to delete the bwverbose-coverage reporter plugin.