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 not showing coverage for continue after assert #1029

Closed
asmeurer opened this issue Sep 3, 2020 · 4 comments
Closed

Coverage not showing coverage for continue after assert #1029

asmeurer opened this issue Sep 3, 2020 · 4 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists

Comments

@asmeurer
Copy link

asmeurer commented Sep 3, 2020

Describe the bug
A clear and concise description of the bug.

Example script

def test():
    for i in range(10):
        if i == 1:
            assert i == 1
            continue


test()
$ coverage run test.py
$ coverage report -m
Name      Stmts   Miss  Cover   Missing
---------------------------------------
test.py       6      1    83%   5
Coverage failure: total of 83 is less than fail-under=100

It thinks the continue line is not covered. If I remove the assert, or put something like print("Covered") between the assert and continue, it works.

To Reproduce
Python 3.8.5 and Coverage 5.2.1

@asmeurer asmeurer added the bug Something isn't working label Sep 3, 2020
@asmeurer
Copy link
Author

asmeurer commented Sep 3, 2020

Actually, if you put any kind of no-op between the assert and continue like a pass or an expression, it still isn't covered. It also doesn't work to put

if True:
    continue

Presumably anything that gets optimized away by the bytecode compiler doesn't work. The simplest workaround I found is to put a name there, like

assert i == 1
i
continue

since that triggers a namespace lookup.

@nedbat
Copy link
Owner

nedbat commented Sep 3, 2020

This looks like a duplicate of #198?

@asmeurer
Copy link
Author

asmeurer commented Sep 3, 2020

It definitely looks similar. You'd have to make the call on whether it's the same because I don't know the technical details. My understanding there is that the optimizer optimizes out the continue. I'm not sure I understand that, because this happens even in cases where removing the continue results in different logic. In my actual code, I have more code below the if block with the continue in it. Something like

 def test():
    for i in range(10):
        if i == 1:
            assert i == 1
            continue
        print(i)


test()

still exhibits the problem. The bytecode seems similar enough with and without the assert. Both use JUMP_ABSOLUTE for the continue. But I don't really know how coverage works, so maybe this isn't the right thing to look at.

@asottile
Copy link
Contributor

asottile commented Oct 4, 2020

yeah the continue is optimized out (dupe of #198):

the primary difference is instruction 26 the POP_JUMP_IF_TRUE skips the continue entirely and goes straight to the top of the loop

>>> def test():
...     for i in range(10):
...         if i == 1:
...             assert i == 1
...             continue
... 
>>> def test2():
...     for i in range(10):
...         if i == 1:
...             assert i == 1
...             i
...             continue
... 
>>> import dis
>>> dis.dis(test)
  2           0 LOAD_GLOBAL              0 (range)
              2 LOAD_CONST               1 (10)
              4 CALL_FUNCTION            1
              6 GET_ITER
        >>    8 FOR_ITER                26 (to 36)
             10 STORE_FAST               0 (i)

  3          12 LOAD_FAST                0 (i)
             14 LOAD_CONST               2 (1)
             16 COMPARE_OP               2 (==)
             18 POP_JUMP_IF_FALSE        8

  4          20 LOAD_FAST                0 (i)
             22 LOAD_CONST               2 (1)
             24 COMPARE_OP               2 (==)
             26 POP_JUMP_IF_TRUE         8
             28 LOAD_GLOBAL              1 (AssertionError)
             30 RAISE_VARARGS            1

  5          32 JUMP_ABSOLUTE            8
             34 JUMP_ABSOLUTE            8
        >>   36 LOAD_CONST               0 (None)
             38 RETURN_VALUE
>>> dis.dis(test2)
  2           0 LOAD_GLOBAL              0 (range)
              2 LOAD_CONST               1 (10)
              4 CALL_FUNCTION            1
              6 GET_ITER
        >>    8 FOR_ITER                30 (to 40)
             10 STORE_FAST               0 (i)

  3          12 LOAD_FAST                0 (i)
             14 LOAD_CONST               2 (1)
             16 COMPARE_OP               2 (==)
             18 POP_JUMP_IF_FALSE        8

  4          20 LOAD_FAST                0 (i)
             22 LOAD_CONST               2 (1)
             24 COMPARE_OP               2 (==)
             26 POP_JUMP_IF_TRUE        32
             28 LOAD_GLOBAL              1 (AssertionError)
             30 RAISE_VARARGS            1

  5     >>   32 LOAD_FAST                0 (i)
             34 POP_TOP

  6          36 JUMP_ABSOLUTE            8
             38 JUMP_ABSOLUTE            8
        >>   40 LOAD_CONST               0 (None)
             42 RETURN_VALUE

@nedbat nedbat closed this as completed Mar 14, 2021
@nedbat nedbat added the duplicate This issue or pull request already exists label Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants