From d3cf7566b8cdfd88074ef7134ec69f4a551a4550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sylvain=20Mari=C3=A9?= Date: Thu, 12 May 2022 11:39:14 +0200 Subject: [PATCH] [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 --- docs/changelog.md | 1 + src/pytest_cases/plugin.py | 62 +++++++++++++++---- .../pytest_extension/issues/test_issue_269.py | 44 +++++++++++++ 3 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 tests/pytest_extension/issues/test_issue_269.py diff --git a/docs/changelog.md b/docs/changelog.md index 74d02281..8fbdc069 100644 --- a/docs/changelog.md +++ b/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 diff --git a/src/pytest_cases/plugin.py b/src/pytest_cases/plugin.py index fabb2b2f..6688592c 100644 --- a/src/pytest_cases/plugin.py +++ b/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=()): """ diff --git a/tests/pytest_extension/issues/test_issue_269.py b/tests/pytest_extension/issues/test_issue_269.py new file mode 100644 index 00000000..44976842 --- /dev/null +++ b/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]" + ]