From 91b7605b9fcb76ae253f0b20e44f38342d9fc8ba Mon Sep 17 00:00:00 2001 From: funman300 Date: Wed, 6 May 2026 18:16:32 -0700 Subject: [PATCH] fix(engine): clear PendingRestoredGame in test_app + harden auto-save flake auto_save_writes_after_30_seconds intermittently failed under heavy parallel cargo-test load. Two contributing factors, both fixable in test fixtures alone: 1. GamePlugin::build() reads dirs::data_dir()/.../game_state.json before per-test resource overrides apply. If a real game_state.json exists on the dev machine, it's loaded into PendingRestoredGame, and auto_save_game_state's pending guard (`pending.0.is_some()`) silently skips the save. test_app now resets PendingRestoredGame(None) after plugin build so the production save state can't leak into per-test world state. 2. Time::delta_secs() on the first MinimalPlugins frame can be 0.0 (nominal) or, under cargo-test parallelism, large enough to consume the 0.1 s pre-seeded margin past the threshold. The test now re-arms AutoSaveTimer(AUTO_SAVE_INTERVAL_SECS + 1.0) every iteration in a 16-frame bounded loop, breaking the moment the file appears. Robust against first-frame Time variance with no behaviour-contract change. No production-code change. Verified: 3 back-to-back single-test runs all pass. Full workspace test suite: 1170 passing / 0 failing. cargo clippy --workspace --all-targets -- -D warnings clean. Co-Authored-By: Claude Opus 4.7 --- solitaire_engine/src/game_plugin.rs | 34 +++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/solitaire_engine/src/game_plugin.rs b/solitaire_engine/src/game_plugin.rs index d1c658f..a107ea4 100644 --- a/solitaire_engine/src/game_plugin.rs +++ b/solitaire_engine/src/game_plugin.rs @@ -1264,6 +1264,14 @@ mod tests { // plugin's build path; clearing them keeps tests self-contained. app.insert_resource(GameStatePath(None)); app.insert_resource(ReplayPath(None)); + // Force `PendingRestoredGame` empty so production saved-game + // state on the dev machine's disk (loaded by `GamePlugin::build`) + // can't leak into per-test world state and trip the + // `pending.0.is_some()` guard in `auto_save_game_state` / + // `save_game_state_on_exit`. Without this clear, an + // unrelated `~/.local/share/solitaire_quest/game_state.json` + // would silently disable the auto-save path under test. + app.insert_resource(PendingRestoredGame(None)); // Override the system-time seed with a known value. app.world_mut() .resource_mut::() @@ -1516,6 +1524,16 @@ mod tests { } /// auto_save_game_state writes to disk once the accumulator crosses 30 s. + /// + /// The timer is pre-seeded just past the threshold and the test + /// re-arms it before each `app.update()` in a small bounded loop: + /// under `MinimalPlugins` the first frame's `Time::delta_secs()` + /// can be 0.0 (or, under heavy parallel cargo-test load, large + /// enough that the pre-seeded margin is consumed by it), so a + /// single-frame check is fragile. Looping until the file appears + /// (or hitting the bound) makes the test robust against + /// first-frame Time variance without changing the underlying + /// behaviour contract. #[test] fn auto_save_writes_after_30_seconds() { use solitaire_data::load_game_state_from; @@ -1531,10 +1549,18 @@ mod tests { .0 .move_count = 1; - // Pre-seed the timer just past the threshold. The system will trigger - // on the very next update() without needing to control Time::delta_secs(). - app.insert_resource(AutoSaveTimer(AUTO_SAVE_INTERVAL_SECS + 0.1)); - app.update(); + // Re-arm the timer past the threshold every frame and pump + // updates until the save fires. Caps at 16 iterations — a + // healthy run hits it on the first or second frame; the cap + // prevents an infinite loop if a future regression skips + // the save unconditionally. + for _ in 0..16 { + app.insert_resource(AutoSaveTimer(AUTO_SAVE_INTERVAL_SECS + 1.0)); + app.update(); + if path.exists() { + break; + } + } assert!(path.exists(), "auto-save file must exist after timer crosses threshold"); let loaded = load_game_state_from(&path).expect("file must be loadable");