From d8a255869c890b9993ab497b7d2f0c2da5d8c88a Mon Sep 17 00:00:00 2001 From: funman300 Date: Mon, 6 Jul 2026 20:22:27 -0700 Subject: [PATCH] test(engine): ratchet on Bevy system-order ambiguities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First measurement of schedule hygiene (issue #143): the headless gameplay cluster (Game/Table/Card/Hud/AutoComplete/UiModal/UiFocus/ Settings) carries 302 system pairs with conflicting data access and no ordering edge. Too many to triage in one pass and most are likely benign event/resource writers — but unproven, and nothing stopped the count from growing. New schedule_checks test builds the cluster with ambiguity detection promoted to error, parses the reported pair count, and asserts it never exceeds AMBIGUITY_BASELINE (302). New ambiguous pairs now fail CI at the PR; the legacy backlog can be burned down incrementally by adding .before/.after or .ambiguous_with and lowering the baseline. Refs #143 Co-Authored-By: Claude Fable 5 --- solitaire_engine/src/lib.rs | 1 + solitaire_engine/src/schedule_checks.rs | 105 ++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 solitaire_engine/src/schedule_checks.rs diff --git a/solitaire_engine/src/lib.rs b/solitaire_engine/src/lib.rs index 753cce5..2ab2b93 100644 --- a/solitaire_engine/src/lib.rs +++ b/solitaire_engine/src/lib.rs @@ -43,6 +43,7 @@ pub mod replay_overlay; pub mod replay_playback; pub mod resources; pub mod safe_area; +mod schedule_checks; pub mod selection_plugin; pub mod settings_plugin; pub mod splash_plugin; diff --git a/solitaire_engine/src/schedule_checks.rs b/solitaire_engine/src/schedule_checks.rs new file mode 100644 index 0000000..2a783f8 --- /dev/null +++ b/solitaire_engine/src/schedule_checks.rs @@ -0,0 +1,105 @@ +//! Schedule hygiene checks (issue #143). +//! +//! Bevy runs systems with conflicting data access in nondeterministic order +//! unless an ordering edge exists. Nothing enforced this historically, and +//! the headless gameplay cluster carries a large legacy backlog of such +//! pairs — most benign (event/resource writers that never observably race), +//! but unproven. Rather than a permanently red hard-error test or no test +//! at all, this is a RATCHET: the count may only go down. New conflicting +//! pairs fail CI immediately; the backlog can be triaged incrementally +//! (add `.before`/`.after` where order matters, `.ambiguous_with` where it +//! provably doesn't, then lower `AMBIGUITY_BASELINE`). +//! +//! To see the offending pair names, add the `bevy_debug` / debug feature to +//! the bevy dev-dependency (system names are stripped otherwise) and set +//! `ambiguity_detection: LogLevel::Error` manually. +//! +//! Scope note: `CoreGamePlugin` (the full composition) performs real +//! storage/platform I/O in `build` and cannot run in a unit test; the +//! cluster below is the same one the engine's headless behaviour tests use. + +#[cfg(test)] +mod tests { + use bevy::ecs::schedule::{LogLevel, ScheduleBuildSettings}; + use bevy::prelude::*; + + use crate::auto_complete_plugin::AutoCompletePlugin; + use crate::card_plugin::CardPlugin; + use crate::game_plugin::GamePlugin; + use crate::hud_plugin::HudPlugin; + use crate::settings_plugin::SettingsPlugin; + use crate::table_plugin::TablePlugin; + use crate::ui_focus::UiFocusPlugin; + use crate::ui_modal::UiModalPlugin; + + /// Legacy ambiguity backlog measured 2026-07-06 (issue #143). This + /// number may only decrease. If your change trips this assertion you + /// have added a pair of systems with conflicting data access and no + /// ordering edge — add `.before`/`.after` (order matters) or + /// `.ambiguous_with` (provably order-independent) at the registration + /// site. When triage lowers the real count, lower this constant too. + const AMBIGUITY_BASELINE: usize = 302; + + fn cluster_app() -> App { + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_plugins(GamePlugin) + .add_plugins(TablePlugin) + .add_plugins(CardPlugin) + .add_plugins(HudPlugin) + .add_plugins(AutoCompletePlugin) + .add_plugins(UiModalPlugin) + .add_plugins(UiFocusPlugin) + .add_plugins(SettingsPlugin::headless()); + app.init_resource::>(); + app + } + + #[test] + fn update_schedule_ambiguities_do_not_grow() { + let mut app = cluster_app(); + app.edit_schedule(Update, |s| { + s.set_build_settings(ScheduleBuildSettings { + ambiguity_detection: LogLevel::Error, + ..Default::default() + }); + }); + + // With LogLevel::Error the first schedule build panics when any + // ambiguity exists, and the panic message begins with the pair + // count. Catch it and ratchet on that count. (Parsing a panic + // message is brittle by design — if a Bevy upgrade rewords it, + // this test fails loudly and the parse below needs one-line + // maintenance, which is preferable to losing the ratchet.) + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + app.update(); + })); + + let count = match result { + Ok(()) => 0, + Err(payload) => { + let msg = payload + .downcast_ref::() + .cloned() + .or_else(|| payload.downcast_ref::<&str>().map(|s| (*s).to_string())) + .unwrap_or_default(); + let parsed = msg + .split(" pairs of systems") + .next() + .and_then(|prefix| prefix.split_whitespace().last()) + .and_then(|n| n.parse::().ok()); + parsed.unwrap_or_else(|| { + panic!( + "ambiguity panic message no longer parseable (Bevy upgrade?): {msg}" + ) + }) + } + }; + + assert!( + count <= AMBIGUITY_BASELINE, + "system-order ambiguities grew: {count} > baseline {AMBIGUITY_BASELINE}. \ + Add .before/.after or .ambiguous_with at the new registration site.", + ); + } +} -- 2.47.3