Merge pull request #3629 from egnartsms/issue-2220-param-breaks-dep

Make test parametrization override indirect fixtures

Author: nicoddemus

AUTHORS

@@ -182,6 +182,7 @@ Ryan Wooden
 Samuel Dion-Girardeau
 Samuele Pedroni
 Segev Finer
+Serhii Mozghovyi
 Simon Gomizelj
 Skylar Downes
 Srinivas Reddy Thatiparthy

appveyor.yml

@@ -46,3 +46,7 @@ test_script:
 cache:
   - '%LOCALAPPDATA%\pip\cache'
   - '%USERPROFILE%\.cache\pre-commit'
+
+# We don't deploy anything on tags with AppVeyor, we use Travis instead, so we
+# might as well save resources
+skip_tags: true

changelog/2220.bugfix.rst

@@ -0,0 +1,3 @@
+In case a (direct) parameter of a test overrides some fixture upon which the
+test depends indirectly, do the pruning of the fixture dependency tree. That
+is, recompute the full set of fixtures the test function needs.

src/_pytest/fixtures.py

@@ -274,11 +274,43 @@ def get_direct_param_fixture_func(request):
     return request.param
 
 
+@attr.s(slots=True)
 class FuncFixtureInfo(object):
-    def __init__(self, argnames, names_closure, name2fixturedefs):
-        self.argnames = argnames
-        self.names_closure = names_closure
-        self.name2fixturedefs = name2fixturedefs
+    # original function argument names
+    argnames = attr.ib(type=tuple)
+    # argnames that function immediately requires. These include argnames +
+    # fixture names specified via usefixtures and via autouse=True in fixture
+    # definitions.
+    initialnames = attr.ib(type=tuple)
+    names_closure = attr.ib(type="List[str]")
+    name2fixturedefs = attr.ib(type="List[str, List[FixtureDef]]")
+
+    def prune_dependency_tree(self):
+        """Recompute names_closure from initialnames and name2fixturedefs
+
+        Can only reduce names_closure, which means that the new closure will
+        always be a subset of the old one. The order is preserved.
+
+        This method is needed because direct parametrization may shadow some
+        of the fixtures that were included in the originally built dependency
+        tree. In this way the dependency tree can get pruned, and the closure
+        of argnames may get reduced.
+        """
+        closure = set()
+        working_set = set(self.initialnames)
+        while working_set:
+            argname = working_set.pop()
+            # argname may be smth not included in the original names_closure,
+            # in which case we ignore it. This currently happens with pseudo
+            # FixtureDefs which wrap 'get_direct_param_fixture_func(request)'.
+            # So they introduce the new dependency 'request' which might have
+            # been missing in the original tree (closure).
+            if argname not in closure and argname in self.names_closure:
+                closure.add(argname)
+                if argname in self.name2fixturedefs:
+                    working_set.update(self.name2fixturedefs[argname][-1].argnames)
+
+        self.names_closure[:] = sorted(closure, key=self.names_closure.index)
 
 
 class FixtureRequest(FuncargnamesCompatAttr):
@@ -1033,11 +1065,12 @@ def getfixtureinfo(self, node, func, cls, funcargs=True):
         usefixtures = flatten(
             mark.args for mark in node.iter_markers(name="usefixtures")
         )
-        initialnames = argnames
-        initialnames = tuple(usefixtures) + initialnames
+        initialnames = tuple(usefixtures) + argnames
         fm = node.session._fixturemanager
-        names_closure, arg2fixturedefs = fm.getfixtureclosure(initialnames, node)
-        return FuncFixtureInfo(argnames, names_closure, arg2fixturedefs)
+        initialnames, names_closure, arg2fixturedefs = fm.getfixtureclosure(
+            initialnames, node
+        )
+        return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs)
 
     def pytest_plugin_registered(self, plugin):
         nodeid = None
@@ -1085,6 +1118,12 @@ def merge(otherlist):
                     fixturenames_closure.append(arg)
 
         merge(fixturenames)
+
+        # at this point, fixturenames_closure contains what we call "initialnames",
+        # which is a set of fixturenames the function immediately requests. We
+        # need to return it as well, so save this.
+        initialnames = tuple(fixturenames_closure)
+
         arg2fixturedefs = {}
         lastlen = -1
         while lastlen != len(fixturenames_closure):
@@ -1106,7 +1145,7 @@ def sort_by_scope(arg_name):
                 return fixturedefs[-1].scopenum
 
         fixturenames_closure.sort(key=sort_by_scope)
-        return fixturenames_closure, arg2fixturedefs
+        return initialnames, fixturenames_closure, arg2fixturedefs
 
     def pytest_generate_tests(self, metafunc):
         for argname in metafunc.fixturenames:

src/_pytest/python.py

@@ -439,6 +439,11 @@ def _genfunctions(self, name, funcobj):
             # add funcargs() as fixturedefs to fixtureinfo.arg2fixturedefs
             fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm)
 
+            # add_funcarg_pseudo_fixture_def may have shadowed some fixtures
+            # with direct parametrization, so make sure we update what the
+            # function really needs.
+            fixtureinfo.prune_dependency_tree()
+
             for callspec in metafunc._calls:
                 subname = "%s[%s]" % (name, callspec.id)
                 yield Function(

testing/python/collect.py

@@ -630,6 +630,37 @@ def test_overridden_via_param(value):
         rec = testdir.inline_run()
         rec.assertoutcome(passed=1)
 
+    def test_parametrize_overrides_indirect_dependency_fixture(self, testdir):
+        """Test parametrization when parameter overrides a fixture that a test indirectly depends on"""
+        testdir.makepyfile(
+            """
+            import pytest
+
+            fix3_instantiated = False
+
+            @pytest.fixture
+            def fix1(fix2):
+               return fix2 + '1'
+
+            @pytest.fixture
+            def fix2(fix3):
+               return fix3 + '2'
+
+            @pytest.fixture
+            def fix3():
+               global fix3_instantiated
+               fix3_instantiated = True
+               return '3'
+
+            @pytest.mark.parametrize('fix2', ['2'])
+            def test_it(fix1):
+               assert fix1 == '21'
+               assert not fix3_instantiated
+        """
+        )
+        rec = testdir.inline_run()
+        rec.assertoutcome(passed=1)
+
     @ignore_parametrized_marks
     def test_parametrize_with_mark(self, testdir):
         items = testdir.getitems(