Pytest: foo.test_foo is imported twice, including once before running conftest, when using `pytest --pyargs foo.test_foo`

[anntzer]

Thanks for submitting an issue!

Here's a quick checklist in what to include:

• [X] Include a detailed description of the bug or suggestion
• [X] pip list of the virtual environment you are using
• [X] pytest and operating system versions
• [X] Minimal example if possible

foo.test_foo is imported twice, including once before running conftest, when using pytest --pyargs foo.test_foo.

Python 3.7 / Arch Linux

$ pip list
Package        Version
-------------- -------
atomicwrites   1.3.0  
attrs          19.1.0 
more-itertools 7.0.0  
pip            19.0.3 
pluggy         0.9.0  
py             1.8.0  
pytest         4.4.0  
setuptools     40.8.0 
six            1.12.0

$ tree
.
└── foo
    ├── conftest.py
    ├── __init__.py
    └── test_foo.py
$ cat foo/conftest.py
def pytest_configure(config):
    print("configuring")
$ cat foo/test_foo.py
print("in test_foo")
def test_1(): pass



md5-805479f56824c81307e28483d02ef5ca



$ pytest --pyargs foo.test_foo -s
===================================== test session starts ======================================
platform linux -- Python 3.7.3, pytest-4.4.0, py-1.8.0, pluggy-0.9.0
rootdir: /tmp/foo
collecting ... in test_foo
configuring
in test_foo
collected 1 item                                                                               

foo/test_foo.py .

=================================== 1 passed in 0.01 seconds ===================================

Note that "in test_foo" appears twice, including once before the call to pytest_configure (which prints out "configuring").

