Hypothesis: Get check-coverage run time back under control

[DRMacIver]

The idea of tests/cover is that it's supposed to be a relatively small fast set of tests that gets 100% coverage and anything not required for that should go in tests/nocover. This idea has been seen more than in breach than observance (including from me), and as a result the coverage check is one of our slowest build jobs. See e.g. this build where it took 13 minutes.

It would be nice to get the coverage check under 5 minutes on Travis. I propose many small applications of the following algorithm:

1. Run "PYTHONPATH=src python -m pytest tests/cover --durations=10"
2. Pick the slowest test
3. Move it into nocover.
4. Run "make check-coverage" (or "tox -e coverage")
5. If this causes us to have less than 100% coverage, come up with a faster test to cover that line to put in tests/cover.
6. Open a pull request with that one test move.

This probably won't be sufficient to get the time under control on its own, but if we get it to the point where no individual test takes > a second we can start thinking about other measures.

[DRMacIver]

Of course I just tried to apply this algorithm and found that the first thing I ran into required a completely different solution that.

[bilderbuchi]

I have researched a bit how to do this in a more efficient way. It turns out that there is an ongoing discussion about how to add the information "who tests what" to coverage (still unsolved).

From the discussion, there is a tool worth investigating called "smother" which basically is a pytest plugin that generates some additional information about which tests execute which lines in your code. Crucially, it can also dump the full mapping of tests->sources of your test run into a csv file.

I think it should be possibly to process that csv file to find out which minimal set of tests you need to retain full coverage with some fancy set logic. If you're fancy and hack around on it maybe you can even weigh that with test runtimes, although I haven't explored that.

As the make check-coverage is part of a pretty complex testing machinery, including coverage and tox, I did not try for long to identify a simple pytest invocation which would run the equivalent test.

A first step would probably be to identify a suitable simplified pytest call that still exercises the set of tests you want. I don't think it will be easy to use this in conjunction with tox to test on different python versions and whatnot.
Edit: would that be PYTHONPATH=src python -m pytest tests/cover --durations=0? Is this what check-coverage in the end executes?` If so, we're done with that I think.

[DRMacIver]

I think it should be possibly to process that csv file to find out which minimal set of tests you need to retain full coverage with some fancy set logic. If you're fancy and hack around on it maybe you can even weigh that with test runtimes, although I haven't explored that.

Oh, that's a good idea. Thanks!

If we were being really fancy we could treat this as an integer linear programming problem and feed the results into pulp. It should actually be relatively straightforward to do.

One question: We want not just line coverage but also branch coverage. It wasn't obvious to me if smother supported that. Do you know?

Edit: would that be PYTHONPATH=src python -m pytest tests/cover --durations=0? Is this what check-coverage in the end executes?` If so, we're done with that I think.

Yes, that would do it.

[bilderbuchi]

If we were being really fancy we could treat this as an integer linear programming problem and feed the results into pulp. It should actually be relatively straightforward to do.

Too fancy for me, I fear. :D

One question: We want not just line coverage but also branch coverage. It wasn't obvious to me if smother supported that. Do you know?

Apparently not: ChrisBeaumont/smother#3. Does that postpone this idea until that is implemented, i.e. does check-coverage depend on branch coverage to reach 100%?

[DRMacIver]

Does that postpone this idea until that is implemented, i.e. does check-coverage depend on branch coverage to reach 100%?

Sadly, yes, and I'd be very reticent to change that - the difference has caught some interesting bad behaviour in the past.

[DRMacIver]

That being said, doing a half-arsed version of smother (we only need to run it once after all!) wouldn't be too hard. We could just add a fixture in conftest.py which wraps Coverage around each test and writes the data to a file somewhere.

[bilderbuchi]

hah! well, there's also this PR for branch coverage, could use that one directly: ChrisBeaumont/smother#8

[bilderbuchi]

So, an initial draft of this seems to work more or less correctly (rough edges probably because of missing branch coverage).
I then tried to apply this to Hypothesis tests. I have to say, your test harness, while obviously quite sophisticated, is really complex (involving tox, coverage, xdist, flaky, make,..), and hard to understand/run on Windows (e.g. no makefile). I miss the possibility to just hit pytest in a repo's root, and it works. :-/

