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

Yielded twisted failure marked as missed #440

Closed
nedbat opened this issue Oct 27, 2015 · 7 comments
Closed

Yielded twisted failure marked as missed #440

nedbat opened this issue Oct 27, 2015 · 7 comments
Labels
bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented Oct 27, 2015

Originally reported by Jakob de Maeyer (Bitbucket: jdemaeyer, GitHub: jdemaeyer)


Hi,

in this except clause (lines 75 through 81), we catch all exceptions, do some cleanup, and then reraise the exception by wrapping it in a twisted.Failure. The whole function is wrapped inside twisted's inlineCallbacks magic.

The coverage report (XML here, run with timid = True) shows the variable assignment in line 80 being hit but the subsequent line being missed. An accompanying test passes when line 81 is there and fails when I remove it, so I'm pretty sure it should indeed be hit.

I tried to create a minimal working example here but failed, sorry :/


@nedbat
Copy link
Owner Author

nedbat commented Oct 27, 2015

Can you provide me with detailed instructions on how to run the test suite?

BTW: I used scrapy yesterday, good stuff :)

@nedbat
Copy link
Owner Author

nedbat commented Oct 27, 2015

Original comment by Jakob de Maeyer (Bitbucket: jdemaeyer, GitHub: jdemaeyer)


Hey Ned, thanks for looking into it.

I'm afraid I'm not too familiar with the test suite internals. I clone the repository, then run tox2 in it, or tox2 -- tests/test_crawl.py::CrawlTestCase::test_graceful_crawl_error_handling to run the relevant test only, after enabling timid mode in .coveragerc.

btw I use coverage.py every day, also good stuff :)

@nedbat
Copy link
Owner Author

nedbat commented Nov 14, 2015

@jdemaeyer The line you're talking about ("testvar = True") isn't in the current version of the repo (commit 57f87b95d4d705f8afdd8fb9f7551033a7d88ee2 (bb)). Did you add it there specifically for investigating this bug? Is there a problem with the code as it exists in the repo?

@nedbat
Copy link
Owner Author

nedbat commented Nov 15, 2015

Original comment by Jakob de Maeyer (Bitbucket: jdemaeyer, GitHub: jdemaeyer)


Hey Ned! Yeah, the testvar line was to make sure that the yield statement really is hit. The code as it is in the current master works as expected (i.e. the failure is yielded). There is a test that explicitly tests that here but the line is reported as missed.

@nedbat
Copy link
Owner Author

nedbat commented Jan 31, 2016

@jdemaeyer Looks like this is a really simple thing that doesn't involve Twisted at all. I didn't understand your smaller test cases that didn't work. I think this boils down to: yields that are never returned to are marked as not executed:

$ cat foo.py
def gen():
    print("yup")
    yield "yielded"
    print("nope")

print(next(gen()))
$ coverage run --branch foo.py
yup
yielded
$ coverage report -m
Name     Stmts   Miss Branch BrPart  Cover   Missing
----------------------------------------------------
foo.py       5      2      0      0    60%   3-4

When run without branch coverage, it's understood better:

$ coverage run foo.py
yup
yielded
$ coverage report -m
Name     Stmts   Miss  Cover   Missing
--------------------------------------
foo.py       5      1    80%   4

@nedbat
Copy link
Owner Author

nedbat commented Jan 31, 2016

Fixed in b03f78976a78 (bb)

@nedbat nedbat closed this as completed Jan 31, 2016
@nedbat
Copy link
Owner Author

nedbat commented Mar 28, 2016

Issue #482 was marked as a duplicate of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant