collective
Browse / github.com/storytold/lightcraft / patch
Issue #3065: the settings test puts the memory budget back as it found it, one budget-changing test at a time
`settings_dialog_by_keyboard_and_clicks` changes the process-wide memory budget while other tests run Part of #3057. ## Summary `settings_dialog_by_keyboard_and_clicks` (`crates/ui-egui/src/headless.rs`) changes the process-wide memory budget through `app.memoryBudget`, asserts it, then sets it back to `default_budget()`. Other ui-egui tests run in the same process at the same time, so this causes three problems: - **Other tests are affected.** Every concurrent test sees the budget change: the decode gate, the GPU pool, the export gate and the demo-scene cache all size from it. - **Its own assertion can fail** if another test changes the budget in between. - **It doesn't restore what was th
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- THEtheChad (imported from pull request #3088, open)
- Base
c9860ea7a3fa367451acd9d8cf23538c345192c6- Size
- +115 −7 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
acc9386ad5924351666da982
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/headless/tests.rs crates/ui-egui/src/lib.rs crates/ui-egui/src/test_budget.rs
@@ -4268,13 +4268,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);@@ -39,6 +39,8 @@ pub mod theme; pub mod widgets; pub mod window_place; +#[cfg(test)]+mod test_budget; #[cfg(test)] mod tests; #[cfg(test)]new file mode 100644@@ -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();+ }+}