[WIP] Support removal from fixture closure tree to improve compatibility with other plugins (#273)

* Initial test to reproduce the bug.

* Improved compatibility with other `pytest` plugins, in particular `pytest-repeat`, by support removal from fixture closure tree. Fixed #269

* Improved readability of fix for future maintenance. Fixed test.

Co-authored-by: Sylvain MARIE <[email protected]>

Author: smarie

docs/changelog.md

@@ -2,6 +2,7 @@
 
 ### 3.6.12 - type hint fix
 
+ - Improved compatibility with other `pytest` plugins, in particular `pytest-repeat`, by support removal from fixture closure tree. Fixed [#269](https://github.com/smarie/python-pytest-cases/issues/269).
  - Fixed type hint errors detected by `pyright`. Fixed [#270](https://github.com/smarie/python-pytest-cases/issues/270)
 
 ### 3.6.11 - bugfix for pytest-xdist and `get_all_cases` API improvement

src/pytest_cases/plugin.py

@@ -633,23 +633,47 @@ def __setitem__(self, i, o):
         #     full_replace = i == slice(None, None, None)
         # except:  # noqa
         #     full_replace = False
+
+        # Get the existing value(s) that we wish to replace
         ref = list(self)[i]
 
         if o == ref:
             # no change at all: of course we accept.
             return
+
+        if not isinstance(i, slice):
+            # In-place change of a single item: let's be conservative and reject for now
+            # if i == 0:
+            #     self.remove(ref)
+            #     self.insert(0, o)
+            # elif i == len(self) - 1:
+            #     self.remove(ref)
+            #     self.append(o)
+            # else:
+            raise NotImplementedError("Replacing an element in a super fixture closure is not currently implemented. "
+                                      "Please report this issue to the `pytest-cases` project.")
         else:
-            if isinstance(i, slice):
-                if set(o) == set(ref):
-                    # a change is required in the order of fixtures. Ignore but continue
-                    warn("WARNING: An attempt was made to reorder a super fixture closure with unions. This is not yet "
-                         "supported since the partitions use subsets of the fixtures ; please report it so that we can "
-                         "find a suitable solution for your need.")
-                    return
-
-        # At least one change of fixture name is required: not supported, and reject as it is
-        raise NotImplementedError("It is not possible to replace an element in a super fixture closure,"
-                                  "as the partitions inside it do not have the same size")
+            # Replacement of multiple items at once: support reordering (ignored) and removal (actually done)
+            new_set = set(o)
+            ref_set = set(ref)
+            if new_set == ref_set:
+                # A change is required in the order of fixtures. Ignore but continue
+                warn("WARNING: An attempt was made to reorder a super fixture closure with unions. This is not yet "
+                     "supported since the partitions use subsets of the fixtures ; please report it so that we can "
+                     "find a suitable solution for your need.")
+                return
+
+            added = new_set.difference(ref_set)
+            removed = ref_set.difference(new_set)
+            if len(added) == 0:
+                # Pure removal: ok.
+                self.remove_all(removed)
+                return
+            else:
+                # self.append_all(added)
+                # Rather be conservative for now
+                raise NotImplementedError("Adding elements to a super fixture closure with a slice is not currently"
+                                          "implemented. Please report this issue to the `pytest-cases` project.")
 
     def __delitem__(self, i):
         self.remove(self[i])
@@ -695,6 +719,14 @@ def insert(self, index, fixture_name):
         # Finally update self.fixture_defs so that the "list" view reflects the changes in self.tree
         self._update_fixture_defs()
 
+    def append_all(self, fixture_names):
+        """Append various fixture names to the closure"""
+        # appending is natively supported in our tree growing method
+        self.tree.build_closure(tuple(fixture_names))
+
+        # Finally update self.fixture_defs so that the "list" view reflects the changes in self.tree
+        self._update_fixture_defs()
+
     def remove(self, value):
         """
         Try to transparently support removal. Note: since the underlying structure is a tree,
@@ -709,6 +741,14 @@ def remove(self, value):
         # update fixture defs
         self._update_fixture_defs()
 
+    def remove_all(self, values):
+        """Multiple `remove` operations at once."""
+        # remove in the tree
+        self.tree.remove_fixtures(tuple(values))
+
+        # update fixture defs
+        self._update_fixture_defs()
+
 
 def getfixtureclosure(fm, fixturenames, parentnode, ignore_args=()):
     """

tests/pytest_extension/issues/test_issue_269.py

@@ -0,0 +1,44 @@
+import pytest
+from pytest_cases import fixture, parametrize
+
+
+@pytest.fixture
+def __my_repeat_step_number(request):
+    return request.param
+
+
+@pytest.hookimpl(trylast=True)
+def pytest_generate_tests(metafunc):
+    """This hook and fixture above are similar to what happens in pytest-repeat.
+    See https://github.com/smarie/python-pytest-cases/issues/269
+    """
+    if metafunc.function is test_repeat:
+        metafunc.fixturenames.append("__my_repeat_step_number")
+
+        def make_progress_id(i, n=2):
+            return '{0}-{1}'.format(i + 1, n)
+
+        metafunc.parametrize(
+            '__my_repeat_step_number',
+            range(2),
+            indirect=True,
+            ids=make_progress_id
+        )
+
+
+@fixture
+def my_fix():
+    return 2
+
+
+@parametrize("arg", (my_fix,))
+def test_repeat(arg):
+    assert arg == 2
+
+
+def test_synthesis(module_results_dct):
+    """Make sure that two tests were created."""
+    assert list(module_results_dct) == [
+        "test_repeat[my_fix-1-2]",
+        "test_repeat[my_fix-2-2]"
+    ]