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 <noreply@anthropic.com>
This commit is contained in:
@@ -1264,6 +1264,14 @@ mod tests {
|
|||||||
// plugin's build path; clearing them keeps tests self-contained.
|
// plugin's build path; clearing them keeps tests self-contained.
|
||||||
app.insert_resource(GameStatePath(None));
|
app.insert_resource(GameStatePath(None));
|
||||||
app.insert_resource(ReplayPath(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.
|
// Override the system-time seed with a known value.
|
||||||
app.world_mut()
|
app.world_mut()
|
||||||
.resource_mut::<GameStateResource>()
|
.resource_mut::<GameStateResource>()
|
||||||
@@ -1516,6 +1524,16 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// auto_save_game_state writes to disk once the accumulator crosses 30 s.
|
/// 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]
|
#[test]
|
||||||
fn auto_save_writes_after_30_seconds() {
|
fn auto_save_writes_after_30_seconds() {
|
||||||
use solitaire_data::load_game_state_from;
|
use solitaire_data::load_game_state_from;
|
||||||
@@ -1531,10 +1549,18 @@ mod tests {
|
|||||||
.0
|
.0
|
||||||
.move_count = 1;
|
.move_count = 1;
|
||||||
|
|
||||||
// Pre-seed the timer just past the threshold. The system will trigger
|
// Re-arm the timer past the threshold every frame and pump
|
||||||
// on the very next update() without needing to control Time::delta_secs().
|
// updates until the save fires. Caps at 16 iterations — a
|
||||||
app.insert_resource(AutoSaveTimer(AUTO_SAVE_INTERVAL_SECS + 0.1));
|
// healthy run hits it on the first or second frame; the cap
|
||||||
app.update();
|
// 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");
|
assert!(path.exists(), "auto-save file must exist after timer crosses threshold");
|
||||||
let loaded = load_game_state_from(&path).expect("file must be loadable");
|
let loaded = load_game_state_from(&path).expect("file must be loadable");
|
||||||
|
|||||||
Reference in New Issue
Block a user