Better explanation and implementation for empty ids in pytest 5.4+

Author: Sylvain MARIE

pytest_cases/fixture_parametrize_plus.py

@@ -23,7 +23,7 @@
 
 from .common_mini_six import string_types
 from .common_others import AUTO, robust_isinstance
-from .common_pytest_marks import has_pytest_param, get_param_argnames_as_list, PYTEST421_OR_GREATER
+from .common_pytest_marks import has_pytest_param, get_param_argnames_as_list, PYTEST421_OR_GREATER, PYTEST54_OR_GREATER
 from .common_pytest_lazy_values import is_lazy_value, get_lazy_args
 from .common_pytest import get_fixture_name, remove_duplicates, mini_idvalset, is_marked_parameter_value, \
     extract_parameterset_info, ParameterSet, cart_product_pytest, mini_idval, inject_host, \
@@ -516,9 +516,26 @@ def get_alternative_id(self):
             return mini_idvalset(self.argnames, argval, idx=self.alternative_index)
 
 
-if PYTEST421_OR_GREATER:
+if PYTEST54_OR_GREATER:
+    # an empty string will be taken into account but NOT filtered out in CallSpec2.id.
+    # so instead we create a dedicated unique string and return it.
+    # Ugly but the only viable alternative seems worse: it would be to return an empty string
+    # and in `remove_empty_ids` to always remove all empty strings (not necessary the ones set by us).
+    # That is too much of a change.
+
+    EMPTY_ID = "13h#987__36a721f8Z44526!8*72"
+
+    def remove_empty_ids(callspec):
+        # used by plugin.py to remove the EMPTY_ID from the callspecs
+        callspec._idlist = [c for c in callspec._idlist if c != EMPTY_ID]
+
+elif PYTEST421_OR_GREATER:
+    # an empty string will be taken into account and filtered out in CallSpec2.id.
     EMPTY_ID = ""
+
 else:
+    # an empty string will only be taken into account if its truth value is True
+    # it will be filtered out in CallSpec2.id
     class EmptyId(str):
         def __new__(cls):
             return str.__new__(cls, "")

pytest_cases/plugin.py

@@ -4,7 +4,6 @@
 # License: 3-clause BSD, <https://github.com/smarie/python-pytest-cases/blob/master/LICENSE>
 from collections import OrderedDict, namedtuple
 from copy import copy
-from distutils.version import LooseVersion
 from functools import partial
 from warnings import warn
 
@@ -34,6 +33,9 @@
 
 from .fixture_core1_unions import NOT_USED, is_fixture_union_params, UnionFixtureAlternative
 
+if PYTEST54_OR_GREATER:
+    # we will need to clean the empty ids explicitly in the plugin :'(
+    from .fixture_parametrize_plus import remove_empty_ids
 
 _DEBUG = False
 
@@ -729,19 +731,22 @@ def __str__(self):
 def parametrize(metafunc, argnames, argvalues, indirect=False, ids=None, scope=None, **kwargs):
     """
     This alternate implementation of metafunc.parametrize creates a list of calls that is not just the cartesian
-    product of all parameters (like the pytest behaviour).
-
-    Instead, it offers an alternate list of calls takinginto account all union fixtures.
+    product of all parameters (like the pytest behaviour). Instead, it offers an alternate list of calls taking into
+    account all "union" fixtures.
 
     For this, it replaces the `metafunc._calls` attribute with a `CallsReactor` instance, and feeds it with all
-    parameters and parametrized fixtures independently (not doing any cross-product).
-
-    The resulting `CallsReactor` instance is then able to dynamically behave like the correct list of calls,
-    lazy-creating that list when it is used.
+    parameters and parametrized fixtures independently (not doing any cross-product during this call). The resulting
+    `CallsReactor` instance is then able to dynamically behave like the correct list of calls, lazy-creating that list
+    when it is used.
     """
     if not isinstance(metafunc.fixturenames, SuperClosure):
         # legacy method
         metafunc.__class__.parametrize(metafunc, argnames, argvalues, indirect=indirect, ids=ids, scope=scope, **kwargs)
+
+        # clean EMPTY_ID : since they are never set by us in a normal parametrize, no need to do this here.
+        # if PYTEST54_OR_GREATER:
+        #     for callspec in metafunc._calls:
+        #         remove_empty_ids(callspec)
     else:
         # get or create our special container object
         if not isinstance(metafunc._calls, CallsReactor):  # noqa
@@ -864,6 +869,11 @@ def create_call_list_from_pending_parametrizations(self):
                                                                  c.id, c.funcargs, c.params)
                              for c in calls]) + "\n")
 
+        # clean EMPTY_ID set by @parametrize when there is at least a MultiParamsAlternative
+        if PYTEST54_OR_GREATER:
+            for callspec in calls:
+                remove_empty_ids(callspec)
+
         # save the list and put back self as the _calls facade
         self._call_list = calls
         self.metafunc._calls = self
