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

Regression in branch coverage using Coverage 7.6.3 on Python 3.12+ #1880

Closed
freakboy3742 opened this issue Oct 15, 2024 · 13 comments
Closed
Labels
bug Something isn't working fixed

Comments

@freakboy3742
Copy link
Contributor

Describe the bug

Coverage 7.6.3 appears to have introduced a regression in the calculation of branch coverage related to the handling of while loops inside context managers.

To Reproduce
How can we reproduce the problem? Please be specific. Don't link to a failing CI job. Answer the questions below:

  1. What version of Python are you using?

Observed using 3.12.7 and 3.13.0. It doesn't appear in 3.11 or earlier.

  1. What version of coverage.py shows the problem? The output of coverage debug sys is helpful.

7.6.3. The problem doesn't exist in 7.6.1.

  1. What versions of what packages do you have installed? The output of pip freeze is helpful.
  2. What code shows the problem? Give us a specific commit of a specific repo that we can check out. If you've already worked around the problem, please provide a commit before that fix.

The problem has been found in this dependabot update on Briefcase, which bumps coverage.py from 7.6.1 to 7.6.3.

  1. What commands should we run to reproduce the problem? Be specific. Include everything, even git clone, pip install, and so on. Explain like we're five!

$ git clone [email protected]:beeware/briefcase.git
$ cd briefcase
$ git checkout -b coverage-7.6.3 origin/dependabot/pip/coverage-toml--7.6.3
$ python3.12 -m venv venv
$ source venv/bin/activate
(venv) $ pip install tox
$ tox -m test312

This will run the full test suite, reporting 2 branch coverage misses:

...
coverage312: commands[1]> python -m coverage report --fail-under=100
Name                                   Stmts   Miss Branch BrPart  Cover   Missing
----------------------------------------------------------------------------------
src/briefcase/platforms/iOS/xcode.py     219      0     48      2  99.3%   525->528, 541->544
----------------------------------------------------------------------------------
TOTAL                                   7084      0   1598      2  99.9%

67 files skipped due to complete coverage.

Expected behavior

If you omit the git checkout -b ... line, the main branch uses 7.6.1, and shows 100% coverage.

...
coverage312: commands[1]> python -m coverage report --fail-under=100
Name    Stmts   Miss Branch BrPart   Cover   Missing
----------------------------------------------------
TOTAL    7104      0   2354      0  100.0%

68 files skipped due to complete coverage.

Additional context

It seems likely this is related to #1876, and commit 378c321.

Both of the new "missing" branches follow the same basic pattern of a while loop with a walrus operator, inside a context manager:

https://github.com/beeware/briefcase/blob/563c6acd7b0875ee3f8e808c70065a10c077ed05/src/briefcase/platforms/iOS/xcode.py#L525-L528

https://github.com/beeware/briefcase/blob/563c6acd7b0875ee3f8e808c70065a10c077ed05/src/briefcase/platforms/iOS/xcode.py#L541-L544

Unfortunately, I haven't had any luck reducing this to a simpler example case.

@freakboy3742 freakboy3742 added the bug Something isn't working label Oct 15, 2024
@nedbat
Copy link
Owner

nedbat commented Oct 19, 2024

Thanks, I can reproduce it. It seems to have been introduced with 7.6.2.

@udifuchs
Copy link

I have a very similar issue. False positive in a for loop inside a context manager. coverage 7.6.1 worked correctly. 7.6.2 and 7.6.3 fail.

The difference is that it shows up with python 3.13, but not with python 3.12.

The bug shows up in pylint-silent branch coverage-branching:

https://github.com/udifuchs/pylint-silent/tree/coverage-branching

And can be reproduced using:

tox -e py313-pytest

@nedbat
Copy link
Owner

nedbat commented Oct 20, 2024

@udifuchs Thanks, but I'm not sure I have enough information to reproduce. When I clone your repo and checkout that branch, I get two test failures:

FAILED tests/test_samples.py::test_apply - AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py tests/sAmple_1_after_apply.py
FAILED tests/test_samples.py::test_apply_signature - AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py tests/sAmple_1_after...
Full output
% tox -e py313-pytest
py313-pytest: install_deps> python -I -m pip install coverage 'pylint>=2.16' pytest
.pkg: install_requires> python -I -m pip install 'setuptools>=60' wheel
.pkg: _optional_hooks> python /System/Volumes/Data/src/bugs/bug1880/briefcase/venv/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /System/Volumes/Data/src/bugs/bug1880/briefcase/venv/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_sdist> python /System/Volumes/Data/src/bugs/bug1880/briefcase/venv/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta
py313-pytest: install_package> python -I -m pip install --force-reinstall --no-deps /System/Volumes/Data/src/bugs/bug1880/pylint-silent/.tox/.tmp/package/1/pylint_silent-1.4.1.tar.gz
py313-pytest: commands[0]> coverage run -m pytest tests
====================================================================== test session starts =======================================================================
platform darwin -- Python 3.13.0, pytest-8.3.3, pluggy-1.5.0
cachedir: .tox/py313-pytest/.pytest_cache
rootdir: /System/Volumes/Data/src/bugs/bug1880/pylint-silent
configfile: pyproject.toml
collected 9 items

