Pytest: [Feature request] Force pytest to collect classes with __init__ constructor

Created on 6 Apr 2020  路  14Comments  路  Source: pytest-dev/pytest

We have a large test suite based on nose which we are in the process of transforming to pytest and the only thing stopping us at the moment is that a lot of classes have and rely on an init constructor without any arguments. But the tests are not collected by pytest: https://github.com/pytest-dev/pytest/blob/2d9dac95ecbc67c73fc9f834a9a551d93befe159/src/_pytest/python.py#L683

When letting https://github.com/pytest-dev/pytest/blob/2d9dac95ecbc67c73fc9f834a9a551d93befe159/src/_pytest/python.py#L773 return False I can run and collect the tests perfectly fine.

I know some reasons for pytest not collecting classes with init:

  • Classes are used for structure not functionality
  • Constructor cannot take an argument
  • ...?

But couldn't there be a simple ini file option (force_init_collection) or some modification option (like for the names of classes and functions) that would let people that like to live dangerously run some tests?

collection question

Most helpful comment

My opinion is that there should at most be one state changing action per fixture (this makes teardowns safe if it's a transactional system), and there's nothing wrong with providing a separate fixture for each resource you want to be available. The latter comes with the caveat that you have solid abstractions, which is what my User object was meant to be (and would likely be composition-based).

Fixtures also don't all have to be in the same file. conftest.py files work similar to __init__.py, in that you can have one per folder, and they extend into all the folders and files inside that folder, so they can be used to provide common fixtures and even be used to more effectively manage the scope of fixtures so they get refreshed when they should be.

I will say, though, that if you start having a _ton_ of fixtures, it could be that you're involving far too much in your tests, and could be a sign that you need to engineer things so that your tests can be less complex.

All 14 comments

pytest itself will not add support for that as its currently under-specified what should happen with constructors in a sensible manner

however you can write a custom collection hook, that just returns a Class for your own classes

@RonnyPfannschmidt Thank you for the fast reply. I think I need some additional information. I was trying the pytest_pycollect_makeitem hook since I could not find a specific one for classes: https://docs.pytest.org/en/latest/reference.html#collection-hooks

For me the return value of this hook is unclear. What do you mean by just return a Class should I overwrite the https://github.com/pytest-dev/pytest/blob/af2b0e117459b7f41c845f9f676e904d3b35e5a2/src/_pytest/python.py#L678 or should I just return my test class like:

conftest.py

import inspect

def pytest_pycollect_makeitem(collector, name, obj):
    if inspect.isclass(obj):
        if collector.istestclass(obj, name):
            return obj

test.py

class TestStuff:

    def __init__(self):

        pass

    def test_stuff(self):
        assert True    

I get:

AttributeError: type object 'TestStuff' has no attribute 'reportinfo'

return pytest.Class.from_parent(collector, name=name) on recent pytest
pytest.Class(name=name, parent=collector) on older pytest

I could get it to work by additionally overwriting the pytest.Class

class Class(pytest.Class):

    def collect(self):
        if not getattr(self.obj, "__test__", True):
            return []

        self._inject_setup_class_fixture()
        self._inject_setup_method_fixture()

        return [pytest.Instance.from_parent(self, name="()")]


def pytest_pycollect_makeitem(collector, name, obj):
    if inspect.isclass(obj):
        if collector.istestclass(obj, name):
            return Class.from_parent(collector, name=name)

@RonnyPfannschmidt is this what you had in mind? I'm not entirely happy with this approach since I now have to keep up with changes to pytest.Class.

@PhilippSelenium neither i am, i misremembered what was necessary to set this up,

there should be a followup in pytest to mae the subclas no longer required at some point

Okay but it is perfectly fine for us as a workaround during our nose/pytest transition phase. @RonnyPfannschmidt Thank you for the fast help!

@PhilippSelenium in pytest-based test classes, I would highly recommend not tying anything to the class's state, as it throws a wrench into how pytest normally operates and now creates multiple, potentially conflicting, sources of truth.

In unittest.TestCase-based test classes, such as those in nose, state and other resources have to be managed through the state of both the class, and the instances of that class. In pytest, this is done through fixtures.

As you migrate from nose, fixtures will be your best bet.

If you have any examples of what you're trying to do in your __init__ methods, I can help you rework it into pytest fixtures.

@SalmonMode Thanks for the offer but the problem is not the reworking the problem was/is the transition phase in which the tests have to run with pytest and we gradually switch to the fixture approach. As mentioned above the init constructor is empty and is mostly use to initialize variables to None. The class base approach cannot be discarded instantly because there is lot's of inheritance logic.

Closing this as an inactive question.

@SalmonMode if the offer to help rewrite __init__ methods to fixtures isn't expired yet ;)

What would be the recommended approach to rewrite a code like this one:

class TestClusterCreationWithDBonTheSameOS(object):

def __init__(self, get_server_info):   # get_server_info is a tuple fixture with server ip, username and such
    self.get_server_info = get_server_info
    self.cmd = CMD(get_server_info=self.get_server_info) # setting up ssh connection to run commands on linux OS

    # Fetch settings to be checked after cluster is created
    _, self.expected_tcp_port = self.cmd.get_config_value(key="my.daemon.tcp.port")
    _, self.expected_udp_port = self.cmd.get_config_value(key="my.server.daemon.udp.port")

    # Fetch user details, to be passed to other functions in test class
    self.user_login = User.local_users["cluster_new_user"]["login"]
    self.user_pass = User.local_users["cluster_new_user"]["pass"]

    # Create User account 
    self.cmd.add_local_user(username=self.user_login, password=self.user_pass)

    # Fetch a configuration file to be used for other tests
    got_conf_file, self.conf_file_path = fetch_profile(host=self.get_server_info[0], user=self.user_login, pass=self.user_pass)

Any help would be much appreciated, thanks.

@BohdanHamulets You're more or less there. This seems to be pulling in various configurations, which makes this a little more complex. But, I would go with something like this:

@pytest.fixture(scope='module')
def command_config():
    # stuff to get config values like server info
    return config_object

@pytest.fixture(scope='module', autouse=True)
def cmd(command_config):
    return CMD(server_info=command_config.server_info, ssh_user=command_config.ssh_user)

@pytest.fixture(scope='class', autouse=True)
def local_user(cmd):
    user = User(cmd=cmd, username=random_username(), password=some_password)  # uses cmd to create user and then later to delete user
    # custom data types are useful for dependency injection
    yield user
    user.delete()

# I would provide an accessor through the user object to fetch the
# profile info (i.e. user.profile), as I think that object would have the
# interfaces it needs to fetch that info.

I'm not quite sure what the test would be doing, or how these things would be leveraged, but this is how I would tweak things a bit and then structure them just going a quick look.

Hope this helps!

@SalmonMode thanks a lot for taking the time :)
Yeah, this test code is even a bit more complex, __init__ configures a lot of stuff which is then shared with class functions.
To redo all of it I'll need to have 23 fixtures in one test file. Just wondering what the community thinks, is it okay or a bad practice? Thanks.

