When selecting tests based on node id's, optimize setup_module() and teardown_module if the tests belong to the same module

[shreyashah]

Following is the structure of my test script:

test_a.py
    Class Test_A:
        test_one()
        test_two()
        test_three()
        test_four()
    Class Test_B:
        test_one()
        test_five()
        test_six()

If I want to run as:

pytest test_a.py::Test_A::test_one   test_a.py::Test_B::test_five

then, setup_module() and teardown_module is run twice because the collection has assumed the node id's belong to different module. However, in this case I don't want to use -k option because I have a bunch of test cases from different classes but from the same script to be selected and the function names could be same in different classes.

Is there a way we can have collection hook to figure out if the node id's specified on the cmd line belong to the same module or same class and if yes, execute setup_module, setup_class, teardown_class and teardown_module only once?

[pytestbot]

GitMate.io thinks possibly related issues are #499 (Selecting a test by its node id not documented), #50 (Select tests according to their mark), #1360 (Simplify running parametrized tests by node ID), #1978 (pytest not finding any tests under module), and #723 (module scope fixture invoked multiple times if using test node specifiers for same module).

[shreyashah]

[shreyashah]

[brianmaissy]

I think the best way to solve this is to use fixtures rather than xunit-style setup and teardown methods. Would that be a feasible refactor?

[RonnyPfannschmidt]

the actual issue that when 2 node ids are given, then py.test collects against the module multiple time (downright duplication of tests)

this is a problematic design issue

[shreyashah]

@RonnyPfannschmidt : Thanks for your reply. At this moment, do you think I should implement one of the collection hooks like pytest_collection_modifyitems() or pytest_collection().
What approach would you take?

[brianmaissy]

I see. Instead of independently calling collect_one_node on each argument, we should probably reduce all of the arguments to a tree of nodes, and then collect them together.

It seems from the "remarks" section at the bottom of the "classic xunit-style setup" section of the docs that this is a known issue. I'm guessing that it's a design issue to which attention was consciously diverted, given that the fixture style of setup/teardown management is preferred.

The possible outright duplication of nodes is more problematic, although it could be argued that the current behavior is the desired one, and one should use at his own risk. If so, it should be more clearly documented though.

[RonnyPfannschmidt]

@brianmaissy the current behaviour is most likely the result of minimal change that was only test driven and not design driven

[shreyashah]

@RonnyPfannschmidt : So what approach would you recommend me for this case? Implementing which collection hook : pytest_collection_modifyitems ? or if not this hook, which would be the other one?

[nicoddemus]

the actual issue that when 2 node ids are given, then py.test collects against the module multiple time (downright duplication of tests)

Indeed it is not related to xunit-style at all, I get the same behavior if I use an autouse fixture:

import pytest

@pytest.fixture(scope='module', autouse=True)
def _setup_module():
    print('SETUP MODULE')

def test_1():
    pass

def test_2():
    pass

> pytest .tmp\foo.py::test_1 .tmp\foo.py::test_2 -sq
SETUP MODULE
.SETUP MODULE
.
2 passed in 0.01 seconds

xref: #2679, #723 is a duplicate

It might be simplistic, but can't we just remember during argument parsing which modules were loaded and reuse the module object for collection when a module appears twice?

[RonnyPfannschmidt]

that should be relatively simple - we should keep a plugin object to store the data somewhere
also actually just storing the modules wont do, we would have to remember the items/collection tree

[nicoddemus]

we would have to remember the items/collection tree

Yep, you are right, thanks for the heads up.