Skip to content

fix(tests): order-independence pin for S7 migration preflight (#763) - #780

Merged
vybe merged 1 commit into
devfrom
fix/763-s7-test-isolation
May 11, 2026
Merged

fix(tests): order-independence pin for S7 migration preflight (#763)#780
vybe merged 1 commit into
devfrom
fix/763-s7-test-isolation

Conversation

@dolho

@dolho dolho commented May 11, 2026

Copy link
Copy Markdown
Contributor

Summary

4 tests in tests/git-sync/test_s7_reserve_instance_id.py::TestMigrationPreflight fail in the full integration suite but pass in isolation. Root cause is a sys.modules pollution + reimport ordering bug. Fixed in tests/git-sync/conftest.py with an autouse pin that restores the real utils.helpers and evicts the cached db.* subtree before every git-sync test.

Related to #763.

Root cause

tests/test_validation.py:35 and tests/test_watchdog_unit.py:54 execute, at module import time:

sys.modules["utils.helpers"] = _helpers_mod

The stub has utc_now / utc_now_iso / to_utc_iso / parse_iso_timestamp — but no iso_cutoff. The top-level tests/conftest.py preload installs the real utils.helpers first, but either polluter overwrites it permanently when collected ahead of tests/git-sync/.

TestMigrationPreflight then does:

sys.modules.pop("db.migrations", None)
from db.migrations import _find_duplicate_working_branches

The reimport chain hits db/__init__.pydb/schedules.pyfrom utils.helpers import iso_cutoff, ...ImportError, surfaced by the test as pytest.fail("S7 migration helper ... is missing").

Verified by reproducing the exact ImportError outside pytest with python -c "...install stub then reimport db.migrations...".

Fix

Per the issue's recommendation, an order-independence pin in tests/git-sync/conftest.py:

  1. Autouse fixture restores the real utils.helpers if the current sys.modules entry lacks iso_cutoff (the discriminator for the stub).
  2. Same fixture evicts cached db / db.* modules so the next from db.migrations import … reloads db/__init__.py and db/schedules.py against the restored real utils.helpers.

Both steps are idempotent (no-op when state is already clean), cheap, and run before every git-sync test. Integration-tier counterpart to the unit-tier pin added by #714.

Test plan

  • test_s7_reserve_instance_id.py::TestMigrationPreflight passes in isolation — 4 passed
  • test_validation.py → test_s7 passes — 23 passed, 3 skipped (was 4 failures pre-fix)
  • test_watchdog_unit.py → test_s7 passes — 39 passed
  • Full test_s7_reserve_instance_id.py still passes — 12 passed
  • test_validation.py / test_watchdog_unit.py collection errors are pre-existing and unrelated (verified via git stash)
  • Reviewer pass

Files

File Change
tests/git-sync/conftest.py +68 lines: _restore_real_utils_helpers() and _evict_db_module_cache() helpers + autouse _pin_module_state_for_db_reimports fixture

🤖 Generated with Claude Code

Root cause
----------
`tests/test_validation.py:35` and `tests/test_watchdog_unit.py:54`
unconditionally execute

    sys.modules["utils.helpers"] = _helpers_mod

at module import time. The stub has `utc_now`, `utc_now_iso`,
`to_utc_iso`, `parse_iso_timestamp` — but is missing `iso_cutoff`. The
top-level `tests/conftest.py` preload installs the real `utils.helpers`
first, but either polluter overwrites it permanently when collected
ahead of `tests/git-sync/`.

`tests/git-sync/test_s7_reserve_instance_id.py::TestMigrationPreflight`
later does

    sys.modules.pop("db.migrations", None)
    from db.migrations import _find_duplicate_working_branches

The reimport chain hits `db/__init__.py` → `db/schedules.py` →
`from utils.helpers import iso_cutoff, ...` → ImportError, which the
test surfaces as `pytest.fail("S7 migration helper ... is missing")`.

All 4 preflight tests then fail in the full suite. They pass in
isolation because no polluter has run.

Fix
---
Per the issue's recommendation, add an order-independence pin in
`tests/git-sync/conftest.py`:

1. Autouse fixture restores the real `src/backend/utils/helpers.py`
   module if the current `sys.modules["utils.helpers"]` lacks
   `iso_cutoff` (the discriminator for the stub).
2. Same fixture evicts the cached `db` / `db.*` modules so the next
   `from db.migrations import ...` reloads `db/__init__.py` and
   `db/schedules.py` against the restored real `utils.helpers`.

Both steps are idempotent (no-op when state is already clean) and
cheap. They run before every git-sync test, defending against
arbitrary polluter ordering.

This is the integration-tier counterpart to the unit-tier pin added by
issue #714.

Related to #763.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean, minimal test infra fix. Root cause well-documented, fix is idempotent and correctly scoped to git-sync/.

@vybe
vybe merged commit fb6825c into dev May 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants