Consider nonparametrized tests in reordering

src/_pytest/fixtures.py

@@ -70,7 +70,6 @@
     from typing import Deque
 
     from _pytest.main import Session
-    from _pytest.python import CallSpec2
     from _pytest.python import Function
     from _pytest.python import Metafunc
 
@@ -159,45 +158,48 @@ def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]:
 @dataclasses.dataclass(frozen=True)
 class FixtureArgKey:
     argname: str
-    param_index: int
+    param_index: Optional[int]
     scoped_item_path: Optional[Path]
     item_cls: Optional[type]
 
 
-def get_parametrized_fixture_keys(
-    item: nodes.Item, scope: Scope
-) -> Iterator[FixtureArgKey]:
-    """Return list of keys for all parametrized arguments which match
+def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]:
+    """Return list of keys for all arguments which match
     the specified scope."""
     assert scope is not Scope.Function
-    try:
-        callspec = item.callspec  # type: ignore[attr-defined]
-    except AttributeError:
-        pass
-    else:
-        cs: CallSpec2 = callspec
-        # cs.indices is random order of argnames.  Need to
-        # sort this so that different calls to
-        # get_parametrized_fixture_keys will be deterministic.
-        for argname in sorted(cs.indices):
-            if cs._arg2scope[argname] != scope:
+    if hasattr(item, "_fixtureinfo"):
+        for argname in item._fixtureinfo.names_closure:
+            if argname not in item._fixtureinfo.name2fixturedefs:
+                # We can also raise FixtureLookupError
                 continue
+            is_parametrized = (
+                hasattr(item, "callspec") and argname in item.callspec._arg2scope
+            )
+            fixturedef = item._fixtureinfo.name2fixturedefs[argname][-1]
+            # In the case item is parametrized on the `argname` with
+            # a scope, it overrides that of the fixture.
+            if (
+                is_parametrized
+                and cast("Function", item).callspec._arg2scope[argname] == scope
+            ) or (not is_parametrized and fixturedef._scope == scope):
+                param_index = None
+                if is_parametrized:
+                    param_index = cast("Function", item).callspec.indices[argname]
+
+                item_cls = None
+                if scope is Scope.Session:
+                    scoped_item_path = None
+                elif scope is Scope.Package:
+                    scoped_item_path = item.path
+                elif scope is Scope.Module:
+                    scoped_item_path = item.path
+                elif scope is Scope.Class:
+                    scoped_item_path = item.path
+                    item_cls = item.cls  # type: ignore[attr-defined]
+                else:
+                    assert_never(scope)
 
-            item_cls = None
-            if scope is Scope.Session:
-                scoped_item_path = None
-            elif scope is Scope.Package:
-                scoped_item_path = item.path
-            elif scope is Scope.Module:
-                scoped_item_path = item.path
-            elif scope is Scope.Class:
-                scoped_item_path = item.path
-                item_cls = item.cls  # type: ignore[attr-defined]
-            else:
-                assert_never(scope)
-
-            param_index = cs.indices[argname]
-            yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls)
+                yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls)
 
 
 # Algorithm for sorting on a per-parametrized resource setup basis.
@@ -215,7 +217,7 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]:
         item_d: Dict[FixtureArgKey, Deque[nodes.Item]] = defaultdict(deque)
         items_by_argkey[scope] = item_d
         for item in items:
-            keys = dict.fromkeys(get_parametrized_fixture_keys(item, scope), None)
+            keys = dict.fromkeys(get_fixture_keys(item, scope), None)
             if keys:
                 d[item] = keys
                 for key in keys:

testing/_py/test_local.py

@@ -618,6 +618,7 @@ def test_initialize_curdir(self):
 
     @skiponwin32
     def test_chdir_gone(self, path1):
+        original_cwd = os.getcwd()
         p = path1.ensure("dir_to_be_removed", dir=1)
         p.chdir()
         p.remove()
@@ -628,15 +629,18 @@ def test_chdir_gone(self, path1):
         with pytest.raises(error.ENOENT):
             with p.as_cwd():
                 raise NotImplementedError
+        os.chdir(original_cwd)
 
     @skiponwin32
     def test_chdir_gone_in_as_cwd(self, path1):
+        original_cwd = os.getcwd()
         p = path1.ensure("dir_to_be_removed", dir=1)
         p.chdir()
         p.remove()
 
         with path1.as_cwd() as old:
             assert old is None
+        os.chdir(original_cwd)
 
     def test_as_cwd(self, path1):
         dir = path1.ensure("subdir", dir=1)

testing/acceptance_test.py

@@ -6,6 +6,7 @@
 
 import pytest
 from _pytest.config import ExitCode
+from _pytest.monkeypatch import MonkeyPatch
 from _pytest.pathlib import symlink_or_skip
 from _pytest.pytester import Pytester
 
@@ -651,7 +652,7 @@ def test_cmdline_python_package(self, pytester: Pytester, monkeypatch) -> None:
         result.stderr.fnmatch_lines(["*not*found*test_missing*"])
 
     def test_cmdline_python_namespace_package(
-        self, pytester: Pytester, monkeypatch
+        self, pytester: Pytester, monkeypatch: MonkeyPatch
     ) -> None:
         """Test --pyargs option with namespace packages (#1567).
 
@@ -731,6 +732,7 @@ def test_cmdline_python_namespace_package(
         result.stdout.fnmatch_lines(
             ["*test_world.py::test_other*PASSED*", "*1 passed*"]
         )
+        monkeypatch.undo()
 
     def test_invoke_test_and_doctestmodules(self, pytester: Pytester) -> None:
         p = pytester.makepyfile(

testing/python/fixtures.py

@@ -2726,16 +2726,19 @@ def test2(reprovision):
         """
         )
         result = pytester.runpytest("-v")
+        # Order changed because fixture keys were sorted by their names in fixtures::get_fixture_keys
+        # beforehand so encap key came before flavor. This isn't problematic here as both fixtures
+        # are session-scoped but when this isn't the case, it might be problematic.
         result.stdout.fnmatch_lines(
             """
             test_dynamic_parametrized_ordering.py::test[flavor1-vxlan] PASSED
             test_dynamic_parametrized_ordering.py::test2[flavor1-vxlan] PASSED
-            test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED
-            test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED
-            test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED
-            test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED
             test_dynamic_parametrized_ordering.py::test[flavor1-vlan] PASSED
             test_dynamic_parametrized_ordering.py::test2[flavor1-vlan] PASSED
+            test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED
+            test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED
+            test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED
+            test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED
         """
         )
 
@@ -4531,3 +4534,65 @@ def test_fixt(custom):
     result.assert_outcomes(errors=1)
     result.stdout.fnmatch_lines([expected])
     assert result.ret == ExitCode.TESTS_FAILED
+
+
+def test_reorder_with_nonparametrized_fixtures(pytester: Pytester):
+    path = pytester.makepyfile(
+        """
+        import pytest
+
+        @pytest.fixture(scope='module')
+        def a():
+            return "a"
+
+        @pytest.fixture(scope='module')
+        def b():
+            return "b"
+
+        def test_0(a):
+            pass
+
+        def test_1(b):
+            pass
+
+        def test_2(a):
+            pass
+
+        def test_3(b):
+            pass
+
+        def test_4(b):
+            pass
+        """
+    )
+    result = pytester.runpytest(path, "-q", "--collect-only")
+    result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1, 3, 4]])
+
+
+def test_reorder_with_both_parametrized_and_nonparametrized_fixtures(
+    pytester: Pytester,
+):
+    path = pytester.makepyfile(
+        """
+        import pytest
+
+        @pytest.fixture(scope='module',params=[None])
+        def parametrized():
+            yield
+
+        @pytest.fixture(scope='module')
+        def nonparametrized():
+            yield
+
+        def test_0(parametrized, nonparametrized):
+            pass
+
+        def test_1():
+            pass
+
+        def test_2(nonparametrized):
+            pass
+        """
+    )
+    result = pytester.runpytest(path, "-q", "--collect-only")
+    result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1]])

testing/test_argcomplete.py

@@ -67,22 +67,21 @@ def __call__(self, prefix, **kwargs):
 
 class TestArgComplete:
     @pytest.mark.skipif("sys.platform in ('win32', 'darwin')")
-    def test_compare_with_compgen(
-        self, tmp_path: Path, monkeypatch: MonkeyPatch
-    ) -> None:
+    def test_compare_with_compgen(self, tmp_path: Path) -> None:
         from _pytest._argcomplete import FastFilesCompleter
 
         ffc = FastFilesCompleter()
         fc = FilesCompleter()
 
-        monkeypatch.chdir(tmp_path)
+        with MonkeyPatch.context() as mp:
+            mp.chdir(tmp_path)
 
-        assert equal_with_bash("", ffc, fc, out=sys.stdout)
+            assert equal_with_bash("", ffc, fc, out=sys.stdout)
 
-        tmp_path.cwd().joinpath("data").touch()
+            tmp_path.cwd().joinpath("data").touch()
 
-        for x in ["d", "data", "doesnotexist", ""]:
-            assert equal_with_bash(x, ffc, fc, out=sys.stdout)
+            for x in ["d", "data", "doesnotexist", ""]:
+                assert equal_with_bash(x, ffc, fc, out=sys.stdout)
 
     @pytest.mark.skipif("sys.platform in ('win32', 'darwin')")
     def test_remove_dir_prefix(self):

testing/test_collection.py

@@ -244,7 +244,7 @@ def test_custom_norecursedirs(self, pytester: Pytester) -> None:
         rec = pytester.inline_run("xyz123/test_2.py")
         rec.assertoutcome(failed=1)
 
-    def test_testpaths_ini(self, pytester: Pytester, monkeypatch: MonkeyPatch) -> None:
+    def test_testpaths_ini(self, pytester: Pytester) -> None:
         pytester.makeini(
             """
             [pytest]
@@ -275,10 +275,11 @@ def test_testpaths_ini(self, pytester: Pytester, monkeypatch: MonkeyPatch) -> No
 
         # changing cwd to each subdirectory and running pytest without
         # arguments collects the tests in that directory normally
-        for dirname in ("a", "b", "c"):
-            monkeypatch.chdir(pytester.path.joinpath(dirname))
-            items, reprec = pytester.inline_genitems()
-            assert [x.name for x in items] == ["test_%s" % dirname]
+        with MonkeyPatch.context() as mp:
+            for dirname in ("a", "b", "c"):
+                mp.chdir(pytester.path.joinpath(dirname))
+                items, reprec = pytester.inline_genitems()
+                assert [x.name for x in items] == ["test_%s" % dirname]
 
 
 class TestCollectPluginHookRelay:
@@ -660,7 +661,7 @@ def test_global_file(self, pytester: Pytester) -> None:
         for parent in col.listchain():
             assert parent.config is config
 
-    def test_pkgfile(self, pytester: Pytester, monkeypatch: MonkeyPatch) -> None:
+    def test_pkgfile(self, pytester: Pytester) -> None:
         """Verify nesting when a module is within a package.
         The parent chain should match: Module<x.py> -> Package<subdir> -> Session.
             Session's parent should always be None.
@@ -669,7 +670,7 @@ def test_pkgfile(self, pytester: Pytester, monkeypatch: MonkeyPatch) -> None:
         subdir = tmp_path.joinpath("subdir")
         x = ensure_file(subdir / "x.py")
         ensure_file(subdir / "__init__.py")
-        with monkeypatch.context() as mp:
+        with MonkeyPatch.context() as mp:
             mp.chdir(subdir)
             config = pytester.parseconfigure(x)
         col = pytester.getnode(config, x)

testing/test_config.py

@@ -40,26 +40,26 @@ def test_getcfg_and_config(
         tmp_path: Path,
         section: str,
         filename: str,
-        monkeypatch: MonkeyPatch,
     ) -> None:
         sub = tmp_path / "sub"
         sub.mkdir()
-        monkeypatch.chdir(sub)
-        (tmp_path / filename).write_text(
-            textwrap.dedent(
-                """\
-                [{section}]
-                name = value
-                """.format(
-                    section=section
-                )
-            ),
-            encoding="utf-8",
-        )
-        _, _, cfg = locate_config([sub])
-        assert cfg["name"] == "value"
-        config = pytester.parseconfigure(str(sub))
-        assert config.inicfg["name"] == "value"
+        with MonkeyPatch.context() as mp:
+            mp.chdir(sub)
+            (tmp_path / filename).write_text(
+                textwrap.dedent(
+                    """\
+                    [{section}]
+                    name = value
+                    """.format(
+                        section=section
+                    )
+                ),
+                encoding="utf-8",
+            )
+            _, _, cfg = locate_config([sub])
+            assert cfg["name"] == "value"
+            config = pytester.parseconfigure(str(sub))
+            assert config.inicfg["name"] == "value"
 
     def test_setupcfg_uses_toolpytest_with_pytest(self, pytester: Pytester) -> None:
         p1 = pytester.makepyfile("def test(): pass")