From 2ae63a189aad0d3be2e6c3b93b89d3aa61dd01c2 Mon Sep 17 00:00:00 2001 From: francoisgn <32835330+francoisgn@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:53:57 +0300 Subject: [PATCH 1/2] #1290: the permission test cleans up even when it fails CodeRabbit nitpick on #2701: a failing assertion skipped giving `Locked` its permissions back, and a folder left at mode 000 can't be removed. A Drop guard restores the mode and removes the test's folder on every exit (found one left behind by an earlier failing run). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JbBu4MRiaKxEbSrMTZiLj8 --- crates/ui-egui/src/tests_launch_library.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/tests_launch_library.rs b/crates/ui-egui/src/tests_launch_library.rs index b4753a45d..54491beb6 100644 --- a/crates/ui-egui/src/tests_launch_library.rs +++ b/crates/ui-egui/src/tests_launch_library.rs @@ -874,15 +874,23 @@ fn photos_dropped_on_the_launch_list_go_into_the_library_chosen() { #[test] fn a_failed_lookup_doesnt_mark_a_recent_library_missing() { use std::os::unix::fs::PermissionsExt; + /// Gives the folder its permissions back and removes the test's folder, also when an assertion + /// fails (a folder left at mode 000 can't be removed). + struct Cleanup(std::path::PathBuf, std::path::PathBuf); + impl Drop for Cleanup { + fn drop(&mut self) { + let _ = std::fs::set_permissions(&self.1, std::fs::Permissions::from_mode(0o755)); + let _ = std::fs::remove_dir_all(&self.0); + } + } let dir = temp("lookup"); let locked = dir.join("Locked"); + let _cleanup = Cleanup(dir.clone(), locked.clone()); let inside = make_library(&locked.join("Inside")); let gone = dir.join("Gone").to_string_lossy().into_owned(); std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap(); if std::fs::read_dir(&locked).is_ok() { // running as root (some CI containers): the mode bits don't stop it, so no lookup fails - std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o755)).unwrap(); - let _ = std::fs::remove_dir_all(&dir); lightcraft_testkit::skip!(System, "a folder with no permissions can still be read (running as root)"); return; } @@ -890,7 +898,6 @@ fn a_failed_lookup_doesnt_mark_a_recent_library_missing() { app.ui.settings.recent_libraries = vec![inside.clone(), gone.clone()]; let mut h = Headless::new(app, [900.0, 600.0], 1.0); let answered = h.step_until(T, |h| h.app.recent_checks.is_known(&inside) && h.app.recent_checks.is_known(&gone)); - std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o755)).unwrap(); assert!(answered); assert_eq!(h.app.recent_checks.exists(&gone), Some(false), "a library that isn't there is missing"); assert_eq!(h.app.recent_checks.exists(&inside), None, "one that couldn't be looked at isn't"); @@ -901,7 +908,6 @@ fn a_failed_lookup_doesnt_mark_a_recent_library_missing() { assert!(!Unavailable::Unreadable("denied".into()).is_missing() && !Unavailable::LookupFailed("timed out".into()).is_missing()); assert!(Unavailable::NotFound.is_missing()); drop(h); - let _ = std::fs::remove_dir_all(&dir); } /// A library found is asked about again when the window comes back to the front, not on a timer: a From 670fb9c93086410a9a676ac8c14ace0e0b6a2359 Mon Sep 17 00:00:00 2001 From: francoisgn <32835330+francoisgn@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:53:57 +0300 Subject: [PATCH 2/2] Round 3a: two Slideshow settings tests wait for their result instead of one frame The gate on cf9334907 failed on tests_slideshow::playback_settings_drive_the_slideshow_and_bad_ones_are_refused and ::settings_shows_the_slide_layout_and_a_live_preview (#1287); they passed on the two gates before and in 15 of 15 local runs of the module with 4 threads, and nothing since touches the slideshow. Each checked its result one frame after the click / after the preview changed, which a loaded runner doesn't always give: they wait for it now (the checkbox state, the green backdrop pixel). Test code only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01JbBu4MRiaKxEbSrMTZiLj8 --- crates/ui-egui/src/tests_slideshow.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/tests_slideshow.rs b/crates/ui-egui/src/tests_slideshow.rs index 890abde7a..f4f54c3ad 100644 --- a/crates/ui-egui/src/tests_slideshow.rs +++ b/crates/ui-egui/src/tests_slideshow.rs @@ -241,8 +241,8 @@ fn playback_settings_drive_the_slideshow_and_bad_ones_are_refused() { reveal(&mut h, "check:settings.slideshowRandom"); let r = h.request("ui.clickWidget", json!({"id": "check:settings.slideshowRandom"}), T); assert_eq!(r["ok"], true, "{r}"); - h.step(); - assert!(h.app.session.slideshow.random_order, "the checkbox goes through slideshow.playback"); + // (waits for it: one frame isn't always enough on a loaded machine) + assert!(h.step_until(T, |h| h.app.session.slideshow.random_order), "the checkbox goes through slideshow.playback"); h.settle(SETTLE); } @@ -394,9 +394,17 @@ fn settings_shows_the_slide_layout_and_a_live_preview() { h.step(); // (the preview is pinned above the page: in view already) reveal(&mut h, "slideshow:preview"); - let rect = h.app.widgets.iter().find(|(id, _)| id == "slideshow:preview").map(|(_, r)| *r).unwrap(); - let img = h.paint(); - let c = painted(&img, (rect.left() + 3.0) as usize, (rect.center().y) as usize); + // (painted once the new preview is on screen: one frame isn't always enough on a loaded machine) + let t0 = std::time::Instant::now(); + let c = loop { + let rect = h.app.widgets.iter().find(|(id, _)| id == "slideshow:preview").map(|(_, r)| *r).unwrap(); + let img = h.paint(); + let c = painted(&img, (rect.left() + 3.0) as usize, (rect.center().y) as usize); + if (c[1] > 200 && c[0] < 60) || t0.elapsed() > T { + break c; + } + h.step(); + }; assert!(c[1] > 200 && c[0] < 60, "the green backdrop in the preview margin: {c:?}"); h.settle(SETTLE); }