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

Confusing branching failure #493

Closed
nedbat opened this issue May 24, 2016 · 7 comments
Closed

Confusing branching failure #493

nedbat opened this issue May 24, 2016 · 7 comments
Labels
branch bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented May 24, 2016

Originally reported by Joe Doherty (Bitbucket: docapotamus, GitHub: docapotamus)


Hello,

I have a strange issue with coverage since upgrading to 4.1

I am getting a branch failure which I know I have a test for. The file: https://codecov.io/gh/pjuu/pjuu/src/508aafc3f35814677fa6d29e555e8636efab0bb8/pjuu/lib/tokens.py

If I change the code and remove the else and add a print, the print shows. However I get a failure on the print saying it isn't covered.

Not sure if this is intended and I can't see any obvious as to why this would fail.

Thanks in advance.


@nedbat
Copy link
Owner Author

nedbat commented May 24, 2016

Can you run the "coverage html" report and see what the line is annotated with? (Hover over the line in the coverage html report).

@nedbat
Copy link
Owner Author

nedbat commented May 25, 2016

Original comment by Joe Doherty (Bitbucket: docapotamus, GitHub: docapotamus)


2 missed branches: 1) line 69 didn't return from function 'check_token', because the return on line 59 wasn't executed or the return on line 62 wasn't executed, 2) line 69 didn't jump to line 74

line 71 didn't jump to line 74

I don't understand why it isn't noticing that line 74 will be run.

I remember a problem here in the past (looking at the comment, sorry!)

I wrote this code a while ago.

@nedbat
Copy link
Owner Author

nedbat commented Dec 16, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


Here is a minimal reproducer, confirmed with python-2.7.12 & python-3.5

#!python

def check_token(data):
    if data:
        try:
            return data

        finally:
            print('abc')
    return None

check_token(data=False)
check_token(data=True)

#!bash

Statements: {1, 2, 3, 4, 7, 8, 10, 11}
Multiline map: {}
<Module
    body: [
        <FunctionDef @ 1
            name: 'check_token'
            args:
                <arguments
                    args: [
                        <arg @ 1
                            arg: 'data'
                            annotation: None
                        >
                    ]
                    vararg: None
                    kwonlyargs: []
                    kw_defaults: []
                    kwarg: None
                    defaults: []
                >
            body: [
                <If @ 2
                    test:
                        <Name @ 2 id: 'data'>
                    body: [
                        <Try @ 3
                            body: [
                                <Return @ 4
                                    value:
                                        <Name @ 4 id: 'data'>
                                >
                            ]
                            handlers: []
                            orelse: []
                            finalbody: [
                                <Expr @ 7
                                    value:
                                        <Call @ 7
                                            func:
                                                <Name @ 7 id: 'print'>
                                            args: [
                                                <Str @ 7 s: 'abc'>
                                            ]
                                            keywords: []
                                        >
                                >
                            ]
                        >
                    ]
                    orelse: []
                >
                <Return @ 8
                    value:
                        <NameConstant @ 8 value: None>
                >
            ]
            decorator_list: []
            returns: None
        >
        <Expr @ 10
            value:
                <Call @ 10
                    func:
                        <Name @ 10 id: 'check_token'>
                    args: []
                    keywords: [
                        <keyword
                            arg: 'data'
                            value:
                                <NameConstant @ 10 value: False>
                        >
                    ]
                >
        >
        <Expr @ 11
            value:
                <Call @ 11
                    func:
                        <Name @ 11 id: 'check_token'>
                    args: []
                    keywords: [
                        <keyword
                            arg: 'data'
                            value:
                                <NameConstant @ 11 value: True>
                        >
                    ]
                >
        >
    ]
>

Adding arc: (-1, 1): None, None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
          _code_object__Module : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @932
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (1, 10): None, None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
          _code_object__Module : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @932
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (10, 11): None, None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
          _code_object__Module : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @932
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (11, -1): None, "didn't exit the module"
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/python.py @171
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
          _code_object__Module : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @934
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (-1, 2): None, None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
     _code_object__FunctionDef : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @943
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (2, 3): 'the condition on line {lineno} was never true', None
     _code_object__FunctionDef : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @943
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @617
                      add_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @588
                   _handle__If : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @766
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (3, 4): None, None
                   _handle__If : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @766
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @617
                      add_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @588
                  _handle__Try : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @803
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (4, 7): "the return on line {lineno} wasn't executed", None
                   _handle__If : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @766
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @617
                      add_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @588
                  _handle__Try : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @846
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (7, -1): "the return on line 4 wasn't executed", "didn't return from function 'check_token'"
                   _handle__If : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @766
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @617
                      add_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @588
                  _handle__Try : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @858
          process_return_exits : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @692
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (2, 8): 'the condition on line {lineno} was never false', None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
     _code_object__FunctionDef : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @943
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (7, 8): None, None
                          arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @264
                  _analyze_ast : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @274
                       analyze : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @515
     _code_object__FunctionDef : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @943
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @616
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521

Adding arc: (8, -1): "the return on line {lineno} wasn't executed", "didn't return from function 'check_token'"
     _code_object__FunctionDef : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @943
                 add_body_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @617
                      add_arcs : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @588
               _handle__Return : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @783
          process_return_exits : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @692
                       add_arc : /home/loic/software/coveragepy/issue-493/coverage.py/coverage/parser.py @521
Name    Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------
a.py        8      0      4      1    92%   7->8

@nedbat
Copy link
Owner Author

nedbat commented Dec 17, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


There indeed is no way for the code to go from print('abc') to return None and PythonParser::analyze must not create that arc.

@nedbat
Copy link
Owner Author

nedbat commented Dec 18, 2016

Original comment by Loic Dachary (Bitbucket: dachary, GitHub: dachary)


For the record the proposed fix is at https://bitbucket.org/ned/coveragepy/pull-requests/108/finally-happens-before-return-in-a-try-493/diff

@nedbat
Copy link
Owner Author

nedbat commented Dec 19, 2016

finally happens before return in a try #493

In a try block such as:

if expr:
try:
return
finally
print
pass

the print happens before the return and cannot be followed by
pass. The general case is that when the body/else/handlers in a try
block all have return, break etc., the code behind finally: has no arc
to the statement following the try block.

close #493

→ <<cset ac10ea1a8653 (bb)>>

@nedbat
Copy link
Owner Author

nedbat commented Dec 27, 2016

This fix was released as part of Coverage.py 4.3.

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

No branches or pull requests

1 participant