forked from 1b-agent/opencadstudio-upstream
Stop the test suite reading and writing the developer's own settings
`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 <noreply@anthropic.com>
This commit is contained in:
1 parent
4cc8e7db62
commit
3093b63113
2 files changed
+19
-1
No files matched your search
@@ -460,7 +460,16 @@ impl AppConfig {
|
|||||||
|
|
||||||
/// Persist the config as JSON. Best-effort; silent on unavailable or
|
/// Persist the config as JSON. Best-effort; silent on unavailable or
|
||||||
/// read-only storage.
|
/// 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) {
|
pub fn save(&self) {
|
||||||
|
if cfg!(test) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(not(target_arch = "wasm32"))]
|
#[cfg(not(target_arch = "wasm32"))]
|
||||||
{
|
{
|
||||||
let Some(path) = config_path() else { return };
|
let Some(path) = config_path() else { return };
|
||||||
|
|||||||
+10
-1
@@ -3856,7 +3856,16 @@ impl OpenCADStudio {
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
pub(crate) fn new_for_test() -> Self {
|
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`.
|
/// Install `cmd` as the active interactive command for tab `tab`.
|
||||||
|
|||||||
Reference in new issue
Block a user