In the end, I arrived at the command pytest tests/cover --smother=hypothesis --durations=0 -n 0. Why do you have PYTHONPATH=src in front of the call? (btw this does not work on Windows command terminal). I used -n 0 to disable xdist, which made problems. Then I ran into problems with, apparently, flaky and smother (see ChrisBeaumont/smother#11). When uninstalling flaky to see if it then gets disabled, I get ImportErrors (of course) - is there another way to disable flaky for now?

[bilderbuchi]

By the way, I got to thinking about the tests/cover job, and I was wondering if you're not generating a false sense of security when keeping minimal coverage and all tests green?

For example, let's assume you have the function

def square(x):
    return 2*x

and the two tests

test_square_2():
    assert square(2) == 4

test_square_3():
    assert square(3) == 9`

you could totally remove the second test (yeah, cut runtime in half!), while having full coverage and green tests, but you do not find a critical error.

[Zac-HD]

By the way, I got to thinking about the tests/cover job, and I was wondering if you're not generating a false sense of security when keeping minimal coverage and all tests green?

I absolutely agree - 100% branch coverage may be necessary but is certainly not sufficient to test the 'interesting' behavior of any particular program. That said, as long as we also run all the 'nocover' (ie, additional) tests in CI it doesn't really make much difference to me how we group them.

[bilderbuchi]

Another way to probably get the data we need for this is to use pytest-testmon which determines which regions are touched by which tests, and stores the information an sqlite database, which should be easily queryable. Downside: currently not compatible with pytest-xdist.

[DRMacIver]

currently not compatible with pytest-xdist.

We don't run coverage tests under xdist anyway, so that's not intrinsically a problem.

[Zac-HD]

I've actually been looking into testmon or smother as a way to discover unused code - having found ClassMap and mergedb, I think there may be more that hasn't been used in a long time. So far my causal attempts have stopped when everything is giving coverage CTracer errors, but that should pass...

With good coverage profiling, you could reduce the problem of finding uncalled code to calculating connected components of the graph (connecting "test function" to "executed line" nodes), and do a manual check to see if you can eliminate each of them. To be really useful we'd probably also want to look at the only-loosely-connected subcomponents and see if they could be removed too.

[bilderbuchi]

I've actually come quite far when using smother and some set logic around smother's csv report data, but did get bogged down when tracking down why the results did not work perfectly - I lost a couple of percent coverage after trimming when I should have lost none. I think lack of reliable branch coverage in smother could maybe be to blame. If you find it useful, see my rough draft at https://gist.github.com/bilderbuchi/95bf3d91f959922c838a59463baf16e6

[ChrisBeaumont]

@bilderbuchi I'm curious, is your coverage regression purely a branch coverage decrease, or is it a line coverage decrease? The former wouldn't surprise me given smother's lack of branch tracking (I haven't thought through how much work it would take to make smother branch-aware, but I think it should be feasible).

[bilderbuchi]

I was working with a smother pr (number 8?) That implemented branch coverage, but was wip. I didn't find the time to extend my script to somehow give a coverage diff (so I could easily see which lines lost coverage). It's also possible that there is simply a bug in my script...

[DRMacIver]

I'd like to propose the following somewhat radical approach:

• Create a new directory of tests called fast. check-coverage runs all tests in both fast and coverage (possibly renaming coverage to slow-coverage)
• Add a new build job called check-fast that runs everything in fast under coverage.
• check-fast fails if it takes more than 10 seconds to run.
• We implement a ratcheting scheme on check-fast's coverage, requiring that the number of lines not covered by check-fast must never go up (this will require some work, but should be doable with our usual charmingly ad hoc build mix of Python and bash and storing state in git).
• Once that number reaches zero, rename fast to coverage and move any tests still left in coverage into nocover.

This basically gives us a build enforced way of incrementally moving in the direction of fast 100% coverage.

[bilderbuchi]

Just a minor point regarding test directories for fast - pytest has a marking mechanism by which you add a decorator to a test (e.g. @pytest.mark.fast), and can then run only those marked tests with pytest -m fast. This way there is no need to physically move tests around (same with coverage, in fact).

[DRMacIver]

That's true, but I think I actually prefer to physically organise the tests into separate categories.

[Zac-HD]

I'm nervous about this plan, for two reasons:

1. Any time limit is going to be amazingly flaky, no matter what the limit is - Travis is just too variable, let alone "anyone's machine".
2. The assertions we make about behaviour matter at least as much as the number of branches covered, and this scheme will inevitably optimise out the more powerful tests. You've read Seeing like a State; just say "No" to single-metric optimisation of complex systems!

[Zac-HD]

1450 cut the runtime of the coverage job from 20 to 13 minutes on Travis, and that's probably the limit of the low-hanging fruit.

@DRMacIver, is it even worth doing much more?

[DRMacIver]

@Zac-HD Yeah I think it is. If you look at https://travis-ci.org/HypothesisWorks/hypothesis/jobs/410758917, we've got nearly three minutes in the twenty slowest tests alone! I don't know how many of those could be straight up moved to nocover, but I bet all of them could be moved to nocover and then replaced with a much tighter test in cover. Some of those we might even be OK to just straight up delete.

It would be really nice if we could get the cover set down to something we could reliably run fast enough to just put the whole cover test suite (without coverage enabled) relatively centrally into our development loop. Certainly I think we should be able to get the coverage job down to under 5 minutes without too much pain.

[Zac-HD]

The coverage job is now faster than the other jobs; and the slowest 20 tests took 108s, down from 188s.

[CuriousLearner]

Hi,

I'm looking into this :)

[Zac-HD]

A fun project in data collection and analysis would be:

• Determine the duration of each test, perhaps with the --durations argument to pytest
• Use Coverage's new support for "who tests what"
• Calculate the slowest tests that do not uniquely cover any branches (which can trivially be moved to tests/nocover/)

This is a different approach to "unconditionally move the slowest tests and write fast replacements", but potentially complementary. Just remember the tradeoff - moving tests means the coverage suite is weaker - and focus on the slow ones.

[cefisher1]

Hi! I'm looking for my first open-source contribution and this issue seems like a good one to get started on. Has anything changed since Oct. 2018 that I should know about before attempting this?

[Zac-HD]

Nope, it's just as described above.

(personally I'd go for the decorator-check issue though; I think it's a little more self-contained and will also give you a release with your name on it 😁)

[Zac-HD]

Via this CI run, the following runtimes:

========================== slowest test durations ===========================
104.96s call     tests/cover/test_targeting.py::test_targeting_can_be_disabled
 98.07s call     tests/numpy/test_gen_data.py::test_advanced_integer_index_can_generate_any_pattern
 64.73s call     tests/cover/test_stateful.py::test_bad_machines_fail[CanSwarm]
 35.79s call     tests/cover/test_stateful.py::test_bad_machines_fail[RoseTreeStateMachine]
 23.13s call     tests/cover/test_regex.py::test_fullmatch_generates_example[[ab]*-aBb]
 21.35s call     tests/cover/test_searchstrategy.py::test_example_raises_unsatisfiable_when_too_filtered
 16.87s call     tests/cover/test_datetimes.py::test_bordering_on_a_leap_year
 16.27s call     tests/numpy/test_gen_data.py::test_infer_strategy_from_dtype
 15.65s call     tests/numpy/test_gen_data.py::test_can_generate_data_compound_dtypes
 11.60s call     tests/cover/test_stateful.py::test_removes_needless_steps
 10.30s call     tests/numpy/test_gen_data.py::test_can_generate_compound_dtypes
  9.73s call     tests/cover/test_regex.py::test_negative_lookbehind
  7.90s call     tests/cover/test_regex.py::test_negative_lookahead
  6.96s call     tests/cover/test_stateful.py::test_bad_machines_fail[DepthMachine]
  6.22s call     tests/cover/test_stateful.py::test_saves_failing_example_in_database

So there is some pretty obvious low-hanging fruit! Some of these tests we can probably make much faster just by limiting the size of the data we generate; for others we might need to move them to the nocover test suite.