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

Coverage issue with python short-circuit for loop #593

Closed
nedbat opened this issue Jul 26, 2017 · 2 comments
Closed

Coverage issue with python short-circuit for loop #593

nedbat opened this issue Jul 26, 2017 · 2 comments
Labels
duplicate This issue or pull request already exists

Comments

@nedbat
Copy link
Owner

nedbat commented Jul 26, 2017

Originally reported by Glen Nelson (Bitbucket: glen_nelson, GitHub: Unknown)


Sample code below was run with:

  • python 3.5.2
  • coverage 4.4.1
  • nose 1.3.7

Consider the following two files:

#!python

#foo.py
def foo(items):
    for item in items:
        v1 = 5 if item > 5 else None
        v2 = item
        if not v1 or not v2:
            continue
        print(v1)
        print(v2)
#!python

#test_foo.py

from unittest import TestCase

from reproduce import foo


class TestFoo(TestCase):
    def test_foo(self):
        foo.foo([1, 2, 3, 4, 5, 6, 7])

We would expect full coverage of 'foo' here from the test. As we can see, when item is 1-5, we should hit the continue, else we should reach the prints.

Running coverage through nosetests was done with the following command

nosetests -P --with-coverage --cover-erase --cover-inclusive --cover-package reproduce --cover-html-dir=test_coverage_unit --cover-html tests/unit/reproduce

...
Name                                            Stmts   Miss  Cover
-------------------------------------------------------------------
reproduce/foo.py                                    8      1    88%

The missing line is the 'continue', due to short-circuting.


@nedbat
Copy link
Owner Author

nedbat commented Jul 26, 2017

Original comment by Glen Nelson (Bitbucket: glen_nelson, GitHub: Unknown)


The assembly:

  3           0 SETUP_LOOP              81 (to 84)
              3 LOAD_FAST                0 (items)
              6 GET_ITER
        >>    7 FOR_ITER                73 (to 83)
             10 STORE_FAST               1 (item)

  4          13 LOAD_FAST                1 (item)
             16 LOAD_CONST               1 (5)
             19 COMPARE_OP               4 (>)
             22 POP_JUMP_IF_FALSE       31
             25 LOAD_CONST               1 (5)
             28 JUMP_FORWARD             3 (to 34)
        >>   31 LOAD_CONST               0 (None)
        >>   34 STORE_FAST               2 (v1)

  5          37 LOAD_FAST                1 (item)
             40 STORE_FAST               3 (v2)

  6          43 LOAD_FAST                2 (v1)
             46 UNARY_NOT
             47 POP_JUMP_IF_TRUE         7
             50 LOAD_FAST                3 (v2)
             53 UNARY_NOT
             54 POP_JUMP_IF_FALSE       60

  7          57 JUMP_ABSOLUTE            7

  8     >>   60 LOAD_GLOBAL              0 (print)
             63 LOAD_FAST                2 (v1)
             66 CALL_FUNCTION            1 (1 positional, 0 keyword pair)
             69 POP_TOP

  9          70 LOAD_GLOBAL              0 (print)
             73 LOAD_FAST                3 (v2)
             76 CALL_FUNCTION            1 (1 positional, 0 keyword pair)
             79 POP_TOP
             80 JUMP_ABSOLUTE            7
        >>   83 POP_BLOCK
        >>   84 LOAD_CONST               0 (None)
             87 RETURN_VALUE

We see on 47, we jump directly back to 7 (the for loop iterator) if v1 is false. If instead v2 is false, we fall through the jump to 60, hitting the continue (line 57), which brings us back to line 7. It looks the coverage is only counting it if we hit line 57.

@nedbat
Copy link
Owner Author

nedbat commented Jul 27, 2017

Duplicate of #198.

@nedbat nedbat closed this as completed Jul 27, 2017
@nedbat nedbat added major bug Something isn't working labels Jun 23, 2018
@nedbat nedbat added duplicate This issue or pull request already exists and removed 4.4 bug Something isn't working labels Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant