Merge pull request #2862 from tom-dalton-fanduel/issue-2836-fixture-collection-bug

Issue 2836 fixture collection bug

AUTHORS

@@ -164,6 +164,7 @@ Stephan Obermann
 Tareq Alayan
 Ted Xiao
 Thomas Grainger
+Tom Dalton
 Tom Viner
 Trevor Bekolay
 Tyler Goodlet

_pytest/fixtures.py

@@ -1,13 +1,14 @@
 from __future__ import absolute_import, division, print_function
-import sys
-
-from py._code.code import FormattedExcinfo
-
-import py
-import warnings
 
 import inspect
+import sys
+import warnings
+
+import py
+from py._code.code import FormattedExcinfo
+
 import _pytest
+from _pytest import nodes
 from _pytest._code.code import TerminalRepr
 from _pytest.compat import (
     NOTSET, exc_clear, _format_args,
@@ -15,9 +16,10 @@ from _pytest.compat import (
     is_generator, isclass, getimfunc,
     getlocation, getfuncargnames,
     safe_getattr,
+    FuncargnamesCompatAttr,
 )
 from _pytest.outcomes import fail, TEST_OUTCOME
-from _pytest.compat import FuncargnamesCompatAttr
+
 
 if sys.version_info[:2] == (2, 6):
     from ordereddict import OrderedDict
@@ -981,8 +983,8 @@ class FixtureManager:
             # by their test id)
             if p.basename.startswith("conftest.py"):
                 nodeid = p.dirpath().relto(self.config.rootdir)
-                if p.sep != "/":
+                if p.sep != nodes.SEP:
-                    nodeid = nodeid.replace(p.sep, "/")
+                    nodeid = nodeid.replace(p.sep, nodes.SEP)
         self.parsefactories(plugin, nodeid)
 
     def _getautousenames(self, nodeid):
@@ -1132,5 +1134,5 @@ class FixtureManager:
 
     def _matchfactories(self, fixturedefs, nodeid):
         for fixturedef in fixturedefs:
-            if nodeid.startswith(fixturedef.baseid):
+            if nodes.ischildnode(fixturedef.baseid, nodeid):
                 yield fixturedef

_pytest/junitxml.py

@@ -17,6 +17,7 @@ import re
 import sys
 import time
 import pytest
+from _pytest import nodes
 from _pytest.config import filename_arg
 
 # Python 2.X and 3.X compatibility
@@ -252,7 +253,7 @@ def mangle_test_address(address):
     except ValueError:
         pass
     # convert file path to dotted path
-    names[0] = names[0].replace("/", '.')
+    names[0] = names[0].replace(nodes.SEP, '.')
     names[0] = _py_ext_re.sub("", names[0])
     # put any params back
     names[-1] += possible_open_bracket + params

_pytest/main.py

@@ -6,6 +6,7 @@ import os
 import sys
 
 import _pytest
+from _pytest import nodes
 import _pytest._code
 import py
 try:
@@ -14,8 +15,8 @@ except ImportError:
     from UserDict import DictMixin as MappingMixin
 
 from _pytest.config import directory_arg, UsageError, hookimpl
-from _pytest.runner import collect_one_node
 from _pytest.outcomes import exit
+from _pytest.runner import collect_one_node
 
 tracebackcutdir = py.path.local(_pytest.__file__).dirpath()
 
@@ -516,14 +517,14 @@ class FSCollector(Collector):
             rel = fspath.relto(parent.fspath)
             if rel:
                 name = rel
-            name = name.replace(os.sep, "/")
+            name = name.replace(os.sep, nodes.SEP)
         super(FSCollector, self).__init__(name, parent, config, session)
         self.fspath = fspath
 
     def _makeid(self):
         relpath = self.fspath.relto(self.config.rootdir)
-        if os.sep != "/":
+        if os.sep != nodes.SEP:
-            relpath = relpath.replace(os.sep, "/")
+            relpath = relpath.replace(os.sep, nodes.SEP)
         return relpath
 
 

_pytest/nodes.py

@@ -0,0 +1,37 @@
+SEP = "/"
+
+
+def _splitnode(nodeid):
+    """Split a nodeid into constituent 'parts'.
+
+    Node IDs are strings, and can be things like:
+        ''
+        'testing/code'
+        'testing/code/test_excinfo.py'
+        'testing/code/test_excinfo.py::TestFormattedExcinfo::()'
+
+    Return values are lists e.g.
+        []
+        ['testing', 'code']
+        ['testing', 'code', 'test_excinfo.py']
+        ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo', '()']
+    """
+    if nodeid == '':
+        # If there is no root node at all, return an empty list so the caller's logic can remain sane
+        return []
+    parts = nodeid.split(SEP)
+    # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()'
+    parts[-1:] = parts[-1].split("::")
+    return parts
+
+
+def ischildnode(baseid, nodeid):
+    """Return True if the nodeid is a child node of the baseid.
+
+    E.g. 'foo/bar::Baz::()' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp'
+    """
+    base_parts = _splitnode(baseid)
+    node_parts = _splitnode(nodeid)
+    if len(node_parts) < len(base_parts):
+        return False
+    return node_parts[:len(base_parts)] == base_parts

_pytest/python_api.py

@@ -217,7 +217,8 @@ class ApproxScalar(ApproxBase):
         absolute tolerance or a relative tolerance, depending on what the user
         specified or which would be larger.
         """
-        def set_default(x, default): return x if x is not None else default
+        def set_default(x, default):
+            return x if x is not None else default
 
         # Figure out what the absolute tolerance should be.  ``self.abs`` is
         # either None or a value specified by the user.

_pytest/terminal.py

@@ -13,6 +13,7 @@ import sys
 import time
 import platform
 
+from _pytest import nodes
 import _pytest._pluggy as pluggy
 
 
@@ -452,7 +453,7 @@ class TerminalReporter:
 
         if fspath:
             res = mkrel(nodeid).replace("::()", "")  # parens-normalization
-            if nodeid.split("::")[0] != fspath.replace("\\", "/"):
+            if nodeid.split("::")[0] != fspath.replace("\\", nodes.SEP):
                 res += " <- " + self.startdir.bestrelpath(fspath)
         else:
             res = "[location]"

changelog/2836.bug

@@ -0,0 +1 @@
+Match fixture paths against actual path segments in order to avoid matching folders which share a prefix.

testing/test_collection.py

@@ -2,6 +2,7 @@ from __future__ import absolute_import, division, print_function
 import pytest
 import py
 
+import _pytest._code
 from _pytest.main import Session, EXIT_NOTESTSCOLLECTED, _in_venv
 
 
@@ -830,3 +831,28 @@ def test_continue_on_collection_errors_maxfail(testdir):
         "*Interrupted: stopping after 3 failures*",
         "*1 failed, 2 error*",
     ])
+
+
+def test_fixture_scope_sibling_conftests(testdir):
+    """Regression test case for https://github.com/pytest-dev/pytest/issues/2836"""
+    foo_path = testdir.mkpydir("foo")
+    foo_path.join("conftest.py").write(_pytest._code.Source("""
+        import pytest
+        @pytest.fixture
+        def fix():
+            return 1
+    """))
+    foo_path.join("test_foo.py").write("def test_foo(fix): assert fix == 1")
+
+    # Tests in `food/` should not see the conftest fixture from `foo/`
+    food_path = testdir.mkpydir("food")
+    food_path.join("test_food.py").write("def test_food(fix): assert fix == 1")
+
+    res = testdir.runpytest()
+    assert res.ret == 1
+
+    res.stdout.fnmatch_lines([
+        "*ERROR at setup of test_food*",
+        "E*fixture 'fix' not found",
+        "*1 passed, 1 error*",
+    ])

testing/test_nodes.py

@@ -0,0 +1,18 @@
+import pytest
+
+from _pytest import nodes
+
+
+@pytest.mark.parametrize("baseid, nodeid, expected", (
+    ('', '', True),
+    ('', 'foo', True),
+    ('', 'foo/bar', True),
+    ('', 'foo/bar::TestBaz::()', True),
+    ('foo', 'food', False),
+    ('foo/bar::TestBaz::()', 'foo/bar', False),
+    ('foo/bar::TestBaz::()', 'foo/bar::TestBop::()', False),
+    ('foo/bar', 'foo/bar::TestBop::()', True),
+))
+def test_ischildnode(baseid, nodeid, expected):
+    result = nodes.ischildnode(baseid, nodeid)
+    assert result is expected

tox.ini

@@ -213,3 +213,8 @@ filterwarnings =
 [flake8]
 max-line-length = 120
 exclude = _pytest/vendored_packages/pluggy.py
+ignore=
+    # do not use bare except'
+    E722
+    # ambiguous variable name 'l'
+    E741