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-7.3.3 broke pragma: no cover in match-case statement #1713

Closed
tdivis opened this issue Dec 14, 2023 · 18 comments
Closed

coverage-7.3.3 broke pragma: no cover in match-case statement #1713

tdivis opened this issue Dec 14, 2023 · 18 comments
Labels
bug Something isn't working fixed

Comments

@tdivis
Copy link

tdivis commented Dec 14, 2023

coverage-7.3.3 released few hours ago broke ignoring whole function by # pragma: no cover when the function contains match..case statement with case condition on multiple lines. My minimum example:

# code.py:
def hello():
    print("Hello world")


def hello2(a):  # pragma: no cover
    match a:
        case ("hello one"
              | "hello two"):
            print("hello1-2")
        case "hello three":
            print("hello3")
# test_code.py:
from code import hello
from unittest import TestCase


class HelloTest(TestCase):
    def test_hello(self):
        hello()

Then run coverage and you'll get report with last two match-case lines uncovered even though the whole function should be ignored.

coverage erase && coverage run --parallel-mode --source=code --branch -m unittest test_code.py && coverage combine && coverage report --show-missing
Hello world
.
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK
Combined data file .coverage.glinall.600.XyruySkx
Name      Stmts   Miss Branch BrPart  Cover   Missing
-----------------------------------------------------
code.py       4      2      2      0    33%   10-11
-----------------------------------------------------
TOTAL         4      2      2      0    33%

Worked fine in coverage-7.3.2.

@tdivis tdivis added bug Something isn't working needs triage labels Dec 14, 2023
@nedbat
Copy link
Owner

nedbat commented Dec 14, 2023

Here is a test for the test suite that demonstrates the problem:

