python-pytest-harvest icon indicating copy to clipboard operation
python-pytest-harvest copied to clipboard

broken test with reruns plugin

Open phnmn opened this issue 2 years ago • 3 comments

When test runs with reruns pytest plugin (eg pytest-rerunfailures) failed test will become broken with "already stored" error:

E               KeyError: "Internal Error - This fixture 'database' was already stored for test id 'test_rerun_harvets.py::test_database'"

example of test_rerun_harvets.py

import pytest
from pytest_harvest import saved_fixture

@pytest.fixture
@saved_fixture
def database():
    return "postgresql"


def test_database(database):
    assert database == "clickhouse"

and run test with 2 reruns

python -m pytest --reruns 2 test_rerun_harvets.py

installed deps:

decopatch==1.4.10
exceptiongroup==1.1.3
iniconfig==2.0.0
makefun==1.15.1
packaging==23.2
pluggy==1.3.0
pytest==7.4.2
pytest-harvest==1.10.4
pytest-rerunfailures==12.0
six==1.16.0
tomli==2.0.1

phnmn avatar Oct 09 '23 12:10 phnmn

Thanks @phnmn for reporting this issue !

I guess that pytest-rerunfailures does not change the test node id when re-running tests ? Indeed we currently have a protection against the same node id trying to save data twice:

https://github.com/smarie/python-pytest-harvest/blob/2c934ecfce26c0eb3dab647269ace9cf58abe0aa/pytest_harvest/fixture_cache.py#L115

Should we issue a warning and allow overwriting previous run's saved data ? This seems risky but maybe it is not, after all ?

smarie avatar Nov 29 '23 13:11 smarie

@smarie, thank you for you response. I can't figure out what case you want to avoid with this protection. In my project I found a workaround for this by using a custom storage implementation.

class HarvestStoreMockDict(OrderedDict):
    def __contains__(self, item):
        return False

HARVEST_STORE = HarvestStoreMockDict()

@pytest.fixture(scope="session")
@saved_fixture(HARVEST_STORE)
def some_fixture():
    ...

And I didn't encounter any problems during this time.

phnmn avatar Nov 29 '23 16:11 phnmn

I can't figure out what case you want to avoid with this protection.

I think that this was more of a way for me to check that the storage logic was indeed creating a distinct slot for each test node. Now that the plugin has been around for years, I know that this was the case :)

So we could remove the protection and instead overwrite the previously saved contents. Would you like to propose a PR for this ?

smarie avatar Nov 30 '23 12:11 smarie