Package scoped fixture is evaluated multiple times if used in a sub-package

[s0undt3ch]

• a detailed description of the bug or suggestion
• output of pip list from the virtual environment you are using
• pytest and operating system versions
• minimal example if possible

When defining a package scoped fixture, if used in the package and in a sub-package, the fixture is evaluated twice.

Coinsider the following diff against PyTest 5.4.2 running on Linux:

diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index 7fc87e387..7bc1dc990 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -1566,6 +1566,71 @@ class TestFixtureManagerParseFactories:
         reprec = testdir.inline_run()
         reprec.assertoutcome(passed=2)
 
+    def test_package_fixture_complex_sub(self, testdir):
+        testdir.makepyfile(
+            __init__="""\
+            values = []
+        """
+        )
+        testdir.syspathinsert(testdir.tmpdir.dirname)
+        package = testdir.mkdir("package")
+        package.join("__init__.py").write("")
+        package.join("conftest.py").write(
+            textwrap.dedent(
+                """\
+                import pytest
+                from .. import values
+                @pytest.fixture(scope="package")
+                def one():
+                    try:
+                        one.__calls__ += 1
+                    except AttributeError:
+                        one.__calls__ = 0
+                    values.append("package")
+                    yield values
+                    values.pop()
+                    assert one.__calls__ == 0
+
+                @pytest.fixture(scope="package", autouse=True)
+                def two():
+                    try:
+                        two.__calls__ += 1
+                    except AttributeError:
+                        two.__calls__ = 0
+                    values.append("package-auto")
+                    yield values
+                    values.pop()
+                    assert two.__calls__ == 0
+                """
+            )
+        )
+        package.join("test_x.py").write(
+            textwrap.dedent(
+                """\
+                from .. import values
+                def test_package_autouse():
+                    assert values == ["package-auto"]
+                def test_package(one):
+                    assert values == ["package-auto", "package"]
+                """
+            )
+        )
+        sub1 = package.mkdir("sub1")
+        sub1.join("__init__.py").write("")
+        sub1.join("test_y.py").write(
+            textwrap.dedent(
+                """\
+                from ... import values
+                def test_package_autouse():
+                    assert values == ["package-auto"]
+                def test_package(one):
+                    assert values == ["package-auto", "package"]
+                """
+            )
+        )
+        reprec = testdir.inline_run()
+        reprec.assertoutcome(passed=4)
+
     def test_collect_custom_items(self, testdir):
         testdir.copy_example("fixtures/custom_item")
         result = testdir.runpytest("foo")

py37 inst-nodeps: /home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/.tox/.tmp/package/1/pytest-5.4.1.dev356+g981b09694.d20200525.tar.gz
py37 installed: argcomplete==1.11.1,attrs==19.3.0,certifi==2020.4.5.1,chardet==3.0.4,elementpath==1.4.5,hypothesis==5.15.1,idna==2.9,importlib-metadata==1.6.0,mock==4.0.2,more-itertools==8.3.0,nose==1.3.7,packaging==20.4,pluggy==0.13.1,py==1.8.1,Pygments==2.6.1,pyparsing==2.4.7,pytest @ file:///home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/.tox/.tmp/package/1/pytest-5.4.1.dev356%2Bg981b09694.d20200525.tar.gz,requests==2.23.0,six==1.15.0,sortedcontainers==2.1.0,urllib3==1.25.9,wcwidth==0.1.9,xmlschema==1.1.3,zipp==3.1.0
py37 run-test-pre: PYTHONHASHSEED='2661555164'
py37 run-test: commands[0] | pytest -vs --maxfail=1 testing/python/fixtures.py -k test_package_fixture_complex_sub
============================= test session starts ==============================
platform linux -- Python 3.7.5, pytest-5.4.1.dev356+g981b09694.d20200525, py-1.8.1, pluggy-0.13.1 -- /home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/.tox/py37/bin/python
cachedir: .tox/py37/.pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/.hypothesis/examples')
rootdir: /home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope, inifile: tox.ini
plugins: hypothesis-5.15.1
collecting ... collected 190 items / 189 deselected / 1 selected

testing/python/fixtures.py::TestFixtureManagerParseFactories::test_package_fixture_complex_sub ============================= test session starts ==============================
platform linux -- Python 3.7.5, pytest-5.4.1.dev356+g981b09694.d20200525, py-1.8.1, pluggy-0.13.1
rootdir: /tmp/pytest-of-vampas/pytest-54/test_package_fixture_complex_sub0
collected 4 items

package/test_x.py ..                                                     [ 50%]
package/sub1/test_y.py ..E                                               [100%]

