From 79ff930335e4787483324199fcc45db2a79b787d Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 27 Jun 2024 20:04:01 +0330 Subject: [PATCH 1/7] Do the change --- src/_pytest/python.py | 10 ++++++++ testing/example_scripts/issue_519.py | 4 ++-- testing/python/fixtures.py | 34 ++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9182ce7dfe9..64c43bdd0c4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -464,6 +464,7 @@ def _genfunctions(self, name: str, funcobj) -> Iterator[Function]: if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: + metafunc._recompute_direct_params_indices() # Direct parametrizations taking place in module/class-specific # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure # we update what the function really needs a.k.a its fixture closure. Note that @@ -1131,6 +1132,8 @@ def __init__( # Result of parametrize(). self._calls: list[CallSpec2] = [] + self._params_directness: dict[str, Literal["indirect", "direct"]] = {} + def parametrize( self, argnames: str | Sequence[str], @@ -1273,6 +1276,7 @@ def parametrize( name2pseudofixturedef_key, default ) arg_directness = self._resolve_args_directness(argnames, indirect) + self._params_directness.update(arg_directness) for argname in argnames: if arg_directness[argname] == "indirect": continue @@ -1445,6 +1449,12 @@ def _validate_if_using_arg_names( pytrace=False, ) + def _recompute_direct_params_indices(self): + for argname, param_type in self._params_directness.items(): + if param_type == "direct": + for i, callspec in enumerate(self._calls): + callspec.indices[argname] = i + def _find_parametrized_scope( argnames: Sequence[str], diff --git a/testing/example_scripts/issue_519.py b/testing/example_scripts/issue_519.py index 138c07e95be..da5f5ad6aa9 100644 --- a/testing/example_scripts/issue_519.py +++ b/testing/example_scripts/issue_519.py @@ -23,13 +23,13 @@ def checked_order(): assert order == [ ("issue_519.py", "fix1", "arg1v1"), ("test_one[arg1v1-arg2v1]", "fix2", "arg2v1"), - ("test_one[arg1v1-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v1-arg2v1]", "fix2", "arg2v1"), + ("test_one[arg1v1-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v1-arg2v2]", "fix2", "arg2v2"), ("issue_519.py", "fix1", "arg1v2"), ("test_one[arg1v2-arg2v1]", "fix2", "arg2v1"), - ("test_one[arg1v2-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v2-arg2v1]", "fix2", "arg2v1"), + ("test_one[arg1v2-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v2-arg2v2]", "fix2", "arg2v2"), ] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index bc091bb1f27..d05fd2096d6 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4878,3 +4878,37 @@ def test_result(): ) result = pytester.runpytest() assert result.ret == 0 + + +def test_reordering_in_multiple_parametrization(pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + @pytest.mark.parametrize("arg2", [3, 4]) + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test1(arg1, arg2): + pass + + def test2(): + pass + + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test3(arg1): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ] + ) From 5d038d922f4ee6709f537007bf5347e93d0c2eec Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 27 Jun 2024 20:24:35 +0330 Subject: [PATCH 2/7] Remove redundant test --- testing/python/fixtures.py | 34 ---------------------------------- testing/python/metafunc.py | 6 +++--- 2 files changed, 3 insertions(+), 37 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index d05fd2096d6..bc091bb1f27 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4878,37 +4878,3 @@ def test_result(): ) result = pytester.runpytest() assert result.ret == 0 - - -def test_reordering_in_multiple_parametrization(pytester: Pytester) -> None: - pytester.makepyfile( - """ - import pytest - @pytest.mark.parametrize("arg2", [3, 4]) - @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') - def test1(arg1, arg2): - pass - - def test2(): - pass - - @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') - def test3(arg1): - pass - """ - ) - result = pytester.runpytest("--collect-only") - result.stdout.re_match_lines( - [ - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - ] - ) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 2dd85607e71..be224d9e20b 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1005,14 +1005,14 @@ def test3(arg1): result.stdout.re_match_lines( [ r" ", - r" ", r" ", + r" ", + r" ", r" ", + r" ", r" ", - r" ", r" ", r" ", - r" ", r" ", ] ) From e7e7719d4a39b14630263c90c8638947a6b02eb6 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 27 Jun 2024 20:04:01 +0330 Subject: [PATCH 3/7] Do the change --- src/_pytest/python.py | 10 ++++++++ testing/example_scripts/issue_519.py | 4 ++-- testing/python/fixtures.py | 34 ++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9182ce7dfe9..64c43bdd0c4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -464,6 +464,7 @@ def _genfunctions(self, name: str, funcobj) -> Iterator[Function]: if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: + metafunc._recompute_direct_params_indices() # Direct parametrizations taking place in module/class-specific # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure # we update what the function really needs a.k.a its fixture closure. Note that @@ -1131,6 +1132,8 @@ def __init__( # Result of parametrize(). self._calls: list[CallSpec2] = [] + self._params_directness: dict[str, Literal["indirect", "direct"]] = {} + def parametrize( self, argnames: str | Sequence[str], @@ -1273,6 +1276,7 @@ def parametrize( name2pseudofixturedef_key, default ) arg_directness = self._resolve_args_directness(argnames, indirect) + self._params_directness.update(arg_directness) for argname in argnames: if arg_directness[argname] == "indirect": continue @@ -1445,6 +1449,12 @@ def _validate_if_using_arg_names( pytrace=False, ) + def _recompute_direct_params_indices(self): + for argname, param_type in self._params_directness.items(): + if param_type == "direct": + for i, callspec in enumerate(self._calls): + callspec.indices[argname] = i + def _find_parametrized_scope( argnames: Sequence[str], diff --git a/testing/example_scripts/issue_519.py b/testing/example_scripts/issue_519.py index 138c07e95be..da5f5ad6aa9 100644 --- a/testing/example_scripts/issue_519.py +++ b/testing/example_scripts/issue_519.py @@ -23,13 +23,13 @@ def checked_order(): assert order == [ ("issue_519.py", "fix1", "arg1v1"), ("test_one[arg1v1-arg2v1]", "fix2", "arg2v1"), - ("test_one[arg1v1-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v1-arg2v1]", "fix2", "arg2v1"), + ("test_one[arg1v1-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v1-arg2v2]", "fix2", "arg2v2"), ("issue_519.py", "fix1", "arg1v2"), ("test_one[arg1v2-arg2v1]", "fix2", "arg2v1"), - ("test_one[arg1v2-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v2-arg2v1]", "fix2", "arg2v1"), + ("test_one[arg1v2-arg2v2]", "fix2", "arg2v2"), ("test_two[arg1v2-arg2v2]", "fix2", "arg2v2"), ] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index bc091bb1f27..d05fd2096d6 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4878,3 +4878,37 @@ def test_result(): ) result = pytester.runpytest() assert result.ret == 0 + + +def test_reordering_in_multiple_parametrization(pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + @pytest.mark.parametrize("arg2", [3, 4]) + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test1(arg1, arg2): + pass + + def test2(): + pass + + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test3(arg1): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ] + ) From 859979fa54b79bb59c4f8b6d5b2ec9978536722c Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 27 Jun 2024 20:24:35 +0330 Subject: [PATCH 4/7] Remove redundant test --- testing/python/fixtures.py | 34 ---------------------------------- testing/python/metafunc.py | 6 +++--- 2 files changed, 3 insertions(+), 37 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index d05fd2096d6..bc091bb1f27 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4878,37 +4878,3 @@ def test_result(): ) result = pytester.runpytest() assert result.ret == 0 - - -def test_reordering_in_multiple_parametrization(pytester: Pytester) -> None: - pytester.makepyfile( - """ - import pytest - @pytest.mark.parametrize("arg2", [3, 4]) - @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') - def test1(arg1, arg2): - pass - - def test2(): - pass - - @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') - def test3(arg1): - pass - """ - ) - result = pytester.runpytest("--collect-only") - result.stdout.re_match_lines( - [ - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - ] - ) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 2dd85607e71..be224d9e20b 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1005,14 +1005,14 @@ def test3(arg1): result.stdout.re_match_lines( [ r" ", - r" ", r" ", + r" ", + r" ", r" ", + r" ", r" ", - r" ", r" ", r" ", - r" ", r" ", ] ) From eb0e8decc2252125d4fd654fc4366af8683d091a Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 7 Jul 2024 23:56:36 +0330 Subject: [PATCH 5/7] Add changelog --- changelog/12008.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/12008.bugfix.rst diff --git a/changelog/12008.bugfix.rst b/changelog/12008.bugfix.rst new file mode 100644 index 00000000000..f854c36e183 --- /dev/null +++ b/changelog/12008.bugfix.rst @@ -0,0 +1 @@ +In PR#11220, an unintended change in reordering was introduced by changing the way indices were assigned to direct params. More specifically, before that change, the indices of direct params to metafunc's callspecs were assigned after all parametrizations took place. Now, that change is reverted. From 179f7bd100acc8fb9982e34175b0105003086c37 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 13 Jul 2024 12:24:29 -0300 Subject: [PATCH 6/7] Update changelog/12008.bugfix.rst --- changelog/12008.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/12008.bugfix.rst b/changelog/12008.bugfix.rst index f854c36e183..b9680b89236 100644 --- a/changelog/12008.bugfix.rst +++ b/changelog/12008.bugfix.rst @@ -1 +1 @@ -In PR#11220, an unintended change in reordering was introduced by changing the way indices were assigned to direct params. More specifically, before that change, the indices of direct params to metafunc's callspecs were assigned after all parametrizations took place. Now, that change is reverted. +In :pr:`11220`, an unintended change in reordering was introduced by changing the way indices were assigned to direct params. More specifically, before that change, the indices of direct params to metafunc's callspecs were assigned after all parametrizations took place. Now, that change is reverted. From 88d655bc610879e0e1ed569a19f06855892042f5 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 25 Jul 2024 15:41:09 +0330 Subject: [PATCH 7/7] Update src/_pytest/python.py Co-authored-by: Bruno Oliveira --- src/_pytest/python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 64c43bdd0c4..094113bd1b4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1449,7 +1449,7 @@ def _validate_if_using_arg_names( pytrace=False, ) - def _recompute_direct_params_indices(self): + def _recompute_direct_params_indices(self) -> None: for argname, param_type in self._params_directness.items(): if param_type == "direct": for i, callspec in enumerate(self._calls):