My opinion is that there should at most be one state changing action per fixture (this makes teardowns safe if it's a transactional system), and there's nothing wrong with providing a separate fixture for each resource you want to be available. The latter comes with the caveat that you have solid abstractions, which is what my User object was meant to be (and would likely be composition-based).

Fixtures also don't all have to be in the same file. conftest.py files work similar to __init__.py, in that you can have one per folder, and they extend into all the folders and files inside that folder, so they can be used to provide common fixtures and even be used to more effectively manage the scope of fixtures so they get refreshed when they should be.

I will say, though, that if you start having a _ton_ of fixtures, it could be that you're involving far too much in your tests, and could be a sign that you need to engineer things so that your tests can be less complex.

With advice's from here and other googling I've resolved a missing __init__ in a test class, by creating a Setup class(which is not picked up by pytest by the default), having __init__ there, and then making a fixture that calls this class allowing it to set things up, and then passing this fixture to test functions in the class. Sharing my solution for anyone that could need it.

(get_server_info in my case is a fixture defined in conftest.py which i've had to change scope to be 'session')

class SetupSuite(object):

    def __init__(self, get_server_info):
        self.get_server_info = get_server_info
        self.cmd = CMD(get_server_info=get_server_info)

        # Fetch user details, to be passed to other functions in test class
        self.user_login = User.local_users["cluster_new_user"]["login"]
        self.user_pass = User.local_users["cluster_new_user"]["pass"]

        # Create User account 
        self.cmd.add_local_user(username=self.user_login, password=self.user_pass)

        # Fetch a configuration file to be used for other tests
        got_conf_file, self.conf_file_path = fetch_profile(host=self.get_server_info[0], user=self.user_login, pass=self.user_pass)

@pytest.fixture(scope='module', autouse=True)
def setup(get_server_info):
    setup = SetupSuite(get_server_info=get_server_info)
    return setup

And now in my tests I can use setup fixture with all the needed attributes, connection object and variables:

class TestClusterCreationBla(object):

    def test_local_db_is_installed_blabla(self, setup):
        setup.cmd('ls -la')
        do_stuff(setup.user_login, setup.user_pass)
        ...

     def test_other_thing_bla(self, setup):
        run_tool(setup.conf_file_path)
        assert 1 == 1 
Was this page helpful?
0 / 5 - 0 ratings