tests/test_samples.py ...F.F...                                                                                                                            [100%]

============================================================================ FAILURES ============================================================================
___________________________________________________________________________ test_apply ___________________________________________________________________________

ctx = <tests.test_samples.Context object at 0x10298e900>

    def test_apply(ctx: Context) -> None:
        """Test 'pylint-silent apply'."""
        # Run pylint on test files.
        pylint_output = ctx.temp_sample_filename + "lint"
        ctx.run_pylint_to_file(pylint_output)

        # Apply pylint-silent changed based on output from pylint.
        run_pylint_silent("apply", "--max-line-length=70", pylint_output)

        # Test that the expected python file was generated.
>       assert_files_equal(ctx.temp_sample_filename, ctx.sample_after_apply)

tests/test_samples.py:127:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

file_1 = '/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py', file_2 = 'tests/sAmple_1_after_apply.py'

    def assert_files_equal(file_1: str, file_2: str) -> None:
        """Check that both file are the same and have the same permissions."""
>       assert filecmp.cmp(file_1, file_2), f"diff {file_1} {file_2}"
E       AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py tests/sAmple_1_after_apply.py
E       assert False
E        +  where False = <function cmp at 0x101e8f7e0>('/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py', 'tests/sAmple_1_after_apply.py')
E        +    where <function cmp at 0x101e8f7e0> = filecmp.cmp

tests/test_samples.py:87: AssertionError
---------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------
************* Module sAmple_1
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:1:0: C0302: Too many lines in module (19/10) (too-many-lines)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:1:0: C0114: Missing module docstring (missing-module-docstring)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:1:0: C0103: Module name "sAmple_1" doesn't conform to snake_case naming style (invalid-name)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:10:0: C0116: Missing function or method docstring (missing-function-docstring)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:16:11: W0718: Catching too general exception BaseException (broad-exception-caught)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:15:14: W0123: Use of eval (eval-used)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:19:4: W0602: Using global for 'VAR' but no assignment is done (global-variable-not-assigned)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:10:9: W0613: Unused argument 'name' (unused-argument)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:15:8: W0612: Unused variable 'val' (unused-variable)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:3:0: W0611: Unused import os (unused-import)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py:4:0: W0611: Unused import sys (unused-import)
************* Module sAmple_1_after_apply
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1_after_apply.py:1:0: R0801: Similar lines in 2 files
==sAmple_1:[10:19]
==sAmple_1_after_apply:[15:25]
    try:
        # pylint: disable=exec-used
        exec("1 + 1")
        # pylint: enable=exec-used
        # pylint: disable-next=eval-used,unused-variable
        val = eval("2 + 2")
    except BaseException:  # pylint: disable=broad-exception-caught
        pass

    global VAR  # pylint: disable=global-variable-not-assigned (duplicate-code)

-----------------------------------
Your code has been rated at 4.00/10

______________________________________________________________________ test_apply_signature ______________________________________________________________________

ctx = <tests.test_samples.Context object at 0x1029f4690>

    def test_apply_signature(ctx: Context) -> None:
        """Test 'pylint-silent apply'."""
        # Run pylint on test files.
        pylint_output = ctx.temp_sample_filename + "lint"
        ctx.run_pylint_to_file(pylint_output)

        # Apply pylint-silent changed based on output from pylint.
        run_pylint_silent("apply", "--signature", pylint_output)

        # Test that the expected python file was generated.
>       assert_files_equal(ctx.temp_sample_filename, ctx.sample_after_apply_w_sig)

tests/test_samples.py:155:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

file_1 = '/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py'
file_2 = 'tests/sAmple_1_after_apply_w_signature.py'

    def assert_files_equal(file_1: str, file_2: str) -> None:
        """Check that both file are the same and have the same permissions."""
>       assert filecmp.cmp(file_1, file_2), f"diff {file_1} {file_2}"
E       AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py tests/sAmple_1_after_apply_w_signature.py
E       assert False
E        +  where False = <function cmp at 0x101e8f7e0>('/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py', 'tests/sAmple_1_after_apply_w_signature.py')
E        +    where <function cmp at 0x101e8f7e0> = filecmp.cmp

