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

Ignore ABC abstract methods #1131

Closed
saroad2 opened this issue Mar 2, 2021 · 14 comments
Closed

Ignore ABC abstract methods #1131

saroad2 opened this issue Mar 2, 2021 · 14 comments
Labels
docs enhancement New feature or request fixed

Comments

@saroad2
Copy link

saroad2 commented Mar 2, 2021

Is your feature request related to a problem? Please describe.
Sometimes you want to create an abstract class with abstract methods that will be overridden by other subclasses. For example:

import abc

class Foo(abc.ABC):
    @abc.abstractmethod
    def a():
        ...

class Bar(Foo):
    def a():
        return 1

Pay attention that while Bar.a content should be covered by test, the content of Foo.a should not since there is no actual gain in covering it with a test.

Describe the solution you'd like
Coverage should ignore abstract methods by default and should not require coverage for them.

Describe alternatives you've considered
If you think that sometimes abstract methods should be covered, maybe we can make this ability toggleable by a flag (--ignore-abstract) for example.

Additional context
This API is available only in Python 3, therefore we should make sure we disable it in Python 2.

@saroad2 saroad2 added the enhancement New feature or request label Mar 2, 2021
@nedbat
Copy link
Owner

nedbat commented Mar 2, 2021

Will it work for you to update your .coveragerc file to exclude those methods? Like this:

[report]
exclude_lines:
    pragma: no cover
    @abc.abstractmethod

@saroad2
Copy link
Author

saroad2 commented Mar 2, 2021

No, but the following might will:

[report]
exclude_lines:
    pragma: no cover
    @abc.abstractmethod
    ...

I'll check it.

Even though it is a solid solution, I think that this case is general enough to be handled directly with a flag or a specific configuration.
What do you think?

@nedbat
Copy link
Owner

nedbat commented Mar 2, 2021

You don't want to add "...", that's a regex that matches almost every line of your code (se #1042). Try the setting I suggested, I think it will work for you.

I'm reluctant to add hard-coded options for language features, because different people approach them differently, and the language changes over time.

@nedbat
Copy link
Owner

nedbat commented Mar 4, 2021

I don't know if it would be helpful to add to the default excluded lines pattern?

@ktbarrett
Copy link

Another way to do it might be to automatically ignore "trivial" function bodies like mypy already does. "Trivial" function bodies, according to mypy, have a single pass, ..., or raise NotImplementedError statement. This may be possible with a regex, but making rules that handle formatting, comments, etc might be difficult. mypy operates on AST, which makes the implementation of detecting trivial functions trivial 😏.

@nedbat
Copy link
Owner

nedbat commented Jul 19, 2021

Maybe we could add something to the docs that suggest these sorts of regexes? I think regexes in the configuration file are powerful enough for what people need to do.

@ktbarrett
Copy link

#894 is blocking that.

@nedbat
Copy link
Owner

nedbat commented Jul 21, 2021

@ktbarrett can you show me some of the code you are working with, and what you would like to do?

@ktbarrett
Copy link

I'd like to ignore the bodies of abstract functions.

@abstractmethod
def method(self, arg: int) -> str:
    """docstring with code examples in it"""
    pass

Adding just pass to exclude_lines means I exclude things like

try:
    return may_fail()
except Exception:
    pass
return NotImplemented

I thought for a second I could write a regex that could detect such cases, but I now realize that isn't really reasonable/possible.

I've adjusted my current strategy to:

  • add raise NotImplementedError and ... to exclude_lines
  • by convention use only those statements for type stubs and abstractmethod bodies and not pass
  • by convention never use those statements in executable code, but I can use pass
  • update everything that currently does not follow this convention

@nedbat
Copy link
Owner

nedbat commented Jul 22, 2021

There's something people are missing in this discussion: since coverage.py v4.1, if you exclude a decorator, it excludes the entire decorated function. So the best exclude line here would be:

[report]
exclude_lines =
    @abstractmethod
    @abc.abstractmethod

Once you do that, the entire abstract method is excluded from coverage, no matter what statements it contains.

BTW, this is also being discussed in pytest-cov: pytest-dev/pytest-cov#428

@nedbat
Copy link
Owner

nedbat commented Jul 22, 2021

Also, I see that this behavior of excluding a decorator isn't mentioned in the docs! I will fix that.

@ktbarrett
Copy link

There is the possibility that abstractmethods could have behavior in them that is intended to be run with a super().method() call. I think it's kind of atypical, but possible. However, the decorator solution works for me because I don't do that.

@ktbarrett
Copy link

ktbarrett commented Jul 22, 2021

Also doesn't cover setters of abstractpropertys. I unfortunately have to use abstractproperty because mypy does not like layering property and abstractmethod. But it will be covered by the raise\s+NotImplementedError rule.

If someone were interested I would add the following to your exclude_lines:

    @(?:abc\.)?abstract(?:method|property|classmethod|staticmethod)
    @(?:typing\.)?overload

@nedbat
Copy link
Owner

nedbat commented Jul 25, 2021

I've added to the docs in c6d9eb9.

@nedbat nedbat closed this as completed Jul 25, 2021
1kastner added a commit to 1kastner/conflowgen that referenced this issue Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement New feature or request fixed
Projects
None yet
Development

No branches or pull requests

3 participants