From baf3b12fff71eef93429cb8806132bc749dc3f94 Mon Sep 17 00:00:00 2001 From: Timmy Date: Tue, 12 May 2026 23:02:48 +0100 Subject: [PATCH] test(934): cover the legacy stage-string startup migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests pin down the contract of `migrate_legacy_stage_strings`: rewrite of all pre-934 directory-style strings to clean wire form, the lossy `7_frozen` → backlog + frozen-flag collapse, no-op on already-clean items, idempotence, and graceful behaviour before CRDT init. A test-only `seed_with_raw_stage` helper bypasses the boundary normalisers (which can't produce legacy strings) by writing directly to the CRDT register — the same shape we'll see in real pre-migration data. Co-Authored-By: Claude Opus 4.7 (1M context) --- server/src/crdt_state/write/migrations.rs | 165 ++++++++++++++++++++++ 1 file changed, 165 insertions(+) diff --git a/server/src/crdt_state/write/migrations.rs b/server/src/crdt_state/write/migrations.rs index f94777aa..29d2d233 100644 --- a/server/src/crdt_state/write/migrations.rs +++ b/server/src/crdt_state/write/migrations.rs @@ -266,3 +266,168 @@ pub fn migrate_legacy_stage_strings() { ({frozen_count} of which were '7_frozen' → backlog + frozen=true)" ); } + +#[cfg(test)] +mod stage_migration_tests { + use super::super::super::state::init_for_test; + use super::super::item::write_item; + use super::*; + use crate::crdt_state::read_item; + use crate::pipeline_state::{BranchName, Stage}; + use std::num::NonZeroU32; + + /// Seed a pipeline item with a raw, possibly-legacy stage register value, + /// bypassing the boundary normalisers that production write APIs apply. + /// Inserts via the typed API first so the item is indexed, then directly + /// rewrites the `stage` register to the legacy string. + fn seed_with_raw_stage(story_id: &str, raw_stage: &str) { + // Insert via the typed API so the item exists in state.index. + write_item( + story_id, + &Stage::Backlog, + Some("Migration Test"), + None, + None, + None, + None, + None, + None, + None, + ); + // Then overwrite the stage register with the raw legacy string, + // bypassing `db::normalise_stage_str` / `write_item_str`'s mapping. + let state_mutex = get_crdt().expect("CRDT initialised in test"); + let mut state = state_mutex.lock().unwrap(); + let idx = *state.index.get(story_id).expect("item indexed"); + let raw = raw_stage.to_string(); + apply_and_persist(&mut state, |s| s.crdt.doc.items[idx].stage.set(raw.clone())); + } + + #[test] + fn migrate_rewrites_legacy_directory_strings_to_clean_wire() { + init_for_test(); + // High-numbered IDs to avoid colliding with other tests' globals. + let cases: &[(&str, &str, Stage)] = &[ + ("9501_legacy_upcoming", "0_upcoming", Stage::Upcoming), + ("9502_legacy_backlog", "1_backlog", Stage::Backlog), + ("9503_legacy_coding", "2_current", Stage::Coding), + ( + "9504_legacy_blocked", + "2_blocked", + Stage::Blocked { + reason: String::new(), + }, + ), + ("9505_legacy_qa", "3_qa", Stage::Qa), + ( + "9506_legacy_merge", + "4_merge", + Stage::Merge { + feature_branch: BranchName(String::new()), + commits_ahead: NonZeroU32::new(1).unwrap(), + }, + ), + ( + "9507_legacy_merge_failure", + "4_merge_failure", + Stage::MergeFailure { + reason: String::new(), + }, + ), + ]; + for (id, raw, _) in cases { + seed_with_raw_stage(id, raw); + } + + migrate_legacy_stage_strings(); + + for (id, _, expected_variant) in cases { + let view = read_item(id).expect("item must still exist after migration"); + let projected: Stage = crate::pipeline_state::project_stage(&view) + .expect("projection must succeed after migration"); + assert_eq!( + std::mem::discriminant(&projected), + std::mem::discriminant(expected_variant), + "stage for {id} should project to {expected_variant:?} after migration, got {projected:?}", + ); + } + } + + #[test] + fn migrate_collapses_7_frozen_to_backlog_and_sets_frozen_flag() { + init_for_test(); + let story_id = "9510_legacy_frozen"; + seed_with_raw_stage(story_id, "7_frozen"); + + // Sanity: before migration, the frozen flag is false. + let before = read_item(story_id).expect("seeded item exists"); + assert!(!before.frozen(), "frozen flag should start false"); + + migrate_legacy_stage_strings(); + + let after = read_item(story_id).expect("item must still exist after migration"); + assert!( + matches!(after.stage(), crate::crdt_state::Stage::Backlog), + "7_frozen should collapse to Backlog: got {:?}", + after.stage() + ); + assert!( + after.frozen(), + "frozen flag should be set after 7_frozen migration" + ); + } + + #[test] + fn migrate_leaves_clean_wire_items_untouched() { + init_for_test(); + // Seed two items: one already in clean form, one in legacy form. + write_item( + "9520_already_clean", + &Stage::Coding, + Some("Already Clean"), + None, + None, + None, + None, + None, + None, + None, + ); + seed_with_raw_stage("9521_needs_migration", "2_current"); + + migrate_legacy_stage_strings(); + + // Clean item is unchanged; legacy item is now clean too. + let clean = read_item("9520_already_clean").unwrap(); + let migrated = read_item("9521_needs_migration").unwrap(); + assert!(matches!(clean.stage(), crate::crdt_state::Stage::Coding)); + assert!(matches!(migrated.stage(), crate::crdt_state::Stage::Coding)); + } + + #[test] + fn migrate_is_idempotent() { + init_for_test(); + seed_with_raw_stage("9530_idempotent", "4_merge"); + + migrate_legacy_stage_strings(); + let after_first = read_item("9530_idempotent").unwrap(); + assert!(matches!( + after_first.stage(), + crate::crdt_state::Stage::Merge + )); + + // Second call must be a no-op — the filter pass returns empty. + migrate_legacy_stage_strings(); + let after_second = read_item("9530_idempotent").unwrap(); + assert!(matches!( + after_second.stage(), + crate::crdt_state::Stage::Merge + )); + } + + #[test] + fn migrate_is_noop_when_crdt_not_initialised() { + // Calling before init_for_test should not panic. + migrate_legacy_stage_strings(); + } +}