tests/test_samples.py:87: AssertionError
---------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------
************* Module sAmple_1
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:1:0: C0302: Too many lines in module (19/10) (too-many-lines)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:1:0: C0114: Missing module docstring (missing-module-docstring)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:1:0: C0103: Module name "sAmple_1" doesn't conform to snake_case naming style (invalid-name)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:10:0: C0116: Missing function or method docstring (missing-function-docstring)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:16:11: W0718: Catching too general exception BaseException (broad-exception-caught)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:15:14: W0123: Use of eval (eval-used)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:19:4: W0602: Using global for 'VAR' but no assignment is done (global-variable-not-assigned)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:10:9: W0613: Unused argument 'name' (unused-argument)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:15:8: W0612: Unused variable 'val' (unused-variable)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:3:0: W0611: Unused import os (unused-import)
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py:4:0: W0611: Unused import sys (unused-import)
************* Module sAmple_1_after_apply
/private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1_after_apply.py:1:0: R0801: Similar lines in 2 files
==sAmple_1:[10:19]
==sAmple_1_after_apply:[15:25]
    try:
        # pylint: disable=exec-used
        exec("1 + 1")
        # pylint: enable=exec-used
        # pylint: disable-next=eval-used,unused-variable
        val = eval("2 + 2")
    except BaseException:  # pylint: disable=broad-exception-caught
        pass

    global VAR  # pylint: disable=global-variable-not-assigned (duplicate-code)

------------------------------------------------------------------
Your code has been rated at 4.00/10 (previous run: 4.00/10, +0.00)

==================================================================== short test summary info =====================================================================
FAILED tests/test_samples.py::test_apply - AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply0/sAmple_1.py tests/sAmple_1_after_apply.py
FAILED tests/test_samples.py::test_apply_signature - AssertionError: diff /private/var/folders/6j/khn0mcrj35d1k3yylpl8zl080000gn/T/pytest-of-ned/pytest-109/test_apply_signature0/sAmple_1.py tests/sAmple_1_after...
================================================================== 2 failed, 7 passed in 1.07s ===================================================================
py313-pytest: exit 1 (3.00 seconds) /System/Volumes/Data/src/bugs/bug1880/pylint-silent> coverage run -m pytest tests pid=42139
  py313-pytest: FAIL code 1 (11.33=setup[8.32]+cmd[3.00] seconds)
  evaluation failed :( (11.38 seconds)

@nedbat
Copy link
Owner

nedbat commented Oct 20, 2024

The walrus was a different animal: a red herring. The key here was the grouped with syntax:

from contextlib import nullcontext
import random


rolled = set()

with nullcontext() as x:
    while random.random() < .99:
        rolled.add(1)

print("Done with 1")

with (
    nullcontext() as x,
):
    while random.random() < .99:    # line 16
        rolled.add(2)

print("Done with 2")                # line 19

with (
    nullcontext() as x,
    nullcontext() as y,
):
    while random.random() < .99:    # line 25
        rolled.add(3)

print("Done with 3")                # line 28

assert rolled == {1, 2, 3}

Running this produces this report:

Name        Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------
simple.py      16      0      6      2    91%   16->19, 25->28
-------------------------------------------------------
TOTAL          16      0      6      2    91%

Even having just one context manager, the parenthesized syntax causes the problem... Now to debug.

@nedbat
Copy link
Owner

nedbat commented Oct 20, 2024

Fixed in commit b8c236a.

@nedbat nedbat closed this as completed Oct 20, 2024
@nedbat nedbat added the fixed label Oct 20, 2024
@freakboy3742
Copy link
Contributor Author

Bah! What have parentheses done for us lately? :-P

Thanks for the fix, @nedbat.

@nedbat
Copy link
Owner

nedbat commented Oct 20, 2024

This is now released as part of coverage 7.6.4.

@udifuchs
Copy link

I noticed that you are using macos. Apparently, my tests did not work on macos. Here is a branch that fixes that:
https://github.com/udifuchs/pylint-silent/tree/refs/heads/macos-threads

I don't have access to macos, but here is a successful run with github actions:
https://github.com/udifuchs/pylint-silent/actions/runs/11432810880

But the real issue is that this bug still shows up with python 3.12.3. The tests in coverage.py fail when run with python 3.12.3.

@nedbat
Copy link
Owner

nedbat commented Oct 21, 2024

@udifuchs is your problem fixed by coverage 7.6.4?

@udifuchs
Copy link

@udifuchs is your problem fixed by coverage 7.6.4?

No. With coverage 7.6.4 python 3.12.3 still has the same bug. It is reproduced here:

https://github.com/udifuchs/pylint-silent/actions/runs/11432842782

It can also be reproduced by running coveragepy test with python 3.12.3.

@nedbat
Copy link
Owner

nedbat commented Oct 21, 2024

I see what you mean. 3.12.3 shows that problem. So do 3.12.4 and 3.12.5. But 3.12.6 and 3.12.7 do not. Can you upgrade your Python?

@nedbat
Copy link
Owner

nedbat commented Oct 22, 2024

I updated coverage.py to decide between 3.12.5 and 3.12.6, so it will work properly with 3.12.3 now. But it hasn't been released yet. Install from git to try it out.

@udifuchs
Copy link

I installed from git coveragepy version 7.6.5a0.dev1 and it works correctly with python 3.12.3.

The reason I am using python 3.12.3 is that this is the packaged version in Ubuntu 24.04.

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

No branches or pull requests

3 participants