==================================== ERRORS ====================================
______________________ ERROR at teardown of test_package _______________________

    @pytest.fixture(scope="package")
    def one():
        try:
            one.__calls__ += 1
        except AttributeError:
            one.__calls__ = 0
        values.append("package")
        yield values
        values.pop()
>       assert one.__calls__ == 0
E       assert 1 == 0
E        +  where 1 = one.__calls__

package/conftest.py:12: AssertionError
=========================== short test summary info ============================
ERROR package/sub1/test_y.py::test_package - assert 1 == 0
========================== 4 passed, 1 error in 0.11s ==========================
FAILED

=================================== FAILURES ===================================
______ TestFixtureManagerParseFactories.test_package_fixture_complex_sub _______

self = <fixtures.TestFixtureManagerParseFactories object at 0x7f54e5a9acd0>
testdir = <Testdir local('/tmp/pytest-of-vampas/pytest-54/test_package_fixture_complex_sub0')>

    def test_package_fixture_complex_sub(self, testdir):
        testdir.makepyfile(
            __init__="""\
            values = []
        """
        )
        testdir.syspathinsert(testdir.tmpdir.dirname)
        package = testdir.mkdir("package")
        package.join("__init__.py").write("")
        package.join("conftest.py").write(
            textwrap.dedent(
                """\
                import pytest
                from .. import values
                @pytest.fixture(scope="package")
                def one():
                    try:
                        one.__calls__ += 1
                    except AttributeError:
                        one.__calls__ = 0
                    values.append("package")
                    yield values
                    values.pop()
                    assert one.__calls__ == 0
    
                @pytest.fixture(scope="package", autouse=True)
                def two():
                    try:
                        two.__calls__ += 1
                    except AttributeError:
                        two.__calls__ = 0
                    values.append("package-auto")
                    yield values
                    values.pop()
                    assert two.__calls__ == 0
                """
            )
        )
        package.join("test_x.py").write(
            textwrap.dedent(
                """\
                from .. import values
                def test_package_autouse():
                    assert values == ["package-auto"]
                def test_package(one):
                    assert values == ["package-auto", "package"]
                """
            )
        )
        sub1 = package.mkdir("sub1")
        sub1.join("__init__.py").write("")
        sub1.join("test_y.py").write(
            textwrap.dedent(
                """\
                from ... import values
                def test_package_autouse():
                    assert values == ["package-auto"]
                def test_package(one):
                    assert values == ["package-auto", "package"]
                """
            )
        )
        reprec = testdir.inline_run()