This means that if test_foo has toplevel logic that depends on pytest_configure (e.g., via https://docs.pytest.org/en/latest/example/simple.html#detect-if-running-from-within-a-pytest-run -- an example is matplotlib's test helpers, which has a decorator to either generate a pytest test or a nose test instance), the logic will be wrong -- even though the test collected "pre-configure" will actually be discarded later (this happens in collect_one_node in _perform_collect), this can still e.g. trigger spurious warnings from toplevel code.

[blueyed]

Tried to reproduce this, but the following passes:

def test(testdir):
    testdir.mkpydir("sub")
    testdir.makepyfile(
        **{
            "sub/test_foo": """
            print("import_testfile")

            def test_foo():
                pass
            """,
            "sub/conftest.py": 'print("import_conftest")',
        }
    )

    result = testdir.runpytest(
        "-s", "--pyargs", "sub.test_foo", syspathinsert=True)
    result.stdout.fnmatch_lines(["import_conftest", "import_testfile"])
    assert result.stdout.str().count("import_conftest") == 1

Can you adjust it to make it fail?
(put it into pytest's "testing" dir, or run it with pytest -p pytester)

[anntzer]

It's import_testfile that incorrectly appears twice: replacing "import_conftest" by "import_testfile" in the last line of your test yields

    def test_importtwice(testdir):
        testdir.mkpydir("sub")
        testdir.makepyfile(
            **{
                "sub/test_foo": """
                print("import_testfile")

                def test_foo():
                    pass
                """,
                "sub/conftest.py": 'print("import_conftest")',
            }
        )

        result = testdir.runpytest(
            "-s", "--pyargs", "sub.test_foo", syspathinsert=True)
        result.stdout.fnmatch_lines(["import_conftest", "import_testfile"])
>       assert result.stdout.str().count("import_testfile") == 1
E       AssertionError: assert 2 == 1
E        +  where 2 = <built-in method count of str object at 0x7fdba584d558>('import_testfile')
E        +    where <built-in method count of str object at 0x7fdba584d558> = '============================= test session starts ==============================\nplatform linux -- Python 3.7.3, pyt...cted 1 item\n\nsub/test_foo.py .\n\n=========================== 1 passed in 0.01 seconds ===========================\n'.count
E        +      where '============================= test session starts ==============================\nplatform linux -- Python 3.7.3, pyt...cted 1 item\n\nsub/test_foo.py .\n\n=========================== 1 passed in 0.01 seconds ===========================\n' = <bound method LineMatcher.str of <_pytest.pytester.LineMatcher object at 0x7fdba58db390>>()
E        +        where <bound method LineMatcher.str of <_pytest.pytester.LineMatcher object at 0x7fdba58db390>> = <_pytest.pytester.LineMatcher object at 0x7fdba58db390>.str
E        +          where <_pytest.pytester.LineMatcher object at 0x7fdba58db390> = <RunResult ret=0 len(stdout.lines)=12 len(stderr.lines)=1 duration=0.03s>.stdout

[blueyed]

Great, thanks for catching!

Now we have a failing test at least already. I've checked it with later releases, and it also happens with 3.0, so is not a regression due to later changes with regard to namespaces etc at least.

You can use --assert=plain to work around it, i.e. the assertion rewriting causes the first import:

[21]   /usr/lib/python3.7/site-packages/_pytest/runner.py(253)pytest_make_collect_report()
-> call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
[22]   /usr/lib/python3.7/site-packages/_pytest/runner.py(226)from_call()
-> result = func()
[23]   /usr/lib/python3.7/site-packages/_pytest/runner.py(253)<lambda>()
-> call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
[24]   /usr/lib/python3.7/site-packages/_pytest/main.py(542)collect()
-> for x in self._collect(arg):
[25]   /usr/lib/python3.7/site-packages/_pytest/main.py(554)_collect()
-> names = self._parsearg(arg)
[26]   /usr/lib/python3.7/site-packages/_pytest/main.py(701)_parsearg()
-> parts[0] = self._tryconvertpyarg(parts[0])
[27]   /usr/lib/python3.7/site-packages/_pytest/main.py(680)_tryconvertpyarg()
-> loader = pkgutil.find_loader(x)
[28]   /usr/lib/python3.7/pkgutil.py(493)find_loader()
-> spec = importlib.util.find_spec(fullname)
[29]   /usr/lib/python3.7/importlib/util.py(94)find_spec()
-> parent = __import__(parent_name, fromlist=['__path__'])
[30]   <frozen importlib._bootstrap>(983)_find_and_load()
[31]   <frozen importlib._bootstrap>(967)_find_and_load_unlocked()
[32]   <frozen importlib._bootstrap>(668)_load_unlocked()
[33]   <frozen importlib._bootstrap>(638)_load_backward_compatible()
[34]   /usr/lib/python3.7/site-packages/_pytest/assertion/rewrite.py(299)load_module()
-> six.exec_(co, mod.__dict__)
[35] > /tmp/pytest-of-daniel/pytest-98/test0/sub/test_foo.py(4)<module>()
-> def test_foo():

The second one being then:

[22]   /usr/lib/python3.7/site-packages/_pytest/runner.py(253)pytest_make_collect_report()
-> call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
[23]   /usr/lib/python3.7/site-packages/_pytest/runner.py(226)from_call()
-> result = func()
[24]   /usr/lib/python3.7/site-packages/_pytest/runner.py(253)<lambda>()
-> call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
[25]   /usr/lib/python3.7/site-packages/_pytest/python.py(447)collect()
-> self._inject_setup_module_fixture()
[26]   /usr/lib/python3.7/site-packages/_pytest/python.py(459)_inject_setup_module_fixture()
-> setup_module = _get_non_fixture_func(self.obj, "setUpModule")
[27]   /usr/lib/python3.7/site-packages/_pytest/python.py(263)obj()
-> self._obj = obj = self._getobj()
[28]   /usr/lib/python3.7/site-packages/_pytest/python.py(444)_getobj()
-> return self._importtestmodule()
[29]   /usr/lib/python3.7/site-packages/_pytest/python.py(511)_importtestmodule()
-> mod = self.fspath.pyimport(ensuresyspath=importmode)
[30]   /usr/lib/python3.7/site-packages/py/_path/local.py(701)pyimport()
-> __import__(modname)
[31]   <frozen importlib._bootstrap>(983)_find_and_load()
[32]   <frozen importlib._bootstrap>(967)_find_and_load_unlocked()
[33]   <frozen importlib._bootstrap>(668)_load_unlocked()
[34]   <frozen importlib._bootstrap>(638)_load_backward_compatible()
[35]   /usr/lib/python3.7/site-packages/_pytest/assertion/rewrite.py(299)load_module()
-> six.exec_(co, mod.__dict__)
[36] > /tmp/pytest-of-daniel/pytest-98/test0/sub/test_foo.py(4)<module>()
-> def test_foo():

[blueyed]

Related code: https://github.com/pytest-dev/pytest/blob/13a9d876f74f17907ad04b13132cbd4aa4ad5842/src/_pytest/main.py#L676-L695

Maybe it just has to be added to sys.modules in some way, to not cause the double import?!

IIRC it tries to find a loader only, but the import is a sideeffect there then..

[anntzer]

Looks like this comes from AssertionRewritingHook.find_module not only doing the finding, but also the rewriting, which should arguably be done in load_module instead (or the equivalent, newer spec-based API)?

[asottile]

neat, I fixed this in #5468 unintentionally

[blueyed]

@asottile
Awesome. Is a test for this included in #5468?

[asottile]

yes

[blueyed]

Yeah, found it: https://github.com/pytest-dev/pytest/pull/5468/files#diff-1f1fc4eef6bf4c4da558e0dce9e74e9bR636

Awesome! :+1: