test(storage): stop lifecycle report tests drifting with new declarations - #3322
Conversation
…ions Problem: three tests in test_index_fast_forward_lifecycle.py monkeypatched by appending a synthetic declaration onto the REAL module-level INDEX_DELTA_DECLARATIONS tuple, then asserted an exact literal invalid_versions/missing_versions result. index_delta_declaration_report() flags any declaration whose version exceeds the version under test, and accumulates missing_versions across the whole expected range -- so every schema version declared after these tests were written (38, 39, 41, 42, 43) silently widened the actual result past the hardcoded literal, or (for the missing_versions case) would have let any future undeclared gap leak into an unrelated assertion. What changed: all three tests now build an isolated declarations tuple (filtered to only the versions each test actually needs, e.g. <=36 or <=INDEX_SCHEMA_VERSION) before monkeypatching lifecycle.INDEX_DELTA_DECLARATIONS, instead of splicing onto the live tuple. This matches the isolation convention already used by other tests in this file (e.g. test_semantic_delta_routes_a_plan_away_from_sql_fast_forward, test_plan_orders_declarations_before_validating_contiguity), so the asserted literals are stable regardless of how many future schema versions get declared. Verification: - devtools test tests/unit/storage/test_index_fast_forward_lifecycle.py -> 9 passed - mypy --strict tests/unit/storage/test_index_fast_forward_lifecycle.py -> Success - ruff check / ruff format --check on the touched file -> clean - Future-proofing proof: temporarily added a throwaway v44 IndexDeltaDeclaration to the real module-level INDEX_DELTA_DECLARATIONS tuple in lifecycle.py and reran the suite. All 3 previously-fixed tests still passed unchanged (their isolated fixtures ignore versions beyond what each test needs). Only test_current_index_schema_has_a_complete_delta_declaration failed, which is expected/correct: a declaration beyond the current INDEX_SCHEMA_VERSION is itself an invalid state per the report's own semantics, not a test bug. Reverted the throwaway declaration afterward (lifecycle.py has zero diff). Ref polylogue-z2fj Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThree lifecycle tests now filter ChangesLifecycle test isolation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Fixes 3 tests in
tests/unit/storage/test_index_fast_forward_lifecycle.pythat hardcoded exact literalinvalid_versions/missing_versionsresults computed against the real, ever-growing module-levelINDEX_DELTA_DECLARATIONStuple inpolylogue/storage/sqlite/lifecycle.py.Problem
test_nonsemantic_delta_without_operations_is_rejectedandtest_delta_without_a_declared_class_is_rejectedmonkeypatched by appending a synthetic declaration onto the liveINDEX_DELTA_DECLARATIONStuple, then assertedinvalid_versions == (37,).index_delta_declaration_report()flags any declaration whose version exceeds the version under test (current_version=37in these tests), so every schema version declared since these tests were written (38, 39, 41, 42, 43) widened the actual result to(38, 39, 41, 42, 43, 37), breaking the literal assertion.test_schema_policy_rejects_an_index_bump_without_a_delta_declarationassertedmissing_versions == [INDEX_SCHEMA_VERSION + 1], butmissing_versionsaccumulates across the whole expected range (compatibility_floor+1 .. current_version), so any currently-undeclared gap elsewhere in the live tuple (e.g. the v40 gap that existed before polylogue-5h5y/#3319) leaked into the same assertion.These were real, ongoing test-staleness bugs — correct when the declarations tuple was short, silently masking/breaking indefinitely as more versions get declared.
Solution
All three tests now build an isolated declarations tuple — filtered from the live
INDEX_DELTA_DECLARATIONSto only the versions each test actually needs (<= 36for the first two,<= INDEX_SCHEMA_VERSIONfor the third) — before monkeypatchinglifecycle.INDEX_DELTA_DECLARATIONS, instead of splicing a synthetic declaration onto the unbounded live tuple.This matches the isolation convention already used elsewhere in the same file:
test_semantic_delta_routes_a_plan_away_from_sql_fast_forwardandtest_plan_orders_declarations_before_validating_contiguityboth fully replacelifecycle.INDEX_DELTA_DECLARATIONSwith a small, self-contained fixture rather than layering onto the live tuple. Onlylifecycle.py's test file changed — no production code touched.Verification
devtools test tests/unit/storage/test_index_fast_forward_lifecycle.py→9 passedmypy --strict tests/unit/storage/test_index_fast_forward_lifecycle.py→Success: no issues found in 1 source fileruff check/ruff format --checkon the touched file → cleanFuture-proofing proof (per polylogue-z2fj step 5): temporarily added a throwaway
IndexDeltaDeclaration(version=44, ...)to the real module-levelINDEX_DELTA_DECLARATIONStuple inlifecycle.pyand reran the suite. All 3 fixed tests still passed unchanged — their isolated fixtures ignore versions beyond what each test needs, so the extra declaration never leaked in. The only failure wastest_current_index_schema_has_a_complete_delta_declaration, which is expected/correct: a declaration beyond the currentINDEX_SCHEMA_VERSIONis itself an invalid state per the report's own semantics (declaration.version > current_version), not a test bug this PR is scoped to fix. Reverted the throwaway declaration afterward —lifecycle.pyhas zero diff in the final PR.Ref polylogue-z2fj
Summary by CodeRabbit