Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve param indices using param values, not parameterset index #11257

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9791e1d
Apply comments, rebase and a few improvements
sadra-barikbin Jul 28, 2023
ae5849f
Do the improvement
sadra-barikbin Jul 28, 2023
da499bb
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Aug 10, 2023
9b10ae7
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Dec 15, 2023
30d3231
Add a tiny test
sadra-barikbin Dec 15, 2023
70ca754
Add a test
sadra-barikbin Dec 17, 2023
752d23d
Change a test
sadra-barikbin Dec 18, 2023
d33f65d
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Dec 18, 2023
af390f5
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Dec 18, 2023
82a4e0b
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Dec 18, 2023
d5bf27c
Remove a test
sadra-barikbin Dec 28, 2023
18b7147
Revert a tiny change
sadra-barikbin Dec 29, 2023
2f33561
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Dec 29, 2023
49322ce
Remove a redundant test
sadra-barikbin Dec 29, 2023
ed6c38c
Merge remote-tracking branch 'refs/remotes/upstream/Improvement-catch…
sadra-barikbin Dec 29, 2023
8361ea4
Improve docstring and type annotation
sadra-barikbin Jan 7, 2024
3a3697d
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Jan 7, 2024
b5a7b1a
Fix a test
sadra-barikbin Jan 9, 2024
909f695
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
sadra-barikbin Jan 9, 2024
7efd12b
Merge branch 'main' into Improvement-catch-duplicate-values-when-dete…
obestwalter Jun 20, 2024
f86aa30
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 20, 2024
23cba90
Resolve conflict; Add changelog
sadra-barikbin Oct 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/11257.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When user gives parametersets to :ref:`pytest.mark.parametrize`, try to give the same param index to the same occurrences of a parameter in the sets.
68 changes: 60 additions & 8 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -1050,12 +1050,12 @@
id: str,
marks: Iterable[Mark | MarkDecorator],
scope: Scope,
param_index: int,
param_indices: tuple[int, ...],
) -> CallSpec2:
params = self.params.copy()
indices = self.indices.copy()
arg2scope = dict(self._arg2scope)
for arg, val in zip(argnames, valset):
for arg, val, param_index in zip(argnames, valset, param_indices):
if arg in params:
raise ValueError(f"duplicate parametrization of {arg!r}")
params[arg] = val
Expand Down Expand Up @@ -1084,7 +1084,57 @@
return request.param


# Used for storing pseudo fixturedefs for direct parametrization.
def resolve_values_indices_in_parametersets(
argnames: Sequence[str],
parametersets: Sequence[ParameterSet],
) -> list[tuple[int, ...]]:
"""Resolve indices of the values in parameter sets. The index of a value is determined by
where the value first appears in the existing values of the argname. In other words, given
a subset of the cross-product of some ordered sets, it substitutes the values in the subset
members with their index in the respective sets. For example, given ``argnames`` and
``parametersets`` below, the result would be:
::
argnames = ["A", "B", "C"]
parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")]
result = [(0, 0, 0), (0, 1, 0), (0, 2, 1)]

Result is used in reordering tests to keep items using the same fixture close together.

:param argnames:
Argument names passed to ``metafunc.parametrize()``.
:param parametersets:
The parameter sets, each containing a set of values corresponding
to ``argnames``.
:returns:
List of tuples of indices, each tuple for a parameter set.
"""
indices: list[list[int]] = []
argname_value_indices_for_hashable_ones: dict[str, dict[object, int]] = defaultdict(
dict
)
argvalues_count: dict[str, int] = defaultdict(int)
for i, argname in enumerate(argnames):
argname_indices = []
for parameterset in parametersets:
value = parameterset.values[i]
try:
argname_indices.append(
argname_value_indices_for_hashable_ones[argname][value]
)
except KeyError: # New unique value
argname_value_indices_for_hashable_ones[argname][value] = (
argvalues_count[argname]
)
argname_indices.append(argvalues_count[argname])
argvalues_count[argname] += 1
except TypeError: # `value` is not hashable
argname_indices.append(argvalues_count[argname])
argvalues_count[argname] += 1

Check warning on line 1132 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L1130-L1132

Added lines #L1130 - L1132 were not covered by tests
sadra-barikbin marked this conversation as resolved.
Show resolved Hide resolved
indices.append(argname_indices)
return list(zip(*indices))


# Used for storing artificial fixturedefs for direct parametrization.
name2pseudofixturedef_key = StashKey[Dict[str, FixtureDef[Any]]]()


