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

Hypothesis>=3.31 breaks coverage tracking for test methods. #997

Closed
wsanchez opened this issue Nov 29, 2017 · 13 comments · Fixed by #1031
Closed

Hypothesis>=3.31 breaks coverage tracking for test methods. #997

wsanchez opened this issue Nov 29, 2017 · 13 comments · Fixed by #1031

Comments

@wsanchez
Copy link

I was adding some new code to Twisted Klein and the code is tested with the help of hypothesis (thanks!):

twisted/klein#220

The build is failing because of a drop in coverage, and the coverage report shows that the code in the unit tests themselves has not been used (but the code being tested is covered) in tests where the @given decorator is used.

I'm pretty sure this is a new thing, so I tried older version and have determined that hypothesis 3.30 works, and 3.31 and later do not.

@alexwlchan
Copy link
Contributor

alexwlchan commented Nov 29, 2017

There were five versions of 3.30.x – from 3.30.0 to 3.30.4.

3.30.4 added a bunch of stuff for using coverage information, and seems like an obvious candidate for the bug.

3.31.0 blocked installation on Python 3.3, and I think is a red herring here.

Which patch release of 3.30 do you know works?

wsanchez added a commit to twisted/klein that referenced this issue Nov 29, 2017
@wsanchez
Copy link
Author

wsanchez commented Nov 29, 2017

Interesting… I had assumed that Tox (pip?) would install 3.30.4 when I specified 3.30, but I went ahead and tested specific minor versions and 3.30.0 works as expected, but 3.30.1 does not.

So it appears that 3.30.1 introduced the issue.

@DRMacIver
Copy link
Member

I actually think this is correct behaviour and suspect that the thing that is being diffed against is a Hypothesis run from when the behaviour was broken. 3.30.1 fixed the problem that when run using Hypothesis 3.30.0 you'd get a whole bunch of files you didn't ask for in your coverage. Your coveragerc is configured to only include things under klein, so the test files shouldn't be included.

@DRMacIver
Copy link
Member

Your coveragerc is configured to only include things under klein, so the test files shouldn't be included.

Oh, wait, that's not true - you've got your tests under the klein namespace. Hmm.

@wsanchez
Copy link
Author

I actually think this is correct behaviour and suspect that the thing that is being diffed against…

There is no thing being differ against. I'm running coverage on my computer and when I run my tests with hypothesis 3.30.1 or greater the coverage report shows that tests decorated with @given have not been run. If I use hypothesis 3.30.0, then show as covered. I'm not using any diff thingo (I don't even know how to diff coverage reports, actually).

@DRMacIver
Copy link
Member

Yeah, but my point was that the fact that tests show up in 3.30.0 could just be the fact that 3.30.0 is wrong about what files should be included in coverage! Though having re-examined coverage's behaviour I don't think that's it.

@rhizoome
Copy link

I have the same issue. I tried to come up with an simple example, but I failed. The only thing I can say: the coverage is correct if I set use_coverage=False in conftest.py, otherwise it fails consistently.

@rhizoome
Copy link

rhizoome commented Dec 1, 2017

We are guilty of having the tests inside our project's namespace and therefore running coverage on them. It is nice to see that all the code of the tests runs, though. And it is test code that is not tracked.

@DRMacIver
Copy link
Member

OK. I understand this bug now I think. At the very least I know the culprit and a workaround, though I've not quite nailed down the interactions that cause it, and I'm not sure what the best fix is yet.

TLDR workaround: Add */test_*.py to the source section of your coveragerc.

The culprit is the fact that we lie about what file @given tests come from, using this lovely little function. This is largely for aesthetic reasons, but the aesthetic reasons are quite a big deal.

What I think is then happening is that coverage is tracing the call there, looking up __file__ in the function's globals, using that to conclude it's not a test you want to run, and then caching the result so that when it calls your actual code you don't recheck with the new frame.

There are some subtleties that I haven't quite got though. In particular I haven't found a good reproducible test case yet, and some things I think shouldn't work do work.

@DRMacIver
Copy link
Member

I've been experimenting more with this today, and the really annoying thing is that I simply cannot reproduce this in an isolated way. It happens in klein, but whenever I try to produce a test case that demonstrates it it simply doesn't work. I don't currently have any idea why this might be.

I'm more than happy to remove the functionality that is breaking this and collaborate with the pytest folks about a less terrible way to do it, but I don't really want to do a fix for this without a test to verify that it stays fixed.

If anyone has any ideas it would be much appreciated.

@exarkun
Copy link

exarkun commented Dec 12, 2017

Here's a somewhat minimal example that reproduces the behavior for me, derived from the txkube test suite (perhaps obviously):

https://github.com/LeastAuthority/txkube/blob/53ddb656b222b7926b06e6f396b3dada982c2215/src/txkube/test/test_authentication.py

When I run this test and generate a coverage report:

coverage run --rcfile=${PWD}/.coveragerc $(type -p trial) txkube.test.test_authentication
coverage report

Then I am told there are uncovered lines in that test:

Name                                       Stmts   Miss Branch BrPart  Cover
----------------------------------------------------------------------------
src/txkube/test/test_authentication.py         6      1      0      0    83%

But in my estimation I would say it is trivially obvious that all lines should be covered.

Hope this helps.

@wsanchez
Copy link
Author

Verified with Klein.

Thanks for the fix!

@rhizoome
Copy link

rhizoome commented Dec 18, 2017

Works here too. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants