Protocol method placeholders marked as untested

[andrewlalis]

Originally posted in pytest-cov as issue 593

Describe the bug
When computing the coverage of a project, I am getting cases where the placeholder body of a protocol method is considered as untested code. I would expect that the body of protocol methods would not be marked as untested.

To Reproduce
Consider the following python code:

from typing import Protocol

class Example(Protocol):
    def do_something() -> None:
        ...

Using:

• Python version 3.11
• pytest version 7.3.0
• pytest-cov version 4.0.0
• coverage version 7.2.2

Running coverage on a project containing the above Example protocol will produce a report indicating that the line containing the ... placeholder is untested. I believe that this is incorrect, as it is impossible and useless to test the existence of a placeholder statement in a protocol's method definition.

Expected behavior
I would expect coverage to omit marking the lines in the above code snippet as untested.

Additional context
PEP 544 - Protocols

[nedbat]

I see what you mean that these lines can't be executed. But I'm not sure how coverage.py could know that? You can exclude lines from coverage measurement with the exclude_also configuration setting.

[RonnyPfannschmidt]

@nedbat perhaps a recommended pre commit hook that adds those functions / lines would be of help (or a helper to dummy invoke them in testsuites)

[nedbat]

There are other issues about coverage.py not understanding various typing constructs:

• Assume if TYPE_CHECKING: ... else: ... block is covered #831
• Coverage on overload stubs for typing #970

So perhaps it is time to add some typing patterns to the default exclude patterns.

As an experiment, does this work to exclude the Protocols in your project?

[report]
exclude_also = 
    class \w+\(Protocol\):

[RonnyPfannschmidt]

Note that protocol abc classes ought not to be excluded as they sometimes implement default implementations of

[nedbat]

(your comment is a bit truncated, but:) Can you show a small sample of code that shouldn't be excluded? This is the risk of a simplistic method like regexes for deciding things about code.

[RonnyPfannschmidt]

I believe a fair set would be methods whose body is only the ellipsis

[nedbat]

I believe a fair set would be methods whose body is only the ellipsis

Unfortunately, I think that's not possible to express with a line-based regex. Is there a way to extend the power without becoming a full-fledged AST pattern matcher?

[nedbat]

BTW, links to real-world examples of tricky Protocols would be helpful.

[RonnyPfannschmidt]

@nedbat i believe that a reasonable hackish approximation would be to just consider lines with only a ellipsis covered

[Julian]

It's slightly unfortunate to me that black or whatever other linters want the ... treated like a normal function body (and put on their own line), otherwise I think even "better" would be to limit this to :.*\.\.\.$ and to encourage method bodies to not have their own line for Protocols.

But given they do indeed put the ellipsis on their own line I guess I too agree it's probably most reasonable at the minute to do ^^.

[RonnyPfannschmidt]

@nedbat would it be sensible to ignore all single lines that contain only an ellipsis? i vaugely recall that function definitions get covered at definition time, so the ellipsis should be the only missing covered item

[Julian]

Color me ... well not shocked but certainly pleasantly surprised that

It's slightly unfortunate to me that black or whatever other linters want the ... treated like a normal function body

is now "fixed" / "improved" in Black 24 via psf/black#1797.

I'm not sure if this was closed because @nedbat WONTFIXed it (obviously all good if so) or if @andrewlalis you just closed it yourself because it was remaining open. But perhaps it's worth another consideration in light of the formatter change.