From 2e66a00d554508341a83f0942b9224f2161f2ba8 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 06:39:08 +0100 Subject: [PATCH] Tests: two catalog flakes; a readback waits as long as a render (#3777) a_failed_write_is_held_reported_and_retried raced the log thread: it could fail the first append before the second arrived, which is then refused by design. The test now hands both over in one append. a_file_path_rule_allocates_nothing_per_photo counted heap blocks on every thread, so the test harness's own thread could fail it (four blocks, on PRs that don't touch this code). An allocation per photo is thousands, so the bound stays far below that. read_texture waited 10 s where the render path waits 60 s, so a view could be dropped from a headless screenshot while earlier work ran. The GPU guide now says what the code does about the window's card. Co-authored-by: Claude Opus 5.5 --- crates/catalog/src/tests_log_thread.rs | 4 ++-- crates/catalog/tests/path_fold_heap.rs | 5 ++++- crates/gpu/src/ctx.rs | 3 ++- crates/gpu/src/lib.rs | 2 +- docs/gpu-pipeline.md | 8 ++++---- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/catalog/src/tests_log_thread.rs b/crates/catalog/src/tests_log_thread.rs index b7da3f7090..9c78d61f69 100644 --- a/crates/catalog/src/tests_log_thread.rs +++ b/crates/catalog/src/tests_log_thread.rs @@ -106,8 +106,8 @@ fn a_failed_write_is_held_reported_and_retried() { let fail = Arc::new(AtomicBool::new(true)); let f = fail.clone(); fault::install(&dir, Arc::new(move |_| if f.load(Ordering::SeqCst) { Err(std::io::Error::other("disk full (test)")) } else { Ok(()) })); - store.append(LOG, b"1\n").unwrap(); - store.append(LOG, b"2\n").unwrap(); + // (one append: the log thread can fail the first before a second arrives, and then refuses it) + store.append(LOG, b"1\n2\n").unwrap(); wait_until("the failure is published", || store.unwritten().is_some()); let (held, why) = store.unwritten().unwrap(); assert!(why.contains("disk full"), "{why}"); diff --git a/crates/catalog/tests/path_fold_heap.rs b/crates/catalog/tests/path_fold_heap.rs index 4fc1c8ef14..9c41f3fcf2 100644 --- a/crates/catalog/tests/path_fold_heap.rs +++ b/crates/catalog/tests/path_fold_heap.rs @@ -55,5 +55,8 @@ fn a_file_path_rule_allocates_nothing_per_photo() { } let blocks = dhat::HeapStats::get().total_blocks - before; assert!(hits > 0, "the rules matched nothing: the test measures nothing"); - assert_eq!(blocks, 0, "{blocks} heap blocks for {} rule tests", 100 * rules.len() * photos.len()); + // dhat counts every thread, and the test harness's own thread can allocate a few blocks while this + // runs. An allocation per photo would be thousands, two per rule test. + let tests = 100 * rules.len() * photos.len(); + assert!(blocks < 64, "{blocks} heap blocks for {tests} rule tests"); } diff --git a/crates/gpu/src/ctx.rs b/crates/gpu/src/ctx.rs index 640ef28b77..2735116e39 100644 --- a/crates/gpu/src/ctx.rs +++ b/crates/gpu/src/ctx.rs @@ -895,7 +895,8 @@ pub(crate) fn read_texture( slice.map_async(wgpu::MapMode::Read, move |r| { let _ = tx.send(r); }); - let wait = std::time::Duration::from_secs(10); + // (the render path's wait: queued work ahead of the copy can take that long too) + let wait = READBACK_TIMEOUT; poll_device(device, wgpu::PollType::Wait { submission_index: None, timeout: Some(wait) })?; match rx.recv_timeout(wait) { Ok(Ok(())) => {} diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index c187a764ed..8863b45a04 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -606,7 +606,7 @@ pub fn adopt_window_device(device: wgpu::Device, queue: wgpu::Queue, info: wgpu: /// The pixels of a texture on `device` (a [`DisplayFrame`]'s, or any `Rgba8Unorm` texture with /// `COPY_SRC`): for a headless screenshot of a view drawn straight from the GPU, and for tests. The -/// app never needs them to draw. Waits for the GPU (at most a few seconds); `Err` when the copy or +/// app never needs them to draw. Waits for the GPU (at most a minute); `Err` when the copy or /// the wait fails. #[cfg(not(target_arch = "wasm32"))] pub fn read_texture( diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index 42b3578860..6e063e6f90 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -64,10 +64,10 @@ Overrides (read once at launch): (`gpu: LIGHTCRAFT_GPU_ADAPTER=… names none of the graphics cards (…); using the default choice`) and the default high-performance card is used. Each line is logged once, though the render device, AI Denoise and the face models each ask. Each of those makes its own wgpu instance and device, so - devices on different cards share no buffer, pipeline or queue. The window stays on the card that drives the screen, - and rendering runs on a device of its own on the chosen card (the photo is copied to the screen's - card); a build that lets rendering share the window's device does so only when the variable names - the window's card (`backend::may_adopt_window_device`). The log's `gpu: LIGHTCRAFT_GPU_ADAPTER=…: using …` line names the card + devices on different cards share no buffer, pipeline or queue. The window keeps the card wgpu picks for it by power preference, and + rendering runs on a device of its own on the chosen card (the photo is copied to the window's card). + Rendering shares the window's device when the variable is unset or names the window's card + (`backend::may_adopt_window_device`). The log's `gpu: LIGHTCRAFT_GPU_ADAPTER=…: using …` line names the card used. Handy to keep test runs or a long export off the card the screens hang on. - `LIGHTCRAFT_GPU_ALLOW_SOFTWARE=1`: accept a software adapter (llvmpipe / lavapipe, WARP), which is otherwise refused because the CPU pipeline is faster. For running the GPU tests on a machine