Incomplete abstract class no longer logs an error

[Dreamsorcerer]

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*'

[Dreamsorcerer]

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.
        """
    ...

[The-Compiler]

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:

• python,unittest: don't collect abstract classes #12318
• v8.2.0 raises Can't instantiate abstract class when a test class implements an abstract class #12275

Also see 1a5e0eb which originally introduced that regression.

---

The example above:

• Passes (i.e. raises TypeError) before 1a5e0eb
• Still passes at 1a5e0eb (unittest: make obj work more like Function/Class #12089)
• Fails at 37489d3 (python,unittest: don't collect abstract classes #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'

[Dreamsorcerer]

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.

[bluetech]

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)

[RonnyPfannschmidt]

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

[Dreamsorcerer]

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..

[RonnyPfannschmidt]

@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

[bluetech]

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.

[RonnyPfannschmidt]

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)

[Dreamsorcerer]

As @RonnyPfannschmidt said, it's a common technique in unittest to create a base test class with test cases

The comment above (#13546 (comment)) 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.

[RonnyPfannschmidt]

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

[Dreamsorcerer]

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)..

[levsa]

I created #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.