From 9a56bb05be3c7e88a6dea9766fedc6d104b186fd Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 27 Jun 2018 08:28:34 -0300 Subject: [PATCH 1/4] Skip AppVeyor builds on tag pushes We don't deploy anything on tags with AppVeyor, we use Travis instead, so we might as well save resources --- appveyor.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/appveyor.yml b/appveyor.yml index b30da5c58..339611e72 100644 --- a/appveyor.yml +++ b/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 From 1dc5e97ac255b3a50b80b2a5400d7d467e952532 Mon Sep 17 00:00:00 2001 From: Serhii Mozghovyi Date: Thu, 28 Jun 2018 14:32:29 +0300 Subject: [PATCH 2/4] Make test parametrization override indirect fixtures --- src/_pytest/fixtures.py | 59 +++++++++++++++++++++++++++++++++------ src/_pytest/python.py | 5 ++++ testing/python/collect.py | 31 ++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 151346413..c0eb2e287 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -274,11 +274,45 @@ 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=lambda name: self.names_closure.index(name) + ) class FixtureRequest(FuncargnamesCompatAttr): @@ -1033,11 +1067,12 @@ class FixtureManager(object): 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 +1120,12 @@ class FixtureManager(object): 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 +1147,7 @@ class FixtureManager(object): 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: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 724a6098a..19a6b1593 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -439,6 +439,11 @@ class PyCollector(PyobjMixin, nodes.Collector): # 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( diff --git a/testing/python/collect.py b/testing/python/collect.py index 46694f2d8..c6afe7064 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -630,6 +630,37 @@ class TestFunction(object): 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( From c220fb235a08d1e5ae892c10566a350a8a08a89b Mon Sep 17 00:00:00 2001 From: Serhii Mozghovyi Date: Thu, 28 Jun 2018 14:53:06 +0300 Subject: [PATCH 3/4] Minor fix (code improvement) --- src/_pytest/fixtures.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index c0eb2e287..55df7e9d1 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -310,9 +310,7 @@ class FuncFixtureInfo(object): if argname in self.name2fixturedefs: working_set.update(self.name2fixturedefs[argname][-1].argnames) - self.names_closure[:] = sorted( - closure, key=lambda name: self.names_closure.index(name) - ) + self.names_closure[:] = sorted(closure, key=self.names_closure.index) class FixtureRequest(FuncargnamesCompatAttr): From 76ac670f7d6a249bf2fba0a151cc4e742c1fc2e0 Mon Sep 17 00:00:00 2001 From: Serhii Mozghovyi Date: Thu, 28 Jun 2018 23:42:18 +0300 Subject: [PATCH 4/4] Add changelog description --- AUTHORS | 1 + changelog/2220.bugfix.rst | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 changelog/2220.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 95e989306..9dcb446a8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -182,6 +182,7 @@ Ryan Wooden Samuel Dion-Girardeau Samuele Pedroni Segev Finer +Serhii Mozghovyi Simon Gomizelj Skylar Downes Srinivas Reddy Thatiparthy diff --git a/changelog/2220.bugfix.rst b/changelog/2220.bugfix.rst new file mode 100644 index 000000000..bc74b44bb --- /dev/null +++ b/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.