Expand Down Expand Up @@ -1239,13 +1289,15 @@
ids = self._resolve_parameter_set_ids(
argnames, ids, parametersets, nodeid=self.definition.nodeid
)

param_indices_list = resolve_values_indices_in_parametersets(
argnames, parametersets
)
# Store used (possibly generated) ids with parametrize Marks.
if _param_mark and _param_mark._param_ids_from and generated_ids is None:
object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids)

# Add funcargs as fixturedefs to fixtureinfo.arg2fixturedefs by registering
# artificial "pseudo" FixtureDef's so that later at test execution time we can
# artificial FixtureDef's so that later at test execution time we can
# rely on a proper FixtureDef to exist for fixture setup.
node = None
# If we have a scope that is higher than function, we need
Expand Down Expand Up @@ -1302,16 +1354,16 @@
# of all calls.
newcalls = []
for callspec in self._calls or [CallSpec2()]:
for param_index, (param_id, param_set) in enumerate(
zip(ids, parametersets)
for param_id, param_set, param_indices in zip(
ids, parametersets, param_indices_list
):
newcallspec = callspec.setmulti(
argnames=argnames,
valset=param_set.values,
id=param_id,
marks=param_set.marks,
scope=scope_,
param_index=param_index,
param_indices=param_indices,
)
newcalls.append(newcallspec)
self._calls = newcalls
Expand Down
51 changes: 51 additions & 0 deletions testing/python/metafunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@
from _pytest import python
from _pytest.compat import getfuncargnames
from _pytest.compat import NOTSET
from _pytest.mark import ParameterSet
from _pytest.outcomes import fail
from _pytest.pytester import Pytester
from _pytest.python import Function
from _pytest.python import IdMaker
from _pytest.python import resolve_values_indices_in_parametersets
from _pytest.scope import Scope
import pytest

Expand Down Expand Up @@ -1014,6 +1016,15 @@ def test_parametrize_twoargs(self) -> None:
assert metafunc._calls[1].params == dict(x=3, y=4)
assert metafunc._calls[1].id == "3-4"

def test_parametrize_with_duplicate_values(self) -> None:
metafunc = self.Metafunc(lambda x, y: None)
metafunc.parametrize(("x", "y"), [(1, 2), (3, 4), (1, 5), (2, 2)])
assert len(metafunc._calls) == 4
assert metafunc._calls[0].indices == dict(x=0, y=0)
assert metafunc._calls[1].indices == dict(x=1, y=1)
assert metafunc._calls[2].indices == dict(x=0, y=2)
assert metafunc._calls[3].indices == dict(x=2, y=0)

def test_high_scoped_parametrize_reordering(self, pytester: Pytester) -> None:
pytester.makepyfile(
"""
Expand Down Expand Up @@ -1048,6 +1059,36 @@ def test3(arg1):
]
)

@pytest.mark.parametrize("indirect", [False, True])
def test_high_scoped_parametrize_with_duplicate_values_reordering(
self, indirect: bool, pytester: Pytester
) -> None:
pytester.makepyfile(
f"""
import pytest

@pytest.fixture(scope='module')
def fixture1(request):
pass

@pytest.fixture(scope='module')
def fixture2(request):
pass

@pytest.mark.parametrize("fixture1, fixture2", [("a", 0), ("b", 1), ("a", 2)], indirect={indirect})
def test(fixture1, fixture2):
pass
"""
)
result = pytester.runpytest("--collect-only")
result.stdout.re_match_lines(
[
r" <Function test\[a-0\]>",
r" <Function test\[a-2\]>",
r" <Function test\[b-1\]>",
]
)

def test_parametrize_multiple_times(self, pytester: Pytester) -> None:
pytester.makepyfile(
"""
Expand Down Expand Up @@ -1679,6 +1720,16 @@ def test_3(self, fixture):
]
)

def test_resolve_values_indices_in_parametersets(self, pytester: Pytester) -> None:
indices = resolve_values_indices_in_parametersets(
("a", "b"),
[
ParameterSet.extract_from((a, b))
for a, b in [(1, 1), (1, 2), (2, 3), ([2], 4), ([2], 1)]
],
)
assert indices == [(0, 0), (0, 1), (1, 2), (2, 3), (3, 0)]


class TestMetafuncFunctionalAuto:
"""Tests related to automatically find out the correct scope for
Expand Down
Loading