Merge pull request #4319 from blueyed/harden-test_collect_init_tests

Fix handling of duplicate args with regard to Python packages

Author: RonnyPfannschmidt

changelog/4310.bugfix.rst

@@ -0,0 +1 @@
+Fix duplicate collection due to multiple args matching the same packages.

src/_pytest/main.py

@@ -18,7 +18,6 @@
 from _pytest.config import hookimpl
 from _pytest.config import UsageError
 from _pytest.outcomes import exit
-from _pytest.pathlib import parts
 from _pytest.runner import collect_one_node
 
 
@@ -387,6 +386,7 @@ def __init__(self, config):
         self._initialpaths = frozenset()
         # Keep track of any collected nodes in here, so we don't duplicate fixtures
         self._node_cache = {}
+        self._pkg_roots = {}
 
         self.config.pluginmanager.register(self, name="session")
 
@@ -489,30 +489,26 @@ def _collect(self, arg):
 
         names = self._parsearg(arg)
         argpath = names.pop(0).realpath()
-        paths = set()
 
-        root = self
         # Start with a Session root, and delve to argpath item (dir or file)
         # and stack all Packages found on the way.
         # No point in finding packages when collecting doctests
         if not self.config.option.doctestmodules:
+            pm = self.config.pluginmanager
             for parent in argpath.parts():
-                pm = self.config.pluginmanager
                 if pm._confcutdir and pm._confcutdir.relto(parent):
                     continue
 
                 if parent.isdir():
                     pkginit = parent.join("__init__.py")
                     if pkginit.isfile():
-                        if pkginit in self._node_cache:
-                            root = self._node_cache[pkginit][0]
-                        else:
-                            col = root._collectfile(pkginit)
+                        if pkginit not in self._node_cache:
+                            col = self._collectfile(pkginit, handle_dupes=False)
                             if col:
                                 if isinstance(col[0], Package):
-                                    root = col[0]
+                                    self._pkg_roots[parent] = col[0]
                                 # always store a list in the cache, matchnodes expects it
-                                self._node_cache[root.fspath] = [root]
+                                self._node_cache[col[0].fspath] = [col[0]]
 
         # If it's a directory argument, recurse and look for any Subpackages.
         # Let the Package collector deal with subnodes, don't collect here.
@@ -535,28 +531,34 @@ def filter_(f):
             ):
                 dirpath = path.dirpath()
                 if dirpath not in seen_dirs:
+                    # Collect packages first.
                     seen_dirs.add(dirpath)
                     pkginit = dirpath.join("__init__.py")
-                    if pkginit.exists() and parts(pkginit.strpath).isdisjoint(paths):
-                        for x in root._collectfile(pkginit):
-                            yield x
-                            paths.add(x.fspath.dirpath())
-
-                if parts(path.strpath).isdisjoint(paths):
-                    for x in root._collectfile(path):
-                        key = (type(x), x.fspath)
-                        if key in self._node_cache:
-                            yield self._node_cache[key]
-                        else:
-                            self._node_cache[key] = x
+                    if pkginit.exists():
+                        collect_root = self._pkg_roots.get(dirpath, self)
+                        for x in collect_root._collectfile(pkginit):
                             yield x
+                            if isinstance(x, Package):
+                                self._pkg_roots[dirpath] = x
+                if dirpath in self._pkg_roots:
+                    # Do not collect packages here.
+                    continue
+
+                for x in self._collectfile(path):
+                    key = (type(x), x.fspath)
+                    if key in self._node_cache:
+                        yield self._node_cache[key]
+                    else:
+                        self._node_cache[key] = x
+                        yield x
         else:
             assert argpath.check(file=1)
 
             if argpath in self._node_cache:
                 col = self._node_cache[argpath]
             else:
-                col = root._collectfile(argpath)
+                collect_root = self._pkg_roots.get(argpath.dirname, self)
+                col = collect_root._collectfile(argpath)
                 if col:
                     self._node_cache[argpath] = col
             m = self.matchnodes(col, names)
@@ -570,20 +572,20 @@ def filter_(f):
             for y in m:
                 yield y
 
-    def _collectfile(self, path):
+    def _collectfile(self, path, handle_dupes=True):
         ihook = self.gethookproxy(path)
         if not self.isinitpath(path):
             if ihook.pytest_ignore_collect(path=path, config=self.config):
                 return ()
 
