pytest-dev
Incomplete abstract class no longer logs an error
We have this test in aiohttp:
def test_testcase_no_app(
testdir: pytest.Testdir, loop: asyncio.AbstractEventLoop
) -> None:
testdir.makepyfile(
"""
from aiohttp.test_utils import AioHTTPTestCase
class InvalidTestCase(AioHTTPTestCase):
def test_noop(self) -> None:
pass
"""
)
result = testdir.runpytest()
result.stdout.fnmatch_lines(["*TypeError*"])
This has started failing sometime after 8.1.1.
The log seems to successfully run, rather than erroring.
Output from the test:
FAILED tests/test_test_utils.py::test_testcase_no_app - Failed: nomatch: '*TypeError*'
and: '============================= test session starts =============================='
and: 'platform darwin -- Python 3.11.9, pytest-8.4.1, pluggy-1.6.0'
and: 'codspeed: 3.2.0 (disabled, mode: walltime, timer_resolution: 41.7ns)'
and: 'rootdir: /private/var/folders/y6/nj790rtn62lfktb1sh__79hc0000gn/T/pytest-of-runner/pytest-0/popen-gw1/test_testcase_no_app0'
and: 'plugins: xdist-3.7.0, cov-6.2.1, mock-3.14.1, codspeed-3.2.0'
and: 'collected 0 items'
and: ''
and: '============================ no tests ran in 0.01s ============================='
remains unmatched: '*TypeError*'
AiohttpTestCase is defined like:
class AioHTTPTestCase(IsolatedAsyncioTestCase, ABC):
@abstractmethod
async def get_application(self) -> Application:
"""Get application.
This method should be overridden to return the aiohttp.web.Application
object to test.
"""
...
Standalone reproducer:
import pytest
def test_no_app(testdir: pytest.Testdir) -> None:
testdir.makepyfile(
"""
from abc import ABC, abstractmethod
class TestBase(ABC):
@abstractmethod
async def get_application(self):
...
class TestInvalid(TestBase):
def test_noop(self) -> None:
pass
"""
)
result = testdir.runpytest()
result.stdout.fnmatch_lines(["*TypeError*"])
bisected to 37489d3:
- #12318
- #12275
Also see 1a5e0eb which originally introduced that regression.
The example above:
- Passes (i.e. raises
TypeError) before 1a5e0eb - Still passes at 1a5e0eb (#12089)
- Fails at 37489d3c6c81bc3f1badf7c50e568d4fffe6deb9 (#12318)
I'm a bit confused by the tests added in #12318, because those expect
import abc
import unittest
class TestBase(unittest.TestCase, abc.ABC):
@abc.abstractmethod
def abstract1(self):
pass
@abc.abstractmethod
def abstract2(self):
pass
def test_it(self):
pass
class TestPartial(TestBase):
def abstract1(self):
pass
class TestConcrete(TestPartial):
def abstract2(self):
pass
to pass, but when running that example with unittest (Python 3.13), it fails too:
File "/usr/lib/python3.13/unittest/suite.py", line 57, in addTests
for test in tests:
^^^^^
TypeError: Can't instantiate abstract class TestBase without an implementation for abstract methods 'abstract1', 'abstract2'
I'm guessing that the changes in pytest have made it so that an incomplete abstract class is not collected, whereas I assume that unittest collects any class with a test_ method defined.
So, I suspect that the test_it method in that above test should be removed (or rather, moved to the subclasses), and the behaviour tweaked to actually collect any unittest class with a test defined, regardless of being abstract.
It seems good to me that pytest skip an incomplete ABC. It is still abstract and thus meant as a base class only, not a test iteself. If unittest does run it, perhaps change the test to run the test file using unittest? (testdir supports running custom commands, not just pytest)
we should add a warning for abcs that match test names - after all if a upstream adds a new abstract method, tests can go dark
It is still abstract and thus meant as a base class only, not a test iteself.
Then why does it have a test defined?
The only difference I see here, is that if a class has a test defined, then unittest collects it and ends up raising a TypeError, thus highlighting the user's mistake. While pytest ignores it and doesn't highlight the mistake to a user.
If a class is abstract and doesn't have a test defined, then both behave the same way..
@Dreamsorcerer an abstract class cannot be instantiated - they are commonly used to create templates, where the subclass fills in the blanks
we need to ensure 2 things a) warn when a direct abc is not marked as not a test but discovered as a test (as in that ase user intent is unclear b) error when a subclass of a abc doesn’t fill in all the blanks . if a class is a not a abc, then it ought to fill in everything needed to be a instance
Then why does it have a test defined?
As @RonnyPfannschmidt said, it's a common technique in unittest to create a base test class with test cases, and then have multiple concrete test classes which fill in different data for the same test code (sort of parametrization). It's also possible in pytest as a "pure python" alternative to fixtures/parametrize.
a) warn when a direct abc is not marked as not a test but discovered as a test (as in that ase user intent is unclear
By "marked as not a test" do you mean __test__ = False? The __test__ attribute is inherited so that's not suitable in this case.
b) error when a subclass of a abc doesn’t fill in all the blanks . if a class is a not a abc, then it ought to fill in everything needed to be a instance
The Python semantics is that a subclass of an abc which doesn't fill all blanks is itself an abc, as determined by inheritance rules and inspect.isabstract. We shouldn't go against that IMO - once we decided pytest should skip ABCs, then it should follow the Python semantics.
a key issue i see is that we silently skip them - and a user error of missing a new abstract method can turn tests dark i propose we keep not collecting on abstract classes, but instead of not reporting them at all we report them as collection skip + document a correct way to mark test base classes as not a test ( and perhaps actually raising an error if a abstract class has a base class thats marked as not a test)
As @RonnyPfannschmidt said, it's a common technique in unittest to create a base test class with test cases
The comment above (https://github.com/pytest-dev/pytest/issues/13546#issuecomment-3008272577) says that unittest raises a TypeError if you do this, so I'm not convinced this is correct.
@Dreamsorcerer an abstract class cannot be instantiated - they are commonly used to create templates, where the subclass fills in the blanks
If we accept that a unittest abstract class cannot have a test defined, then my point still stands. i.e. Our abstract class is a template that must be filled, but it doesn't define any tests itself.
b) error when a subclass of a abc doesn’t fill in all the blanks . if a class is a not a abc, then it ought to fill in everything needed to be a instance
As already mentioned, there can be multiple layers of abstract classes, so I don't think this makes sense. It should only be based on whether tests are defined in the class.
pytest cannot know if a abc that doesn’t fill all the blanks was a victim of a missed breaking change in a base class or if it is a actual new abc
the only way to know is to set up something for pytest to explicitly be able to tell, so the error/warning is in fact a workaround for unclear data in the source
im udner the impression the test attribute can function for that else we need something more clear
Or the presence of any test function... Is there a reason to explicitly behave differently to unittest in this regard? As a user, I'd expect unittest tests to work approximately the same as running them under unittest (i.e. they should be portable)..
I created https://github.com/pytest-dev/pytest/issues/14003 before I found this issue, with a minimal example. I think a user expects tests to fail and not be silently ignored, where the names of the classes and tests otherwise indicate that they would be collected.
This happened in a project I'm involved in when an abstract method was added to a base class with tests that were inherited to multiple test classes, some of which forgot to implement the abstract method. We accidentally detected that a bunch of tests stopped running (silently).
I would like to request that test classes that fail to instantiate would be reported as errors.
For instance, if a constructor is defined, it results in a collection warning:
E pytest.PytestCollectionWarning: cannot collect test class 'TestFoo' because it has a __init__ constructor (from: foo.py)
There could be a similar warning.
We accidentally detected that a bunch of tests stopped running (silently).
Unrelated to the actual issue, but as a tip, this is why we track coverage of tests, so we catch test code that's not running in our tests.
We accidentally detected that a bunch of tests stopped running (silently).
Unrelated to the actual issue, but as a tip, this is why we track coverage of tests, so we catch test code that's not running in our tests.
To clarify: the coverage tooling should require 100% coverage on the test folders so that it actually alerts you about such situations.