@@ -1107,12 +1117,6 @@ def _parametrize_calls(metafunc, init_calls, argnames, argvalues, discard_id=Fal
         for callspec in new_calls:
             callspec._idlist.pop(-1)  # noqa
 
-    # Fix in pytest 5.4.0 or greater, empty ids are not filtered out correctly anymore
-    if PYTEST54_OR_GREATER and len(new_calls) > 0:
-        # old_id = type(new_calls[0]).id  # https://github.com/pytest-dev/pytest/blob/5.4.0/src/_pytest/python.py#L798
-        if type(new_calls[0]).id is not id:
-            type(new_calls[0]).id = id
-
     # restore the metafunc and return the new calls
     metafunc._calls = bak
     return new_calls

pytest_cases/tests/cases/doc/test_doc.py

@@ -6,7 +6,7 @@
 
 from pytest_harvest import get_session_synthesis_dct
 from pytest_cases import parametrize_with_cases, AUTO2, fixture, case
-from pytest_cases.common_pytest import has_pytest_param
+from pytest_cases.common_pytest_marks import has_pytest_param
 
 from . import cases_doc
 from .example import foo

pytest_cases/tests/cases/doc/test_generators.py

@@ -7,7 +7,7 @@
 from pytest_harvest import get_session_synthesis_dct
 
 from pytest_cases import parametrize_with_cases, parametrize
-from pytest_cases.common_pytest import has_pytest_param
+from pytest_cases.common_pytest_marks import has_pytest_param
 
 from ...utils import skip
 

pytest_cases/tests/cases/issues/test_issue_142.py

@@ -31,22 +31,10 @@ def test_empty_caseid_both(dummy_amount):
 
 
 def test_synthesis(module_results_dct):
-    if PYTEST421_OR_GREATER:
-        # an empty string id will be considered
-        if PYTEST54_OR_GREATER:
-            # it will even be kept in CallSpec2.id
-            last_id_prefix = "-"
-        else:
-            # it will be filtered out in CallSpec2.id
-            last_id_prefix = ""
-    else:
-        # an empty string will not be considered
-        last_id_prefix = "test_empty_caseid_both_dummy_amount1-"
-
     assert list(module_results_dct) == [
         'test_empty_prefix[<empty_case_id>-1]',
         'test_empty_prefix[<empty_case_id>-0]',
         'test_empty_prefix[<empty_case_id>--1]',
-        'test_empty_caseid_both[%s]' % ("" if PYTEST421_OR_GREATER else "test_empty_caseid_both_dummy_amount0"),
-        'test_empty_caseid_both[%s1]' % last_id_prefix,
+        'test_empty_caseid_both[%s]' % ("0" if PYTEST421_OR_GREATER else "test_empty_caseid_both_dummy_amount0"),
+        'test_empty_caseid_both[%s-1]' % ("1" if PYTEST421_OR_GREATER else "test_empty_caseid_both_dummy_amount1"),
     ]

pytest_cases/tests/pytest_extension/parametrize_plus/test_basics_misc.py

@@ -9,9 +9,9 @@
 from pytest_cases import parametrize, lazy_value
 from pytest_harvest import get_session_synthesis_dct
 
-from pytest_cases.common_pytest import has_pytest_param, cart_product_pytest, get_marked_parameter_values, \
+from pytest_cases.common_pytest import cart_product_pytest, get_marked_parameter_values, \
     extract_parameterset_info, extract_pset_info_single
-from pytest_cases.common_pytest_marks import PYTEST3_OR_GREATER
+from pytest_cases.common_pytest_marks import PYTEST3_OR_GREATER, has_pytest_param
 from pytest_cases.common_pytest_lazy_values import is_lazy
 from pytest_cases.fixture_parametrize_plus import _get_argnames_argvalues
 from ...utils import skip

pytest_cases/tests/pytest_extension/parametrize_plus/test_getcallspecs.py

@@ -5,7 +5,9 @@
 import pytest
 
 from pytest_cases import parametrize
-from pytest_cases.common_pytest import get_callspecs, has_pytest_param
+from pytest_cases.common_pytest import get_callspecs
+from pytest_cases.common_pytest_marks import has_pytest_param
+
 
 if not has_pytest_param:
     @pytest.mark.parametrize('new_style', [False, True])

pytest_cases/tests/utils.py

@@ -4,7 +4,8 @@
 # License: 3-clause BSD, <https://github.com/smarie/python-pytest-cases/blob/master/LICENSE>
 import pytest
 
-from pytest_cases.common_pytest import has_pytest_param
+from pytest_cases.common_pytest_marks import has_pytest_param
+
 
 if has_pytest_param:
     def skip(*argvals):