>       reprec.assertoutcome(passed=4)
E       AssertionError: ([<TestReport 'package/test_x.py::test_package_autouse' when='call' outcome='passed'>, <TestReport 'package/test_x.py:...='call' outcome='passed'>], [], [<TestReport 'package/sub1/test_y.py::test_package' when='teardown' outcome='failed'>])
E       assert {'failed': 1,... 'skipped': 0} == {'failed': 0,... 'skipped': 0}
E         Omitting 2 identical items, use -vv to show
E         Differing items:
E         {'failed': 1} != {'failed': 0}
E         Full diff:
E         - {'failed': 0, 'passed': 4, 'skipped': 0}
E         ?            ^
E         + {'failed': 1, 'passed': 4, 'skipped': 0}...
E         
E         ...Full output truncated (2 lines hidden), use '-vv' to show

/home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/testing/python/fixtures.py:1632: AssertionError
=========================== short test summary info ============================
FAILED testing/python/fixtures.py::TestFixtureManagerParseFactories::test_package_fixture_complex_sub
!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!
====================== 1 failed, 189 deselected in 0.51s =======================
ERROR: InvocationError for command /home/vampas/projects/SaltStack/pytest/pytest/hotfix/fixture-scope/.tox/py37/bin/pytest -vs --maxfail=1 testing/python/fixtures.py -k test_package_fixture_complex_sub (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   py37: commands failed

pip list

❯ pip list
Package            Version
------------------ -------
appdirs            1.4.4  
distlib            0.3.0  
filelock           3.0.12 
importlib-metadata 1.6.0  
packaging          20.4   
pip                19.2.3 
pluggy             0.13.1 
py                 1.8.1  
pyparsing          2.4.7  
setuptools         41.2.0 
six                1.15.0 
toml               0.10.1 
tox                3.15.1 
virtualenv         20.0.21
zipp               3.1.0

❯ python setup.py --version
5.4.1.dev356+g981b09694.d20200525

[s0undt3ch]

This better demonstrates the wanted final behavior

diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index 7fc87e387..162ae0298 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -1566,6 +1566,83 @@ class TestFixtureManagerParseFactories:
         reprec = testdir.inline_run()
         reprec.assertoutcome(passed=2)
 
+    def test_package_fixture_complex_sub(self, testdir):
+        testdir.makepyfile(
+            __init__="""\
+            values = []
+        """
+        )
+        testdir.syspathinsert(testdir.tmpdir.dirname)
+        package = testdir.mkdir("package")
+        package.join("__init__.py").write("")
+        package.join("conftest.py").write(
+            textwrap.dedent(
+                """\
+                import pytest
+                from .. import values
+
+                @pytest.fixture(scope="session")
+                def zero():
+                    try:
+                        zero.__calls__ += 1
+                    except AttributeError:
+                        zero.__calls__ = 1
+                    values.append("session")
+                    yield values
+                    values.pop()
+                    assert zero.__calls__ == 1
+                    assert one.__calls__ == 1
+                    assert two.__calls__ == 1
+
+                @pytest.fixture(scope="package")
+                def one(zero):
+                    try:
+                        one.__calls__ += 1
+                    except AttributeError:
+                        one.__calls__ = 1
+                    values.append("package")
+                    yield values
+                    values.pop()
+
+                @pytest.fixture(scope="package", autouse=True)
+                def two(zero):
+                    try:
+                        two.__calls__ += 1
+                    except AttributeError:
+                        two.__calls__ = 1
+                    values.append("package-auto")
+                    yield values
+                    values.pop()
+                """
+            )
+        )
+        package.join("test_x.py").write(
+            textwrap.dedent(
+                """\
+                from .. import values
+                def test_package_autouse():
+                    assert values == ["session", "package-auto"]
+                def test_package(one):
+                    assert values == ["session", "package-auto", "package"]
+                """
+            )
+        )
+        sub1 = package.mkdir("sub1")
+        sub1.join("__init__.py").write("")
+        sub1.join("test_y.py").write(
+            textwrap.dedent(
+                """\
+                from ... import values
+                def test_package_autouse():
+                    assert values == ["session", "package-auto"]
+                def test_package(one):
+                    assert values == ["session", "package-auto", "package"]
+                """
+            )
+        )
+        reprec = testdir.inline_run()
+        reprec.assertoutcome(passed=4)
+
     def test_collect_custom_items(self, testdir):
         testdir.copy_example("fixtures/custom_item")
         result = testdir.runpytest("foo")

[s0undt3ch]

I've been digging into the source code and I can't seem to find my way around fixture scopes and their termination routines.
I think the solution, kind of, involves package scoped fixture to behave closer to session scoped fixtures instead of function scoped fixtures.
I also notice that the Fixturedef is the same by its id(), but it still gets executeed and finished twice....

Any guidance in the right direction?

[symonk]

Thanks for the report @s0undt3ch, I will get a look into this at some point this week

[s0undt3ch]

Thanks @symonk.
Spent some more hours around it, but I just don't know enough about the internals yet.

[s0undt3ch]

Gentle ping @symonk. Got any updates?

[SalmonMode]

Is there a reference to the details about the intended behavior for this scope? I checked the docs but didn't see any specifics.

My assumption is, based on the description in the docs, that it would tear down twice if the last test to run was tests/sub_package/test_a.py::test_thing, once for exiting the sub_package package and again for exiting tests package, but that doesn't seem ideal (or practical).

My assumption based on the general idea, is that it would be allowed to run once per lowest package level and then teardown once for each lowest package level that it was run for. For example, if I had tests/sub_package_a/ and tests/sub_package_b, then it could run once for the sub_package_a package, and be torn down as it exits that package, and then again for the sub_package_b package.

But this presents a problem, as another sub-package being introduced inside sub_package_a would suddenly prevent tests that are in the sub_package_a package level from using the fixture as they were designed to. And it would also mean you have a lack of an ability to run a fixture for an entire specific package at any level if it has any sub-packages inside it.

Another way I could see it implemented would be if it's only meant to apply to the package who's conftest.py it was defined in, only being allowed to run once in that package, and only being torn down as it leaves that package. For example, being defined in tests/sub_package_a/conftest.py and only running once for that package, despite there also being tests/sub_package_a/sub_sub_package/. But then this would mean you wouldn't be able to define fixtures that are meant to reset between the sub-packages of tests/sub_package_a/ without having to define the same fixture in each one of them.

[s0undt3ch]

To me its straightforward, the behavior I'm after.
If a fixture is defined for package.a it only gets teardown when we exit paclage.a. Meaning, package.a.a1 still has access to the fixture defined with the package.a scope.
When you enter the package.a.a1 scope you haven't left package.a.

[SalmonMode]

But how would you attach the fixture the package.a, and not package.a.a1?

[bluetech]

This is fixed by #11646. The test doesn't pass exactly, because once one runs for the subpackage it stays in scope for package as well (if you change sub -> zzzsub then it's the other way around). This is expected.