Duplicated parameters in @pytest.mark.parametrize

[wolfgang-blaszczyk]

What's the problem this feature will solve?

Make unambigious what the values of the parameters used from parametrize are.

Describe the solution you'd like

When duplicated values are used in parametrize, pytest adds a number after each duplicated value, starting from 0 and this confuses what the actual value of parameter is.

Example:

@pytest.mark.parametrize("a", [1, 2, 10, 11, 2, 1, 12, 11]
def test_params(a):
    print('a:', a)
    assert a > 0

This result in following output from pytest:

test_a_1.py::test_params[10] PASSED                    [ 12%]
test_a_1.py::test_params[20] PASSED                    [ 25%]
test_a_1.py::test_params[10] PASSED                    [ 25%]
test_a_1.py::test_params[110] PASSED                   [ 37%]
test_a_1.py::test_params[21] PASSED                    [ 50%]
test_a_1.py::test_params[11] PASSED                    [ 62%]
test_a_1.py::test_params[12] PASSED                    [ 75%]
test_a_1.py::test_params[111] PASSED                   [ 87%]

It is clear that a has the value 1 the very first time and not 10, so confusing with the real 10 later.
Pytest adds a sequential nbr immediately after each duplicated value and that is very confusing

Alternative Solutions

Do not do this and leave the values as they are, so I would like following output:

1, 2, 10, 11, 2, 1, 12, 11 as in parametrize list

[wolfgang-blaszczyk]

I made a typing mistake: the function definition
is not def test_a(a):
but def test_params(a):

[TanyaAgarwal28]

Hi @Zac-HD, I would like to pick this issue.

[Zac-HD]

Sure, go ahead! I think there are some open questions about what we should do here though, so I'd suggest writing up some design notes here before opening a pull request.

[TanyaAgarwal28]

Sure

[TanyaAgarwal28]

Hi @Zac-HD , I have found the file and the code which is causing above mentioned behaviour.
File Name: Python.py
Method Name: make_unique_parameterset_ids
Code snippet:
if len(resolved_ids) != len(set(resolved_ids)):
id_counts = Counter(resolved_ids)
id_suffixes: Dict[str, int] = defaultdict(int)
for index, id in enumerate(resolved_ids):
if id_counts[id] > 1:
resolved_ids[index] = f"{id}{id_suffixes[id]}"
id_suffixes[id] += 1

Here we can avoid duplicate parameter i.e if 1 is 2 times then it will be displayed in report only 1 time like this.

[Zac-HD]

To summarize, this method is buggy:

pytest/src/_pytest/python.py

Lines 984 to 1007 in 54623f0

	def make_unique_parameterset_ids(self) -> List[str]:
	"""Make a unique identifier for each ParameterSet, that may be used to
	identify the parametrization in a node ID.
	Format is <prm_1_token>-...-<prm_n_token>[counter], where prm_x_token is
	- user-provided id, if given
	- else an id derived from the value, applicable for certain types
	- else <argname><parameterset index>
	The counter suffix is appended only in case a string wouldn't be unique
	otherwise.
	"""
	resolved_ids = list(self._resolve_ids())
	# All IDs must be unique!
	if len(resolved_ids) != len(set(resolved_ids)):
	# Record the number of occurrences of each ID.
	id_counts = Counter(resolved_ids)
	# Map the ID to its next suffix.
	id_suffixes: Dict[str, int] = defaultdict(int)
	# Suffix non-unique IDs to make them unique.
	for index, id in enumerate(resolved_ids):
	if id_counts[id] > 1:
	resolved_ids[index] = f"{id}{id_suffixes[id]}"
	id_suffixes[id] += 1
	return resolved_ids

because it should enforce that len(retval) == len(set(retval)), but it doesn't. Here's what I think we should do:

• If there are no ids ending with a digit, keep the status quo - it's correct this this case, and backwards-compatibility matters.
• If there is an id ending in a digit (even if no collision), use a different algorithm. Do you want to propose one, or for me to make a suggestion?

[TanyaAgarwal28]

@Zac-HD yeah you are right I think instead of adding counter as a suffix we can add UUID to make testcases name unique

[RonnyPfannschmidt]

an uuid would be overkill (going from 1-2 characters to a full uuid would be a ux nightmare)

my understanding is that the disambiguation has a key problem of adding digits when something ends with numbers
a potential workaround could be to just use a-z/A-Z to disambiguate there as a starting point

[TanyaAgarwal28]

We can limit UUID to 2 digits as well because if we will use a-z/A-Z only then what if duplicate numbers are more than 26 then again same values will be repeated.

[RonnyPfannschmidt]

@TanyaAgarwal28 uuids can start with digits, characters is only a starting point its trivial to do a divmod walk if the number is greater than 26 however how realistic is more than 26 items needing disambiguation in a common case, imho its absolutley ok to fail there telling people to get their act about id control together, or at least warn

[TanyaAgarwal28]

Yeah right thanks Can I do the required changes in the code and raise PR?

[Zac-HD]

My suggestion:

• Continue to append numbers rather than letters, because some data-driven parametrizations are very large
• If the id ends in a digit, make then new id f"{id}_{suffix}". If the new id is already in the set of seen ids, increment the suffix number and try again. This is sure to work, and even be fast in practice, because our maximum suffix is the number of duplicate ids we started with.

[TanyaAgarwal28]

Okay I will test this approach first with worst case scenarios