Skip to content

test: harden Redis-backed test isolation under pytest-xdist (#546)#636

Open
vishal-bala wants to merge 1 commit into
mainfrom
fix/redis-test-isolation-546
Open

test: harden Redis-backed test isolation under pytest-xdist (#546)#636
vishal-bala wants to merge 1 commit into
mainfrom
fix/redis-test-isolation-546

Conversation

@vishal-bala

@vishal-bala vishal-bala commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Motivation

The integration suite runs under pytest -n auto, and now and then a single matrix job fails on a test that passes everywhere else — and passes again on rerun. The most recent example was test_hybrid_query_with_geo_filter returning 0 results instead of 3 on exactly one Python/redis-py/Redis combination. These failures read as version-specific, but they aren't: they're a test-isolation problem wearing a disguise.

The key fact is how the suite isolates workers. Each xdist worker gets its own Redis container, so two tests can only interfere when they run on the same worker and reach for the same Redis key names. For that to actually corrupt a result, two things have to line up:

  1. a shared resource name (a name scoped only to the worker, or a hard-coded one), and
  2. a way for state to outlive its test — cleanup that only runs on the success path, or an index recreated with overwrite=True but no drop=True, leaving stale documents under a reused prefix.

When both hold, one test sees another's leftovers — empty results, inflated counts, dtype mismatches — and which test loses depends entirely on the order the scheduler picked. That's the retry-passing, pseudo-random signature.

#543 introduced a redis_test_name helper that mints per-test-unique names and began migrating fixtures onto it. This PR finishes that migration for the modules that can genuinely cause flakiness, and tightens their recreate/cleanup so leftovers can't survive.

What changed

Recreate group — per-test names + create(overwrite=True, drop=True):

  • test_hybrid.py and test_aggregation.py (these two shared the identical user_index_{worker_id} / v1_{worker_id} — the direct cause of the geo-filter failure), plus test_search_results.py, test_svs_integration.py, and the shared conftest.py index fixtures.

Fixed-name collisions — per-test names + guaranteed teardown:

  • from_existing_complex (hard-coded "test") and no_proactive_module_validation ("my_index") in the sync + async search-index suites.
  • The three test_embedcache_warnings.py tests sharing "test_cache".
  • The shared "doc" prefix / "test-index" name across test_no_proactive_module_checks.py.

Raw worker_id collision — unique names + cleanup:

  • The two test_llmcache.py tests that both created float64_cache_{worker_id} and never cleaned up.

Cluster tests — the one place cross-worker collisions are real, since every cluster test points at a single hard-coded redis://localhost:7001: test_redis_cluster_support.py and test_cluster_pipelining.py now use per-test names, drop=True, and try/finally cleanup.

Scope and follow-ups

Testing

Honest caveat for reviewers: this was developed in an environment without Docker, so I could not run the integration suite. black, isort, and mypy pass. Please run make test and a repeated pytest -n auto pass to confirm the flake is gone before merging — this is a test-only change, so behavior is fully validated by the suite itself.


Note

Low Risk
Changes are confined to the test suite; production library behavior is unchanged.

Overview
Hardens integration tests so parallel pytest-xdist workers and shared Redis/cluster endpoints do not leave stale keys or collide on index/cache names.

Fixtures and tests now use redis_test_name (per-test unique index/prefix/cache names) instead of worker_id-only or hard-coded names like "test" / "my_index" / "test_cache". Shared conftest index fixtures (flat_index, hnsw_index, async variants) and modules such as test_hybrid, test_aggregation, test_search_results, and test_svs_integration adopt that naming and call create(overwrite=True, drop=True) so documents under a reused prefix are cleared before load.

Teardown is tightened with try/finally, delete(drop=True), and cache clear() / aclear() on failure paths (e.g. from_existing_complex, module-validation tests, test_llmcache dtype caches, test_embedcache_warnings). Cluster tests use per-test names and drop=True because all workers share localhost:7001.

Reviewed by Cursor Bugbot for commit c4e6b33. Bugbot is set up for automated code reviews on this repo. Configure here.

Eliminates same-worker state contamination and cross-worker cluster
collisions that caused order-dependent, retry-passing integration test
failures (e.g. test_hybrid_query_with_geo_filter returning 0 results).

Converts worker-scoped and fixed Redis resource names to per-test unique
names via the redis_test_name fixture, and tightens recreate/cleanup so
stale state cannot survive an interrupted or co-located test:

- Recreate group: test_hybrid, test_aggregation, test_search_results,
  test_svs_integration and the shared conftest index fixtures now use
  per-test names and create(overwrite=True, drop=True). test_hybrid and
  test_aggregation previously shared user_index_{worker_id}/v1_{worker_id}.
- Fixed-name collisions: from_existing_complex ("test"),
  no_proactive_module_validation ("my_index"), embedcache warning tests
  ("test_cache"), and the shared "doc" prefix in
  test_no_proactive_module_checks now use per-test names with guaranteed
  teardown.
- llmcache: the two tests sharing float64_cache_{worker_id} now use
  unique names and clean up.
- Cluster tests (shared hard-coded localhost:7001 across workers):
  per-test names, drop=True, and try/finally cleanup.

The SemanticRouter {name}:route_config leak is tracked separately in #634.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jit-ci

jit-ci Bot commented Jun 23, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@vishal-bala vishal-bala added the auto:tests Add or improve existing tests label Jun 23, 2026
@vishal-bala vishal-bala marked this pull request as ready for review June 23, 2026 15:41
@vishal-bala vishal-bala requested review from limjoobin and nkanu17 June 24, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:tests Add or improve existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant