Address more review comments, fix massive bug I reintroduced in the node-splitting code :-/

Author: tom-dalton-fanduel

_pytest/fixtures.py

@@ -1,12 +1,12 @@
 from __future__ import absolute_import, division, print_function
-import sys
 
-from py._code.code import FormattedExcinfo
+import inspect
+import sys
+import warnings
 
 import py
-import warnings
+from py._code.code import FormattedExcinfo
 
-import inspect
 import _pytest
 from _pytest._code.code import TerminalRepr
 from _pytest.compat import (
@@ -15,9 +15,11 @@
     is_generator, isclass, getimfunc,
     getlocation, getfuncargnames,
     safe_getattr,
+    FuncargnamesCompatAttr,
 )
+from _pytest.nodes import ischildnode
 from _pytest.outcomes import fail, TEST_OUTCOME
-from _pytest.compat import FuncargnamesCompatAttr
+
 
 if sys.version_info[:2] == (2, 6):
     from ordereddict import OrderedDict
@@ -1130,40 +1132,7 @@ def getfixturedefs(self, argname, nodeid):
         else:
             return tuple(self._matchfactories(fixturedefs, nodeid))
 
-    @classmethod
-    def _splitnode(cls, 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', '()']
-        """
-        parts = nodeid.split(py.path.local.sep)
-        # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()'
-        parts[-1:] = parts[-1].split("::")
-        return parts
-
-    @classmethod
-    def _ischildnode(cls, 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 = cls._splitnode(baseid)
-        node_parts = cls._splitnode(nodeid)
-        if len(node_parts) < len(base_parts):
-            return False
-        return node_parts[:len(base_parts)] == base_parts
-
     def _matchfactories(self, fixturedefs, nodeid):
         for fixturedef in fixturedefs:
-            if self._ischildnode(fixturedef.baseid, nodeid):
+            if ischildnode(fixturedef.baseid, nodeid):
                 yield fixturedef

_pytest/nodes.py

@@ -0,0 +1,37 @@
+import py
+
+
+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(py.path.local.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

changelog/2836.bug

@@ -1,3 +1 @@
-Attempted fix for https://github.com/pytest-dev/pytest/issues/2836
-
-Improves the handling of "nodeid"s to be more aware of path and class separators.
+Match fixture paths against actual path segments in order to avoid matching folders which share a prefix.

testing/test_fixtures.py renamed to testing/test_nodes.py

@@ -1,6 +1,6 @@
 import pytest
 
-from _pytest import fixtures
+from _pytest import nodes
 
 
 @pytest.mark.parametrize("baseid, nodeid, expected", (
@@ -13,6 +13,6 @@
     ('foo/bar::TestBaz::()', 'foo/bar::TestBop::()', False),
     ('foo/bar', 'foo/bar::TestBop::()', True),
 ))
-def test_fixturemanager_ischildnode(baseid, nodeid, expected):
-    result = fixtures.FixtureManager._ischildnode(baseid, nodeid)
+def test_ischildnode(baseid, nodeid, expected):
+    result = nodes.ischildnode(baseid, nodeid)
     assert result is expected