-
-
Notifications
You must be signed in to change notification settings - Fork 440
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
Try/except reports incomplete coverage when both paths are taken #35
Comments
Original comment by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt) Let's try that again...
|
Original comment by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt) I started trying to figure this out, but became quite confused when the following test passed without making changes to coverage:
It seems like the transitions are being recorded at a lower level, but the high-level reporter doesn't report that. |
Unfortunately, an except clause with a type is actually a three-way branch. The case you are missing is a non-IndexError exception. But I agree that this is very counterintuitive and probably needs to be fixed. A few details are at http://nedbatchelder.com/code/coverage/branches_html/branches.html |
Original comment by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt) That explanation makes sense, although I still don't understand why my test passed. It seems like the third invisible branch should've been in arcz_missing. :) |
Gary, I admire your fortitude attempting to add that test! The check_coverage method won't even attempt branch measurement if the arcz argument is omitted. |
Original comment by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt) I spent some more time trying to fix it, but it looks quite hard because it mixes levels in a way that coverage doesn't have to deal with currently. The generic except check is done by comparing the name of the thrown exception to each of the named except: blocks (LOAD_NAME/COMPARE_OP/JUMP_IF_FALSE). I tried to put in a horrible hack where I cross-referenced the line number of the current byte code against the original text of the code, and avoided adding it as an exit point if I was at the except: line. But then I started getting a headache and gave up. :( This probably isn't useful to you, given the amount of time you've spent with this stuff, but I thought I'd share. :) Thanks for all of the work you've done so far; I use coverage hundreds of times in an average day. |
Gary, that might be what's required. In fact, it's very similar to the way I'm dealing with the "class is a branch" problem (<<issue 32>>). |
Gary, there's another possibility: the bytecode for except clauses has a distinctive signature: COMPARE_OP 10 (exception match) But I'm also wondering whether it's right to hide the comparison being done here. And underlying all the uncertainty is the fact that no coverage theory out there seems to consider exception handling in the first place... |
Original comment by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt) Here's my reasoning about it: In Python, any line could potentially throw an exception. Code within a try without a catch-all except isn't actually any different in this regard (unless I'm missing something). So if we count potentially uncaught exceptions within a try as branches, we really should count every non-trivial line of code in that way, which we clearly don't want. :) |
I changed the code parsing to ignore the branch from the except clause to the next. Fixed in <<changeset e01bfcdcc17f (bb)>>. |
Originally reported by Gary Bernhardt (Bitbucket: garybernhardt, GitHub: garybernhardt)
The following file reports that only one of the two branches were followed. Since I'm calling it both ways, I'd expect it to report both! :)
The text was updated successfully, but these errors were encountered: