From 2e0906b71a7c3165c21a5bac4976eb2e4fa0410a Mon Sep 17 00:00:00 2001 From: LochLogic <252105752+LochLogic@users.noreply.github.com> Date: Thu, 15 Oct 2026 21:11:32 -0600 Subject: [PATCH] Tests: make the Windows-only failures pass (paths with `/`, MCP sockets vs child processes) On Windows 11, `cargo test --workspace` on main fails 11 tests: - tests_export::same_folder_as_original_exports_next_to_each_photo built its expected paths with Path::join (`\`), but export joins the subfolder and file name with `/` (join_path) on every platform. Expect what export writes. - tests_folder_drop (8 tests) looked rows up as `source:libfolder:C:\...`, while the Folders section names them with `/` (`source:libfolder:C:/...`), so no row was found. Look them up the same way. - mcp backend::tests::{valid_replies_and_errors_are_preserved, malformed_replies_are_rejected_without_resending} failed only when the whole crate ran: the shutdown and one-shot-process tests start a copy of the test binary, the child can inherit a socket a fake server just accepted, and when the child exits Windows closes its copy abortively, resetting the test's connection. A test-only lock keeps socket tests and child-process tests from overlapping (passes 6 of 6 whole-crate runs, failed 4 of 4 before). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/tests_export.rs | 3 ++- crates/mcp/src/backend.rs | 4 ++++ crates/mcp/src/lib.rs | 11 +++++++++++ crates/mcp/src/progress_tests.rs | 1 + crates/mcp/src/tests.rs | 1 + crates/ui-egui/src/tests_folder_drop.rs | 15 ++++++++++----- 6 files changed, 29 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 2f1a05116..245e71300 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -991,7 +991,8 @@ fn same_folder_as_original_exports_next_to_each_photo() { let exists = |p: &str| std::path::Path::new(p).exists(); let files = crate::export::export_batch(&mut s, &ids, &o, &to, &mut crate::export::write_file, &exists).unwrap(); let written: Vec<&str> = files.iter().map(|f| f["path"].as_str().unwrap()).collect(); - let mut want = vec![a.join("Exports/IMG_1.jpg").to_string_lossy().to_string(), b.join("Exports/IMG_1.jpg").to_string_lossy().to_string()]; + // `export` joins the subfolder and name with `/` (`join_path`), on Windows too. + let mut want = vec![format!("{}/Exports/IMG_1.jpg", a.to_string_lossy()), format!("{}/Exports/IMG_1.jpg", b.to_string_lossy())]; let mut got: Vec = written.iter().map(|p| p.to_string()).collect(); want.sort(); got.sort(); diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 0c59afa6a..759f73d71 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -193,6 +193,7 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; fn with_reply(reply: &[u8], check: impl FnOnce(&mut Remote, Result)) { + let _serial = crate::serial_sockets_and_children(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap().to_string(); let reply = reply.to_vec(); @@ -307,6 +308,7 @@ mod tests { #[test] fn consecutive_replies_match_their_requests_on_one_connection() { + let _serial = crate::serial_sockets_and_children(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap().to_string(); let worker = std::thread::spawn(move || { @@ -331,6 +333,7 @@ mod tests { #[test] fn lost_reply_is_not_resent() { + let _serial = crate::serial_sockets_and_children(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap().to_string(); let seen = Arc::new(AtomicUsize::new(0)); @@ -353,6 +356,7 @@ mod tests { #[test] fn connection_closed_by_app_reconnects_before_sending() { + let _serial = crate::serial_sockets_and_children(); let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap().to_string(); let seen = Arc::new(AtomicUsize::new(0)); diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index f632f145b..4f9251bcb 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -33,3 +33,14 @@ pub const DEFAULT_ADDR: &str = "127.0.0.1:7980"; #[cfg(test)] mod tests; + +/// Held by tests that start a child process (a copy of the test binary) and by tests that run a +/// loopback socket server, so the two never overlap. On Windows the child can inherit a socket the +/// server just accepted; when the child exits, Windows closes its copy abortively and the test's +/// connection is reset ("An existing connection was forcibly closed"), which failed the +/// `backend` reply tests whenever they ran alongside a child-process test. +#[cfg(test)] +pub(crate) fn serial_sockets_and_children() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} diff --git a/crates/mcp/src/progress_tests.rs b/crates/mcp/src/progress_tests.rs index fb7859dc5..29db30a89 100644 --- a/crates/mcp/src/progress_tests.rs +++ b/crates/mcp/src/progress_tests.rs @@ -100,6 +100,7 @@ fn progress_flush_failure_does_not_wait_for_input_eof() { } fn run_shutdown_case(mode: &str) { + let _serial = crate::serial_sockets_and_children(); let mut child = Command::new(std::env::current_exe().unwrap()) .args(["--exact", "server::progress::shutdown_tests::output_failure_child", "--ignored", "--nocapture"]) .env("LIGHTCRAFT_MCP_SHUTDOWN_CASE", mode) diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 99193b480..4caf665f2 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -225,6 +225,7 @@ fn a_one_shot_process_never_ends_during_a_photos_import() { drop(h); // what lightcraft-cli does before it exits return; } + let _serial = crate::serial_sockets_and_children(); for mode in ["default", "nowait"] { let dir = std::env::temp_dir().join(format!("lc-mcp-photos-child-{mode}-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); diff --git a/crates/ui-egui/src/tests_folder_drop.rs b/crates/ui-egui/src/tests_folder_drop.rs index 9b4cb860a..1b6536242 100644 --- a/crates/ui-egui/src/tests_folder_drop.rs +++ b/crates/ui-egui/src/tests_folder_drop.rs @@ -50,8 +50,13 @@ fn widget(h: &Headless, id: &str) -> Option { h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r) } +/// How the Folders section names `dir` in widget ids: with `/` separators, on Windows too. +fn folder_key(dir: &Path) -> String { + dir.to_string_lossy().replace('\\', "/") +} + fn folder_row(h: &Headless, dir: &Path) -> Option { - widget(h, &format!("source:libfolder:{}", dir.to_string_lossy())) + widget(h, &format!("source:libfolder:{}", folder_key(dir))) } fn id_of(h: &Headless, file: &str) -> u64 { @@ -180,7 +185,7 @@ fn photos_already_in_the_folder_stay_put() { let (root, trip, home) = folders("here"); let mut h = app(&[(trip.join("beach.png"), false), (home.join("garden.png"), false)]); // open the folder holding both - let toggle = widget(&h, &format!("libraryFolderToggle:{}", root.to_string_lossy())).unwrap(); + let toggle = widget(&h, &format!("libraryFolderToggle:{}", folder_key(&root))).unwrap(); h.app.synthetic.push(egui::Event::PointerMoved(toggle.center())); h.app.synthetic.push(egui::Event::PointerButton { pos: toggle.center(), @@ -284,7 +289,7 @@ fn cancelling_the_question_moves_nothing() { let target = if folder_row(&h, &home).is_some() { target } else { - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &format!("libraryFolderToggle:{}", folder_key(&root))); folder_row(&h, &home).unwrap() }; let undo = h.app.session.undo.len(); @@ -302,7 +307,7 @@ fn dont_ask_again_moves_the_next_drop_at_once() { let (root, trip, home) = folders("dont-ask"); std::fs::copy(trip.join("beach.png"), trip.join("dunes.png")).unwrap(); let mut h = app(&[(trip.join("beach.png"), false), (trip.join("dunes.png"), false), (home.join("garden.png"), false)]); - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &format!("libraryFolderToggle:{}", folder_key(&root))); let target = folder_row(&h, &home).unwrap(); let beach = id_of(&h, "beach.png"); h.app.session.selection = lightcraft_engine::Selection::single(lightcraft_catalog::PhotoId(beach)); @@ -327,7 +332,7 @@ fn dont_ask_again_moves_the_next_drop_at_once() { fn a_question_left_open_while_another_library_opens_moves_nothing() { let (root, trip, home) = folders("other-library"); let mut h = app(&[(trip.join("beach.png"), false), (home.join("garden.png"), false)]); - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &format!("libraryFolderToggle:{}", folder_key(&root))); let beach = id_of(&h, "beach.png"); let target = folder_row(&h, &home).unwrap(); drop_beach(&mut h, beach, target);