diff --git a/crates/ui-egui/src/headless/tests.rs b/crates/ui-egui/src/headless/tests.rs index 13bd7a2ae..6cf5cb069 100644 --- a/crates/ui-egui/src/headless/tests.rs +++ b/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); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8bf57b341..c74912fd6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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)] diff --git a/crates/ui-egui/src/test_budget.rs b/crates/ui-egui/src/test_budget.rs new file mode 100644 index 000000000..1442cd1a0 --- /dev/null +++ b/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(); + } +}