Merge PR #3088: Issue #3065: the settings test puts the memory budget back as it found it, one budget-changing test at a time

crates/ui-egui/src/headless/tests.rs

@@ -4319,13 +4319,26 @@ fn settings_dialog_by_keyboard_and_clicks() {
     assert_eq!(h.app.session.cache_mb, 512);
     h.request("ui.clickWidget", json!({"id": "button:settingsPreview-3"}), t);
     assert_eq!(h.app.ui.settings.preview_limit, 3840);
-    h.request("ui.clickWidget", json!({"id": "button:settingsMemory-2"}), t);
-    assert_eq!(h.app.ui.settings.memory_mb, 1024);
-    h.step();
-    assert_eq!(lightcraft_engine::memory::budget(), 1024 << 20, "applied through app.memoryBudget");
-    h.request("ui.clickWidget", json!({"id": "button:settingsMemory-0"}), t);
-    h.step();
-    assert_eq!(lightcraft_engine::memory::budget(), lightcraft_engine::memory::default_budget(), "back to automatic");
+    {
+        // the memory budget is process-wide: this test's turn to change it, and it goes back
+        // to what it was (not the default) when the guard drops, also on a failed assertion
+        // (issue #3065). The change lasts these few frames.
+        use lightcraft_engine::memory::{budget, cache_share, default_budget};
+        let _budget = crate::test_budget::BudgetGuard::hold();
+        // the session's own share, set apart from the 1024 MB choice's: only the command
+        // changes it, whatever another test does to the process-wide budget
+        h.app.session.media.set_budget(cache_share(1024 << 20) + (1 << 20));
+        h.request("ui.clickWidget", json!({"id": "button:settingsMemory-2"}), t);
+        assert_eq!(h.app.ui.settings.memory_mb, 1024);
+        h.step();
+        assert_eq!(h.app.session.media.budget(), cache_share(1024 << 20), "the session's caches follow");
+        assert_eq!(budget(), 1024 << 20, "applied through app.memoryBudget");
+        h.request("ui.clickWidget", json!({"id": "button:settingsMemory-0"}), t);
+        h.step();
+        let automatic = (default_budget() >> 20) << 20;
+        assert_eq!(h.app.session.media.budget(), cache_share(automatic), "back to automatic");
+        assert_eq!(budget(), automatic, "back to automatic");
+    }
     // Import: per-camera defaults
     h.request("ui.clickWidget", json!({"id": "button:settingsTab-import"}), t);
     h.request("ui.clickWidget", json!({"id": "check:settings.perCamera"}), t);

crates/ui-egui/src/lib.rs

@@ -48,6 +48,8 @@ pub mod theme;
 pub mod widgets;
 pub mod window_place;
 
+#[cfg(test)]
+mod test_budget;
 #[cfg(test)]
 mod tests;
 #[cfg(test)]

crates/ui-egui/src/test_budget.rs

@@ -0,0 +1,93 @@
+//! For tests that change the process-wide memory budget ([`lightcraft_engine::memory::budget`]).
+//!
+//! The budget is one number for the whole test process: the decode, export and GPU gates, every
+//! new session's caches and the demo scene cache size from it, and other tests run at the same
+//! time. A test that changes it (through `app.memoryBudget` or Settings ▸ Performance) holds a
+//! [`BudgetGuard`] for as long as the change lasts. Tests that change the budget then run one at
+//! a time, so one can't change it under another's assertion. The budget goes back to exactly what
+//! it was before, also when the test fails: putting back the value found, not `default_budget()`,
+//! keeps a budget set from outside (`LIGHTCRAFT_MEMORY_MB`, a test harness) (issue #3065).
+//!
+//! Tests that only read the budget don't take the lock; keep the change short.
+//! The lock is per process, so per test binary, which is all the budget is shared by.
+
+use std::sync::{Mutex, MutexGuard, PoisonError};
+
+use lightcraft_engine::memory;
+
+/// Taken by every test that changes the process-wide budget.
+static CHANGING_BUDGET: Mutex<()> = Mutex::new(());
+
+/// Held while a test changes the process-wide memory budget: the other tests that change it wait,
+/// and the budget is put back as it was found when the guard drops.
+pub(crate) struct BudgetGuard {
+    // fields drop in order: the budget is put back before the next test may take its turn
+    _restore: Restore,
+    _turn: MutexGuard<'static, ()>,
+}
+
+impl BudgetGuard {
+    /// Wait for the other tests that change the budget, then remember the budget as it is now.
+    pub(crate) fn hold() -> BudgetGuard {
+        // poisoned: a test failed holding it, and its guard put the budget back on unwind
+        let turn = CHANGING_BUDGET.lock().unwrap_or_else(PoisonError::into_inner);
+        BudgetGuard { _restore: Restore::new(), _turn: turn }
+    }
+}
+
+/// Puts the process-wide budget back to what it was when made.
+struct Restore(usize);
+
+impl Restore {
+    fn new() -> Restore {
+        Restore(memory::budget())
+    }
+}
+
+impl Drop for Restore {
+    fn drop(&mut self) {
+        // doesn't panic; the value came from `budget()`, so it's within what `set_budget` keeps
+        memory::set_budget(self.0);
+    }
+}
+
+mod tests {
+    use super::*;
+
+    // The budget is put back as it was found, not to the default, also when the test fails
+    // (issue #3065)
+    #[test]
+    fn the_budget_is_put_back_as_it_was_found() {
+        let _turn = BudgetGuard::hold();
+        // a budget set from outside the test, unlike the default
+        let outside = if memory::budget() == 700 << 20 { 600 << 20 } else { 700 << 20 };
+        memory::set_budget(outside);
+
+        {
+            let _r = Restore::new();
+            memory::set_budget(1024 << 20);
+            assert_eq!(memory::budget(), 1024 << 20);
+        }
+        assert_eq!(memory::budget(), outside, "the budget found, not the default");
+
+        let failed = std::panic::catch_unwind(|| {
+            let _r = Restore::new();
+            memory::set_budget(96 << 20);
+            panic!("an assertion fails while the budget is changed");
+        });
+        assert!(failed.is_err());
+        assert_eq!(memory::budget(), outside, "put back on unwind too");
+    }
+
+    // A test that fails holding the lock doesn't make the next one fail
+    #[test]
+    fn a_failed_test_does_not_block_the_next() {
+        let failed = std::thread::spawn(|| {
+            let _g = BudgetGuard::hold();
+            panic!("an assertion fails while holding the turn");
+        })
+        .join();
+        assert!(failed.is_err());
+        let _g = BudgetGuard::hold();
+    }
+}