smarie / python-pytest-harvest

Store data created during your `pytest` tests execution, and retrieve it at the end of the session, e.g. for applicative benchmarking purposes.
https://smarie.github.io/python-pytest-harvest/
BSD 3-Clause "New" or "Revised" License
61 stars 8 forks source link

broken test with reruns plugin #64

Open phnmn opened 9 months ago

phnmn commented 9 months ago

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
smarie commented 8 months ago

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 ?

phnmn commented 8 months ago

@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.

smarie commented 8 months ago

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 ?