From 3093b631137e6b1c05b4fb6a005a82a71e194d3b Mon Sep 17 00:00:00 2001 From: Panos <25664832+mechpanos@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:14:00 +0300 Subject: [PATCH] Stop the test suite reading and writing the developer's own settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new_for_test` was `Self::new()`, and `new` loads the real settings file. So every test ran against whatever the person running it had last set in the application. It surfaced as `a_selection_past_the_limit_gets_no_grips` failing on a machine where GRIPOBJLIMIT had been changed by hand: the test expected the default limit and got 32767. On a clean checkout, and in CI, it passes — which is the worst shape for this kind of bug. Tests now start from `AppConfig::default()`. That alone made things worse rather than better. The suite builds whole applications and changes preferences on them, and `AppConfig::save` writes to the real path, so a test run would have written *defaults* over the user's file where before it wrote back roughly what it had read. `save` is now inert under `cfg(test)`. Both halves matter: the first makes the suite deterministic, the second makes it harmless. This series adds around fifteen persisted settings, so every one of them was another way for a developer's own configuration to turn the suite red — or to be quietly overwritten by it. Co-Authored-By: Claude Opus 5 --- src/app/config.rs | 9 +++++++++ src/app/mod.rs | 11 ++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/app/config.rs b/src/app/config.rs index b22e28e70..6d1c8f0eb 100644 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -460,7 +460,16 @@ impl AppConfig { /// Persist the config as JSON. Best-effort; silent on unavailable or /// read-only storage. + /// + /// Does nothing under `cfg(test)`. The path is the developer's own + /// settings file, and the suite builds whole applications and changes + /// preferences on them — without this, running `cargo test` rewrites the + /// settings of whoever ran it. pub fn save(&self) { + if cfg!(test) { + return; + } + #[cfg(not(target_arch = "wasm32"))] { let Some(path) = config_path() else { return }; diff --git a/src/app/mod.rs b/src/app/mod.rs index 9269c3b3d..eef45d546 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -3856,7 +3856,16 @@ impl OpenCADStudio { #[cfg(test)] pub(crate) fn new_for_test() -> Self { - Self::new() + let mut app = Self::new(); + // `new` loads the real settings file, so without this every test runs + // against whatever the developer last set in the application — a suite + // that passes on a clean machine and fails on a used one. It surfaced + // when a persisted `GRIPOBJLIMIT` made the grip-limit test see 32767 + // where it expected the default, and the number of persisted settings + // only grows. + app.apply_config(crate::app::config::AppConfig::default()); + app.last_saved_config = Some(app.current_config()); + app } /// Install `cmd` as the active interactive command for tab `tab`.