Repository navigation
Global base-path cache leaks between tests #385
Description
Summary
gerrit_urls._BASE_PATH_CACHE is a module-level dict with no test isolation.
Tests that populate it leak state into every later test in the same process.
Detail
src/github2gerrit/gerrit_urls.py:29:
_BASE_PATH_CACHE: dict[str, str] = {}Written by _infer_and_cache_base_path (line 96, from lines 127 and 149) and
directly at lines 187 and 194. Nothing resets it between tests except
tests/test_gerrit_urls_more.py, which clears it explicitly at lines 23 and
212 — an opt-in that other files do not repeat.
gerrit.example.org is used as a fixture host across roughly twenty test
files, and tests/test_gerrit_urls_more.py:195-229 deliberately caches the
base path r for that host via mocked redirects. Any later test that builds a
URL for the same host then gets /r/ inserted.
Observed symptom
Seen intermittently during work on #380 / #381:
FAILED tests/test_core_gerrit_rest_results.py::test_query_gerrit_results_success_base_path
At index 0 diff:
'https://gerrit.example.org/r/c/releng/builder/+/12345' !=
'https://gerrit.example.org/c/releng/builder/+/12345'
The test guards the environment carefully — it deletes GERRIT_HTTP_BASE_PATH,
GERRIT_HTTP_USER and GERRIT_HTTP_PASSWORD via monkeypatch — but has no way
to guard the module-level cache.
The failure did not reproduce on 12 consecutive full-suite runs of the final
tree in #384, so it is not tied to that change. It is latent, and adding or
reordering tests can surface it at any time.
Suggested fix
Add an autouse fixture in tests/conftest.py that clears the cache around every
test:
@pytest.fixture(autouse=True)
def _reset_base_path_cache():
from github2gerrit import gerrit_urls
gerrit_urls._BASE_PATH_CACHE.clear()
yield
gerrit_urls._BASE_PATH_CACHE.clear()The explicit clears in tests/test_gerrit_urls_more.py then become redundant.
Worth auditing for sibling cases while in the area: ssh_config_parser has
_get_cached_git_user_email and _get_respect_user_ssh_setting, which cache
process-wide for the same reason and carry the same isolation risk.
Acceptance criteria
- Base-path cache resets between tests
- Other process-wide caches audited for the same leak
- Redundant per-file clears removed
Activity
ModeSevenIndustrialSolutions commentedon Aug 24, 2026
Root cause of the intermittency identified:
pytest-randomlyis installed and active.Test order is reshuffled on every run, so whether
tests/test_gerrit_urls_more.pyexecutes before
tests/test_core_gerrit_rest_results.py— and therefore whetherthe cached
rbase path forgerrit.example.orgis still present — is decidedby the random seed.
This explains the observed behaviour exactly: roughly one failure in four, with
no relationship to the change under test, and non-reproducible on reruns of an
identical tree.
Two consequences worth noting:
The failure is reproducible on demand with
-p no:randomlyand an explicitordering, which makes this straightforward to write a regression test for:
Any other module-level cache carries the same exposure, and randomised
ordering means such bugs surface as unattributable flakes rather than
consistent failures. The
ssh_config_parsercaches mentioned above are worthchecking on the same pass.
The autouse-fixture fix proposed in the issue body still applies;
pytest-randomlyis the reason the latent bug becomes visible rather than the bug itself.
ModeSevenIndustrialSolutions commentedon Aug 24, 2026
Polluters identified precisely, and the fix is included in #384 rather than deferred — the new tests there changed the
pytest-randomlyshuffle enough to make this flake frequent, so leaving it would have destabilised CI.Instrumenting the cache with an autouse spy named the two writers:
Both mock a 302 with a
/r/...Location, which_infer_and_cache_base_pathwrites into_BASE_PATH_CACHEforgerrit.example.org. Neither cleans up, so every later test using that fixture host inherits a/rbase path.Fixed with the autouse fixture proposed above, in
tests/conftest.py, clearing on both sides of each test. Measured before and after over 8 randomised full-suite runs each: 1 failure before, 0 after.Remaining work for this issue, left open deliberately:
ssh_config_parser._get_cached_git_user_email,_get_respect_user_ssh_setting) for the same exposure._BASE_PATH_CACHE.clear()calls intests/test_gerrit_urls_more.py(lines 23 and 212).ModeSevenIndustrialSolutions commentedon Aug 24, 2026
Correction to my previous comment: the
pytest-randomlyattribution was wrong.That plugin is not a dependency of this repository.
grep -rn randomly pyproject.toml uv.lockreturns nothing, and:The plugin list I quoted came from
/Users/mwatkins/.pyenv/shims/pytest, which is whatuv run pytestresolves to on my machine becausepytestis not a project dependency anduv runfalls through toPATH. My pyenv installation haspytest-randomlyglobally. I read a local-environment detail as a repository fact; apologies for the noise.What this changes:
-p no:randomlyreproduction recipe in my earlier comment does not apply. Reproduce by controlling module order directly instead.What does not change:
test_discover_base_path_3xx_location_relative_and_absoluteandtest_discover_base_path_relative_location_without_leading_slash, both intests/test_gerrit_urls_more.py, cachingrforgerrit.example.org.ssh_config_parsercaches, drop the now-redundant explicitclear()calls, and consider whether host-keyed module state should be injected rather than global.Fix: Reset process-wide caches between tests