Progression is wrong when using empty string in parametrized tests

[fcollonval]

Description

Using a double decorator pytest.mark.parametrize with one of the value being "" (empty string) results in pytest not printing correctly the progress percentage.

It works:

• If there is only one decorator
• If the empty string is present only in one of the two parameter list
• If customized ids not returning an empty string are used

Environment

Environment Conda
pytest 5.3.4 py36_0
python 3.6.9 h5500b2f_0

pytest and OS versions

Python 3.6.9 :: Anaconda, Inc.
Windows 10 64bits
pytest-5.3.4, py-1.8.1, pluggy-0.13.1

minimal example

Buggy

import pytest

@pytest.mark.parametrize("v", ["", " "])
@pytest.mark.parametrize("w", ["", " "])
def test_dummy(v, w):
    assert not v.strip()
    assert not w.strip()

pytest test_dummy.py -v
===== test session starts ===========================================
platform win32 -- Python 3.6.9, pytest-5.3.4, py-1.8.1, pluggy-0.13.1 -- python.exe
cachedir: .pytest_cache
rootdir:  
collected 4 items

test_dummy.py::test_dummy[] PASSED                               [ 25%]
test_dummy.py::test_dummy[ ] PASSED                              [ 50%]
test_dummy.py::test_dummy[ ] PASSED                              [ 50%]
test_dummy.py::test_dummy[ - ] PASSED                            [ 75%]

====== 4 passed in 0.03s ============================================

Working

import pytest

@pytest.mark.parametrize("v", ["", " "], ids=lambda a: f"'{a}'")
@pytest.mark.parametrize("w", ["", " "], ids=lambda a: f"'{a}'")
def test_dummy(v, w):
    assert not v.strip()
    assert not w.strip()

pytest test_dummy.py -v
===== test session starts ===========================================
platform win32 -- Python 3.6.9, pytest-5.3.4, py-1.8.1, pluggy-0.13.1 -- python.exe
cachedir: .pytest_cache
rootdir:  
collected 4 items

test_dummy.py::test_dummy[''-''] PASSED                                [ 25%]
test_dummy.py::test_dummy[''-' '] PASSED                               [ 50%]
test_dummy.py::test_dummy[' '-''] PASSED                               [ 75%]
test_dummy.py::test_dummy[' '-' '] PASSED                              [100%]

===== 4 passed in 0.03s ==================================================

[blueyed]

Thanks for the report!
#6599 should fix it.

[RonnyPfannschmidt]

this one here has 3 incorrectly generated nodeids,
so duplicate nodeids certainly is a issue, but so is incorrectly generated ones (we shouldnt have duplicate nodeids from this input data

[blueyed]

@RonnyPfannschmidt
Agreed.
When fixing nodeid generation in this case the test with #6599 would need to be adjusted then - or should get adjusted probably already.

[asottile]

I took at stab at fixing the nodeids (at least for empty strings) in #6607 -- it can still be broken via something with __str__ so it's probably worth it to still pursue #6599 in some way

[nicoddemus]

I took at stab at fixing the nodeids (at least for empty strings) in #6607 -- it can still be broken via something with str so it's probably worth it to still pursue #6599 in some way

While #6607 is definitely a step in the right direction, I thought nodeids should be unique. I suspect many other plugins expect them to do so, shouldn't pytest guarantee uniqueness?

[RonnyPfannschmidt]

we should eventually ensure unique nodeid per node type, one of the reasons why xdist uses the collection index and bails out about changed collection order is this detail

[nicoddemus]

we should eventually ensure unique nodeid per node type

Why not ensure global node ids? You see this as a design or implementation issue?

one of the reasons why xdist uses the collection index and bails out about changed collection order is this detail

Right, forgot that detail.

[nicoddemus]

Just remembered other plugins which assume unique ids: lastfailed and stepwise. I suspect many other external plugins also assume that.

[nicoddemus]

it can still be broken via something with str so it's probably worth it to still pursue #6599 in some way

Actually it seems there's no way now after @asottile's fix to produce duplicate ids without hacking node ids inside a plugin explicitly... even with objects with a __str__ method we generate unique ids:

    def test_parametrize_with_custom_repr(self, testdir):
        items = testdir.getitems(
            """\
            
            def same_id(v):
                return 'A'
            
            import pytest

            @pytest.mark.parametrize('v', (1, 1), ids=same_id)
            @pytest.mark.parametrize('w', (1, 1), ids=same_id)
            def test(v, w): ...
            """
        )
        names = {item.name for item in items}
        assert names == {'test[A0-A1]', 'test[A1-A1]', 'test[A1-A0]', 'test[A0-A0]'}

The test above passes on features.