Repository: pytest-dev/pytest
Pull request: Duplicated parameters in parametrize marker
## Problem Statement

**What's the problem this feature will solve?**
The current behavior of pytest in handling duplicated values in parametrize can lead to confusion regarding the actual parameter values used in tests. This PR aims to address this issue by providing clear and unambiguous parameter identifiers.

[maintainer edit to [link the issue](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue): fixes #11456]

## Solution Description

The proposed solution involves modifying the `make_unique_parameterset_ids` method in the `Python.py` class. This change will ensure that unique identifiers are generated for ParameterSets, eliminating any confusion caused by duplicated values.

- When a parameter identifier ends in a digit, a new identifier will be generated in the format `"{id}_{suffix}"`, where `suffix` is a numeric counter.
- If the new identifier is already present in the set of resolved identifiers, the counter will be incremented until a unique identifier is obtained.
- This approach guarantees the uniqueness of identifiers, even when the original identifiers have digit endings.

Problem statement:
Duplicated parameters in @pytest.mark.parametrize
#### 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:
```python
@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

Hints from issue discussion:
I made a typing mistake: the function definition 
is not def test_a(a): 
but def test_params(a):
Hi @Zac-HD, I would like to pick this issue.
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.
Sure
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.
![image](https://github.com/pytest-dev/pytest/assets/67827730/2b9255e0-5572-49da-b29b-e3af17246b74)

To summarize, this method is buggy:

https://github.com/pytest-dev/pytest/blob/54623f0f33c367e19c4041d30f11f71787e30719/src/_pytest/python.py#L984-L1007

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?
@Zac-HD yeah you are right I think instead of adding counter as a suffix we can add UUID to make testcases name unique
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
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.
@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

Commit message: Bug Fix 11456: Duplicated parameters in @pytest.mark.parametrize

Patch to review:
diff --git a/AUTHORS b/AUTHORS
index f8c66cae945..3c23d227730 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -367,6 +367,7 @@ Tadek Teleżyński
 Takafumi Arakaki
 Taneli Hukkinen
 Tanvi Mehta
+Tanya Agarwal
 Tarcisio Fischer
 Tareq Alayan
 Tatiana Ovary
diff --git a/changelog/11456.bugfix.rst b/changelog/11456.bugfix.rst
new file mode 100644
index 00000000000..06ddea22993
--- /dev/null
+++ b/changelog/11456.bugfix.rst
@@ -0,0 +1,2 @@
+In this, we propose a solution that ensures the generation of unique identifiers for ParameterSets in the make_unique_parameterset_ids method in Python.py class. It appends numeric suffixes to identifiers, ensuring uniqueness even when the original identifiers have digit endings.
+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.
diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index cbb82e39090..0e9bd212699 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -1002,7 +1002,14 @@ def make_unique_parameterset_ids(self) -> List[str]:
             # 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]}"
+                    suffix = ""
+                    if id[-1].isdigit():
+                        suffix = "_"
+                    new_id = f"{id}{suffix}{id_suffixes[id]}"
+                    while new_id in set(resolved_ids):
+                        id_suffixes[id] += 1
+                        new_id = f"{id}{suffix}{id_suffixes[id]}"
+                    resolved_ids[index] = new_id
                     id_suffixes[id] += 1
         return resolved_ids
 
diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py
index 429fb4e4372..d597311ae38 100644
--- a/testing/acceptance_test.py
+++ b/testing/acceptance_test.py
@@ -341,6 +341,45 @@ def test_func(i):
         assert res.ret == 0
         res.stdout.fnmatch_lines(["*1 passed*"])
 
+    def test_direct_addressing_selects_duplicates(self, pytester: Pytester) -> None:
+        p = pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.mark.parametrize("a", [1, 2, 10, 11, 2, 1, 12, 11])
+            def test_func(a):
+                pass
+            """
+        )
+        result = pytester.runpytest(p)
+        result.assert_outcomes(failed=0, passed=8)
+
+    def test_direct_addressing_selects_duplicates_1(self, pytester: Pytester) -> None:
+        p = pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.mark.parametrize("a", [1, 2, 10, 11, 2, 1, 12, 1_1,2_1])
+            def test_func(a):
+                pass
+            """
+        )
+        result = pytester.runpytest(p)
+        result.assert_outcomes(failed=0, passed=9)
+
+    def test_direct_addressing_selects_duplicates_2(self, pytester: Pytester) -> None:
+        p = pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.mark.parametrize("a", ["a","b","c","a","a1"])
+            def test_func(a):
+                pass
+            """
+        )
+        result = pytester.runpytest(p)
+        result.assert_outcomes(failed=0, passed=5)
+
     def test_direct_addressing_notfound(self, pytester: Pytester) -> None:
         p = pytester.makepyfile(
             """


Code review comments:
diff_hunk: @@ -0,0 +1,2 @@
+In this, we propose a solution that ensures the generation of unique identifiers for ParameterSets in the make_unique_parameterset_ids method in Python.py class. It appends numeric suffixes to identifiers, ensuring uniqueness even when the original identifiers have digit endings.
+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.
path: changelog/11456.bugfix.rst
text: @user1:
We want changelog entries to describe the change from our users' perspective, which means focusing on the interface rather than implementation details or internal names.  I'd write something like:

```suggestion
Parametrized tests now *really do* ensure that the ids given to each input are unique - for
example, ``a, a, a0`` now results in ``a1, a2, a0`` instead of the previous (buggy) ``a0, a1, a0``.
This necessarily means changing nodeids where these were previously colliding, and for
readability adds an underscore when non-unique ids end in a number.
```

diff_hunk: @@ -1002,7 +1002,14 @@ def make_unique_parameterset_ids(self) -> List[str]:
             # 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]}"
+                    suffix = ""
+                    if id[-1].isdigit():
+                        suffix = "_"
+                    new_id = f"{id}{suffix}{id_suffixes[id]}"
+                    while new_id in set(resolved_ids):
+                        id_suffixes[id] += 1
+                        new_id = f"{id}{suffix}{id_suffixes[id]}"
+                    resolved_ids[index] = new_id
                     id_suffixes[id] += 1
         return resolved_ids
path: src/_pytest/python.py
text: @user1:
```suggestion
        assert len(resolved_ids) == len(set(resolved_ids)), f"Internal error: {resolved_ids=}"
        return resolved_ids
```
I think that the tests below would have passed before this PR too - we already had the correct _number_ of tests, just without unique IDs.  This assertion would have failed though.