@ tests/test_coverage.py:1743 @ def my_func_2(super_long_input_argument_0=0, super_long_input_argument_1=1, supe
               [], "5", excludes=['my_func']
           )

  +    def test_excluding_clause_bug1713(self) -> None:
  +        self.check_coverage("""\
  +            print("1")
  +
  +            def hello_3(a):  # no thanks
  +                if ("4" or
  +                    "5"):
  +                    print("6")
  +                else:
  +                    print("8")
  +
  +            print("10")
  +            """,
  +            [1, 10], "", excludes=["no thanks"],
  +        )
  +
       def test_excluding_method(self) -> None:
           self.check_coverage("""\
               class Fooey:

@kajakaj, @maciekgyver, do you have an idea how to handle this case?

woodruffw added a commit to pypa/pip-audit that referenced this issue Dec 14, 2023
See nedbat/coveragepy#1713.

Signed-off-by: William Woodruff <william@trailofbits.com>
@sigma67
Copy link

sigma67 commented Dec 15, 2023

Not sure if related, but I also have a new coverage miss on coverage 7.3.3

Code:

if condition_a:  # pragma: no cover; local only
  func1()
  if (
    condition_b
    == 0
  ):
    return
  func2()  # this statement is now shown as not covered

On prior versions, the whole if block was ignored correctly.

@dhuang
Copy link

dhuang commented Dec 15, 2023

We have seen this regression for a class as well, so seems like anything block level.

class Foo:  # pragma: no cover 
    def greet(self):
        print("hello world")

@nedbat
Copy link
Owner

nedbat commented Dec 17, 2023

Hi all, sorry for the regression. I'm working on a fix. The code uses tokenize to analyze the code, which isn't holding up well. I'm working on switching it to use the AST instead.

@freakboy3742
Copy link
Contributor

Not sure this is exactly the same problem, but in case it helps, we're also seeing a regression in the Briefcase test suite. However, we're only seeing it on Windows. I'll try to reduce this to a simple reproduction case.

@freakboy3742
Copy link
Contributor

I haven't narrowed this down to a specific reproduction case yet, but it appears to be related to an interaction with coverage-conditional-plugin. This is the coverage file for the failing run; it has 100% coverage on coverage 7.3.2, but has the 2 missing blocks of lines on coverage 7.3.3, (conditional-coverage-plugin=0.9.0), with a conditional rule of no-cover-if-is-windows = True defined.

coverage-7.3.2.zip

@ringohoffman
Copy link

I am also seeing a regression in coverage (only in CI though?) for a case like:

class MyClass:  # pragma: no cover
    def method(self):
        pass

Which was previously ignoring the lines in method(), but is now failing for missing coverage.

@freakboy3742
Copy link
Contributor

FWIW: The pattern that @ringohoffman has described is very similar to what is happening in my case; however, I haven't been able to generate a simple reproduction case that fails.

In case it helps someone else: in my case, the no cover is conditional on a sys.platform check. The failure only occurs on Windows; the coverage reports missing lines on Windows, as well as when the Windows report is combined with a Linux and macOS report (Linux and macOS both do have coverage of the given lines).

The code that is being mistakenly marked as a coverage miss is:

  • a method in a class where the entire class is marked no-cover
  • 2 context manager blocks one after another inside a method of a class, where the method is marked no-cover.

The second one is particularly odd, because there's plenty of other lines in the method (both before and after the problematic context managers) that are marked as covered, as well as other context managers in the same method.

woodruffw added a commit to pypa/pip-audit that referenced this issue Dec 19, 2023
* _virtual_env: add --no-input to all invocations

Closes #706.

Signed-off-by: William Woodruff <william@trailofbits.com>

* CHANGELOG: record changes

Signed-off-by: William Woodruff <william@trailofbits.com>

* pyproject: filter coverage==7.3.2

See nedbat/coveragepy#1713.

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
@sigma67
Copy link

sigma67 commented Dec 19, 2023

We are also combining Linux and Windows reports, so the bug could indeed be related to Windows.

Although as you can see from my example above it doesn't have to be a class, it can be any kind of nested block (an if statement in my case).

@tdivis
Copy link
Author

tdivis commented Dec 19, 2023

Just to make it clear, the case in the issue description was run on Linux.

nedbat added a commit that referenced this issue Dec 19, 2023
@mmrzyglockitivo
Copy link

mmrzyglockitivo commented Dec 19, 2023

Another occurrence, which was quite confusing: I have several classes in one file, inheriting after abstract and all individually marked with pragma: no cover, yet several classes did not trigger anything, one triggered errors for all its @property, while two classes later one method was included in coverage with __init__ and @property elements of the said class ignored as expected.

Go figure. Code is in private repository, but I can look for potential triggers of this behaviour, if directed.


The first class has multiline definition of __init__, the final error comes from a method that has multiline for, BUT the preceding method has multiline for as well, so maybe the first is used as a trigger, the other gets the error? Only these two multiline for are in this file and one multiline __init__ declaration.


My case, somewhat simplified:

class BreakingInit:  # pragma: no cover

    def __init__(
        self, absurdly_long_argument_name_to_break_definition_here: int, another_long_argument_name: str
    ) -> None:
        self._number = absurdly_long_argument_name_to_break_definition_here
        self._string = another_long_argument_name

    @property
    def number(self) -> int:
        return self._number

    @property
    def string(self) -> str:
        return self._string


class BreakingFor:  # pragma: no cover

    def __init__(self, just_an_argument: str) -> None:
        self._string = just_an_argument

    def get_all_spaces(self) -> str:
        spaces = ""
        for number, character in enumerate(
            self._string
        ):
            if character == " ":
                spaces += character
        return spaces

    def get_all_non_spaces(self) -> str:
        non_spaces = ""
        for number, character in enumerate(
            self._string
        ):
            if character != " ":
                non_spaces += character
        return non_spaces

Results:

  • BreakingInit first: 9-39 (starts after __init__)
  • BreakingFor first: 13-39 (starts on the first instruction after for)

No idea how to reproduce the "fix" yet (as in, how to add methods that are still excluded from the final coverage and in between erroneous blocks); I tried adding another class in between, did not change behaviour to the one observed.

@nedbat
Copy link
Owner

nedbat commented Dec 19, 2023

I've made a simple fix on the nedbat/bug1713 branch. BUT: only one of the examples shown in the comments here (the match/case in the original description) was broken for me in 7.3.3. I don't see why there would be an OS sensitivity in either the original bug or the fix.

To try the fixed code:

python3 -m pip install git+https://github.com/nedbat/coveragepy@e406af2ac00dfcf7f#egg=coverage==0.0

Please let me know what you find.

@freakboy3742
Copy link
Contributor

@nedbat Can confirm that branch seems to address the problem for me. Thanks for the fast turnaround under very obscure reproduction circumstances :-)

@nedbat
Copy link
Owner

nedbat commented Dec 20, 2023

@nedbat Can confirm that branch seems to address the problem for me. Thanks for the fast turnaround under very obscure reproduction circumstances :-)

Thanks for the confirmation, but it kind of just deepens the mystery...

@sigma67
Copy link

sigma67 commented Dec 20, 2023

I can confirm this fixes the issue for my reported sample (e406af2)

@nedbat
Copy link
Owner

nedbat commented Dec 20, 2023

This is fixed in commit 07b76b2.

@nedbat nedbat closed this as completed Dec 20, 2023
@nedbat nedbat added the fixed label Dec 20, 2023
@tdivis
Copy link
Author

tdivis commented Dec 20, 2023

I can confirm that the match..case from which I made the minimal example for the issue description works OK on master. Thanks for the fix :-).

@nedbat
Copy link
Owner

nedbat commented Dec 20, 2023

This is now released as part of coverage 7.3.4.

renovate bot added a commit to allenporter/flux-local that referenced this issue Dec 24, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [coverage](https://togithub.com/nedbat/coveragepy) | `==7.3.3` ->
`==7.3.4` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/coverage/7.3.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/coverage/7.3.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/coverage/7.3.3/7.3.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/coverage/7.3.3/7.3.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>nedbat/coveragepy (coverage)</summary>

###
[`v7.3.4`](https://togithub.com/nedbat/coveragepy/blob/HEAD/CHANGES.rst#Version-734--2023-12-20)

[Compare
Source](https://togithub.com/nedbat/coveragepy/compare/7.3.3...7.3.4)

- Fix: the change for multi-line signature exclusions in 7.3.3 broke
other
forms of nested clauses being excluded properly. This is now fixed,
closing
    `issue 1713`\_.

- Fix: in the HTML report, selecting code for copying won't select the
line
    numbers also. Thanks, `Robert Harris <pull 1717_>`\_.

.. \_issue
1713:[nedbat/coveragepy#1713
.. \_pull
1717[nedbat/coveragepy#1717

.. \_changes\_7-3-3:

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMDMuMSIsInVwZGF0ZWRJblZlciI6IjM3LjEwMy4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

7 participants