-        # Skip duplicate paths.
-        keepduplicates = self.config.getoption("keepduplicates")
-        if not keepduplicates:
-            duplicate_paths = self.config.pluginmanager._duplicatepaths
-            if path in duplicate_paths:
-                return ()
-            else:
-                duplicate_paths.add(path)
+        if handle_dupes:
+            keepduplicates = self.config.getoption("keepduplicates")
+            if not keepduplicates:
+                duplicate_paths = self.config.pluginmanager._duplicatepaths
+                if path in duplicate_paths:
+                    return ()
+                else:
+                    duplicate_paths.add(path)
 
         return ihook.pytest_collect_file(path=path, parent=self)
 

src/_pytest/python.py

@@ -545,11 +545,24 @@ def gethookproxy(self, fspath):
             proxy = self.config.hook
         return proxy
 
-    def _collectfile(self, path):
+    def _collectfile(self, path, handle_dupes=True):
         ihook = self.gethookproxy(path)
         if not self.isinitpath(path):
             if ihook.pytest_ignore_collect(path=path, config=self.config):
                 return ()
+
+        if handle_dupes:
+            keepduplicates = self.config.getoption("keepduplicates")
+            if not keepduplicates:
+                duplicate_paths = self.config.pluginmanager._duplicatepaths
+                if path in duplicate_paths:
+                    return ()
+                else:
+                    duplicate_paths.add(path)
+
+        if self.fspath == path:  # __init__.py
+            return [self]
+
         return ihook.pytest_collect_file(path=path, parent=self)
 
     def isinitpath(self, path):

testing/test_collection.py

@@ -951,26 +951,58 @@ def test_collect_init_tests(testdir):
     result = testdir.runpytest(p, "--collect-only")
     result.stdout.fnmatch_lines(
         [
-            "*<Module '__init__.py'>",
-            "*<Function 'test_init'>",
-            "*<Module 'test_foo.py'>",
-            "*<Function 'test_foo'>",
+            "collected 2 items",
+            "<Package *",
+            "  <Module '__init__.py'>",
+            "    <Function 'test_init'>",
+            "  <Module 'test_foo.py'>",
+            "    <Function 'test_foo'>",
         ]
     )
     result = testdir.runpytest("./tests", "--collect-only")
     result.stdout.fnmatch_lines(
         [
-            "*<Module '__init__.py'>",
-            "*<Function 'test_init'>",
-            "*<Module 'test_foo.py'>",
-            "*<Function 'test_foo'>",
+            "collected 2 items",
+            "<Package *",
+            "  <Module '__init__.py'>",
+            "    <Function 'test_init'>",
+            "  <Module 'test_foo.py'>",
+            "    <Function 'test_foo'>",
+        ]
+    )
+    # Ignores duplicates with "." and pkginit (#4310).
+    result = testdir.runpytest("./tests", ".", "--collect-only")
+    result.stdout.fnmatch_lines(
+        [
+            "collected 2 items",
+            "<Package */tests'>",
+            "  <Module '__init__.py'>",
+            "    <Function 'test_init'>",
+            "  <Module 'test_foo.py'>",
+            "    <Function 'test_foo'>",
+        ]
+    )
+    # Same as before, but different order.
+    result = testdir.runpytest(".", "tests", "--collect-only")
+    result.stdout.fnmatch_lines(
+        [
+            "collected 2 items",
+            "<Package */tests'>",
+            "  <Module '__init__.py'>",
+            "    <Function 'test_init'>",
+            "  <Module 'test_foo.py'>",
+            "    <Function 'test_foo'>",
         ]
     )
     result = testdir.runpytest("./tests/test_foo.py", "--collect-only")
-    result.stdout.fnmatch_lines(["*<Module 'test_foo.py'>", "*<Function 'test_foo'>"])
+    result.stdout.fnmatch_lines(
+        ["<Package */tests'>", "  <Module 'test_foo.py'>", "    <Function 'test_foo'>"]
+    )
     assert "test_init" not in result.stdout.str()
     result = testdir.runpytest("./tests/__init__.py", "--collect-only")
-    result.stdout.fnmatch_lines(["*<Module '__init__.py'>", "*<Function 'test_init'>"])
+    result.stdout.fnmatch_lines(
+        ["<Package */tests'>", "  <Module '__init__.py'>", "    <Function 'test_init'>"]
+    )
     assert "test_foo" not in result.stdout.str()