Merge pull request #2073 from nicoddemus/fix-hookproxy-cache

Remove hook proxy cache

CHANGELOG.rst

@@ -26,6 +26,10 @@
   once by the ``pytest_plugins`` mechanism.
   Thanks `@nicoddemus`_ for the PR.
 
+* Remove an internal cache which could cause hooks from ``conftest.py`` files in
+  sub-directories to be called in other directories incorrectly (`#2016`_).
+  Thanks `@d-b-w`_ for the report and `@nicoddemus`_ for the PR.
+
 * Remove internal code meant to support earlier Python 3 versions that produced the side effect
   of leaving ``None`` in ``sys.modules`` when expressions were evaluated by pytest (for example passing a condition
   as a string to ``pytest.mark.skipif``)(`#2103`_).
@@ -40,6 +44,7 @@
 
 .. _@mbukatov: https://github.com/mbukatov
 .. _@dupuy: https://bitbucket.org/dupuy/
+.. _@d-b-w: https://bitbucket.org/d-b-w/
 .. _@lwm: https://github.com/lwm
 .. _@adler-j: https://github.com/adler-j
 .. _@DuncanBetts: https://github.com/DuncanBetts
@@ -49,6 +54,7 @@
 .. _#2089: https://github.com/pytest-dev/pytest/issues/2089
 .. _#478: https://github.com/pytest-dev/pytest/issues/478
 .. _#687: https://github.com/pytest-dev/pytest/issues/687
+.. _#2016: https://github.com/pytest-dev/pytest/issues/2016
 .. _#2034: https://github.com/pytest-dev/pytest/issues/2034
 .. _#2038: https://github.com/pytest-dev/pytest/issues/2038
 .. _#2078: https://github.com/pytest-dev/pytest/issues/2078

_pytest/main.py

@@ -539,7 +539,6 @@ class Session(FSCollector):
     def __init__(self, config):
         FSCollector.__init__(self, config.rootdir, parent=None,
                              config=config, session=self)
-        self._fs2hookproxy = {}
         self.testsfailed = 0
         self.testscollected = 0
         self.shouldstop = False
@@ -570,23 +569,18 @@ class Session(FSCollector):
         return path in self._initialpaths
 
     def gethookproxy(self, fspath):
-        try:
+        # check if we have the common case of running
-            return self._fs2hookproxy[fspath]
+        # hooks with all conftest.py filesall conftest.py
-        except KeyError:
+        pm = self.config.pluginmanager
-            # check if we have the common case of running
+        my_conftestmodules = pm._getconftestmodules(fspath)
-            # hooks with all conftest.py filesall conftest.py
+        remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
-            pm = self.config.pluginmanager
+        if remove_mods:
-            my_conftestmodules = pm._getconftestmodules(fspath)
+            # one or more conftests are not in use at this fspath
-            remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
+            proxy = FSHookProxy(fspath, pm, remove_mods)
-            if remove_mods:
+        else:
-                # one or more conftests are not in use at this fspath
+            # all plugis are active for this fspath
-                proxy = FSHookProxy(fspath, pm, remove_mods)
+            proxy = self.config.hook
-            else:
+        return proxy
-                # all plugis are active for this fspath
-                proxy = self.config.hook
-
-            self._fs2hookproxy[fspath] = proxy
-            return proxy
 
     def perform_collect(self, args=None, genitems=True):
         hook = self.config.hook

_pytest/pytester.py

@@ -478,6 +478,7 @@ class Testdir:
         ret = None
         for name, value in items:
             p = self.tmpdir.join(name).new(ext=ext)
+            p.dirpath().ensure_dir()
             source = Source(value)
 
             def my_totext(s, encoding="utf-8"):

testing/test_conftest.py

@@ -423,3 +423,28 @@ def test_conftest_exception_handling(testdir):
     res = testdir.runpytest()
     assert res.ret == 4
     assert 'raise ValueError()' in [line.strip() for line in res.errlines]
+
+
+def test_hook_proxy(testdir):
+    """Session's gethookproxy() would cache conftests incorrectly (#2016).
+    It was decided to remove the cache altogether.
+    """
+    testdir.makepyfile(**{
+        'root/demo-0/test_foo1.py': "def test1(): pass",
+
+        'root/demo-a/test_foo2.py': "def test1(): pass",
+        'root/demo-a/conftest.py': """
+            def pytest_ignore_collect(path, config):
+                return True
+            """,
+
+        'root/demo-b/test_foo3.py': "def test1(): pass",
+        'root/demo-c/test_foo4.py': "def test1(): pass",
+    })
+    result = testdir.runpytest()
+    result.stdout.fnmatch_lines([
+        '*test_foo1.py*',
+        '*test_foo3.py*',
+        '*test_foo4.py*',
+        '*3 passed*',
+    ])

testing/test_pluginmanager.py

@@ -4,7 +4,8 @@ import py
 import os
 
 from _pytest.config import get_config, PytestPluginManager
-from _pytest.main import EXIT_NOTESTSCOLLECTED
+from _pytest.main import EXIT_NOTESTSCOLLECTED, Session
+
 
 @pytest.fixture
 def pytestpm():
@@ -133,6 +134,25 @@ class TestPytestPluginInteractions:
         finally:
             undo()
 
+    def test_hook_proxy(self, testdir):
+        """Test the gethookproxy function(#2016)"""
+        config = testdir.parseconfig()
+        session = Session(config)
+        testdir.makepyfile(**{
+            'tests/conftest.py': '',
+            'tests/subdir/conftest.py': '',
+        })
+
+        conftest1 = testdir.tmpdir.join('tests/conftest.py')
+        conftest2 = testdir.tmpdir.join('tests/subdir/conftest.py')
+
+        config.pluginmanager._importconftest(conftest1)
+        ihook_a = session.gethookproxy(testdir.tmpdir.join('tests'))
+        assert ihook_a is not None
+        config.pluginmanager._importconftest(conftest2)
+        ihook_b = session.gethookproxy(testdir.tmpdir.join('tests'))
+        assert ihook_a is not ihook_b
+
     def test_warn_on_deprecated_multicall(self, pytestpm):
         warnings = []