Parameterized tests with generator as data source fail on second pytest.main() call

[steadyfirmness]

Description:

When using a generator function as the data source for a parameterized test, the second call to pytest.main() fails with an ID mismatch error. The test works correctly on the first run but fails on subsequent runs.

How to reproduce:

Create a test file with the following content:

import pytest

def data_generator():
    yield 1
    yield 2

@pytest.mark.parametrize("bar", data_generator(), ids=lambda x: f"dynamic_{x}")
def test_foo(bar):
    pass

if __name__ == '__main__':
    base_cmd = [
        "-q",
        "--collect-only"
    ]
    pytest.main(base_cmd)  # First run - works
    pytest.main(base_cmd)  # Second run - fails

Run the test file

Actual behavior:

First run succeeds:

tests/test_bar.py::test_foo[dynamic_1]
tests/test_bar.py::test_foo[dynamic_2]

Second run fails with:

ERROR collecting tests/test_bar.py
In test_foo: 1 parameter sets specified, with different number of ids: 2

Expected behavior:

Both runs should successfully collect and run the tests with the same parameter sets.

Versions:

Python: 3.11

pytest: 8.3.5

OS: macos

Additional context:

The issue appears to be related to the generator being exhausted after the first run. When the test collection happens again in the second pytest.main() call, the generator has already been consumed, leading to the ID mismatch error.

A workaround is to convert the generator to a list:

@pytest.mark.parametrize("bar", list(data_generator()), ids=lambda x: f"dynamic_{x}")

However, this defeats the purpose of using a generator for memory efficiency with large datasets.

[RonnyPfannschmidt]

Pytest always evaluates those eagerly

So the memory argument is a strawman

Please use lists

[bluetech]

Coming from #13742, @andrewwillowen demonstrated a perhaps more severe instance of this (I minimized it slightly):

import pytest

@pytest.mark.parametrize("n", iter(range(1)))
class Test:
    def test_1(self, n):
        pass

    def test_2(self, n):
        pass

I'm reopening the issue since the behavior on the above is quite perilous, it skips test_2. I think we should try to improve this if possible.

[bluetech]

Some possible solutions:

1. Just document this gotcha, otherwise keep as is (pr Add warning about generators for parametrizing class or module #13742)

2. Deprecate and eventually stop accepting non-Sequence (or whatever suitable ABC) argvalues, including in the typing

3. Cache the iterator evaluation. I made a very ugly hack below to cache on the mark itself to see if there are any issues (it seems to work, and doesn't break any tests), but needs a cleaner way.

Hacky caching

diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index 3f9da0267..133eecc09 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -111,7 +111,17 @@ def pytest_addoption(parser: Parser) -> None:
 
 def pytest_generate_tests(metafunc: Metafunc) -> None:
     for marker in metafunc.definition.iter_markers(name="parametrize"):
-        metafunc.parametrize(*marker.args, **marker.kwargs, _param_mark=marker)
+        if len(marker.args) >= 2:
+            args = list(marker.args)
+            if not hasattr(marker, "_argvalues_cache"):
+                if not isinstance(args[1], Sequence):
+                    args[1] = list(args[1])
+                object.__setattr__(marker, "_argvalues_cache", args[1])
+            else:
+                args[1] = marker._argvalues_cache
+        else:
+            args = marker.args
+        metafunc.parametrize(*args, **marker.kwargs, _param_mark=marker)

I find 2 tempting, it arguably was a mistake to allow iterators here. Interestingly there is an old issue #122 which discusses this and updated the parametrize marker doc to clarify only list and tuple are accepted (which we actually never updated...).

But probably it would be too much breakage, especially since it would break stuff like map and zip (but not range).

Any opinions/other ideas?

[The-Compiler]

WDYT about special-casing the case of "exhausted generator" in the code generating (hah!) the "got empty parameter set" skip, and turning it into a warning instead?

[RonnyPfannschmidt]

lets warn on any iterator

[bluetech]

WDYT about special-casing the case of "exhausted generator" in the code generating (hah!) the "got empty parameter set" skip, and turning it into a warning instead?

I suspect it might not be desirable because there might be cases where the original iterator being empty makes sense. Like if it's generated dynamically or conditionally and sometimes might be legitimately empty.

lets warn on any iterator

I'm thinking that might be the way. It will be a big annoying deprecation but it's just a footgun. And the caching solution can be misleading.