From dd1a857d49b24724a4443e6adae7d19574b51b79 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst <92640+snofla@users.noreply.github.com> Date: Sun, 11 Oct 2026 17:26:54 +0200 Subject: [PATCH] Issue #989: the ten sidecar repair tests also pass on a case-insensitive volume (item 3, ten of eleven) The tests were written for a case-sensitive disk. rename.rs sidecars() offers both .XMP and .xmp per naming and keeps those that exist; on case-insensitive APFS a.XMP "exists" for a file stored as a.xmp, so the code renames a.XMP while the fault injectors (Faults twice, PartialRename) compared exact paths with the lowercase one: the injected failure never fired and the rename the tests expect to be refused succeeded. Fault injectors now match by same_path_or_file (equal path, or the same file by identity); two assertions that compared an exact spelling use same_path_for_volume (equal, or equal ignoring ASCII case on a case-insensitive volume), and one that required the lowercase packet to be absent is skipped there, where both spellings are one file. Helpers live in tests_xmp.rs. Production code is not touched. Run with rustc 1.99.0 on macOS: case-insensitive APFS, the ten failed before and pass now; case-sensitive APFS (a disk image), the original tests pass and so do these. All 13 sidecar::tests_repair tests pass on both. cargo fmt is clean and clippy over lightcraft-engine with --all-targets is clean. Not touched: the .xmp -> .XMP spelling change on rename, and the Lightroom folder test (its own PR). Co-Authored-By: Claude Sonnet 5.5 --- .../src/sidecar/tests_repair_companions.rs | 7 ++++--- .../src/sidecar/tests_repair_provenance.rs | 3 ++- .../engine/src/sidecar/tests_repair_saves.rs | 8 +++++--- crates/engine/src/tests_xmp.rs | 20 +++++++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/sidecar/tests_repair_companions.rs b/crates/engine/src/sidecar/tests_repair_companions.rs index cefec46e8..94c7996a7 100644 --- a/crates/engine/src/sidecar/tests_repair_companions.rs +++ b/crates/engine/src/sidecar/tests_repair_companions.rs @@ -1,6 +1,7 @@ //! Real Session partial moves, sidecar serialization/merging, AutoWrite and history. use super::*; use crate::rename::{MoveFs, RealFs}; +use crate::tests_xmp::{same_path_for_volume, same_path_or_file}; use lightcraft_catalog::{Op, Photo, Source}; use serde_json::json; @@ -16,7 +17,7 @@ impl MoveFs for Faults { RealFs.same_file(a, b) } fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> { - if self.0.iter().any(|p| p == a) { + if self.0.iter().any(|p| same_path_or_file(p, a)) { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "injected partial move")) } else { RealFs.rename_no_replace(a, b) @@ -261,8 +262,8 @@ fn queued_stem_retry_preserves_shared_packet_after_batch_rollback() { assert!(!second_packet.exists()); let pending = queues(&s); assert_eq!(pending.len(), 1); - assert_eq!(pending[0].from, shared); - assert_eq!(pending[0].to, second_packet); + assert!(same_path_for_volume(&pending[0].from, &shared), "{:?}", pending[0].from); + assert!(same_path_for_volume(&pending[0].to, &second_packet), "{:?}", pending[0].to); let catalog_before = s.catalog.to_snapshot(); let history_before = (s.undo.len(), s.redo.len(), s.commits()); diff --git a/crates/engine/src/sidecar/tests_repair_provenance.rs b/crates/engine/src/sidecar/tests_repair_provenance.rs index 63ff42002..86f7d1dd6 100644 --- a/crates/engine/src/sidecar/tests_repair_provenance.rs +++ b/crates/engine/src/sidecar/tests_repair_provenance.rs @@ -1,6 +1,7 @@ //! Native files and genuine Session entry points: packet ownership survives partial renames. use super::*; use crate::rename::{MoveFs, RealFs}; +use crate::tests_xmp::same_path_or_file; use lightcraft_catalog::{Op, Photo, Source}; use serde_json::json; @@ -16,7 +17,7 @@ impl MoveFs for Faults { RealFs.same_file(a, b) } fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> { - if self.0.iter().any(|p| p == a) { + if self.0.iter().any(|p| same_path_or_file(p, a)) { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "injected provenance partial move")) } else { RealFs.rename_no_replace(a, b) diff --git a/crates/engine/src/sidecar/tests_repair_saves.rs b/crates/engine/src/sidecar/tests_repair_saves.rs index 6f9fa4aa4..6fd3c2e9f 100644 --- a/crates/engine/src/sidecar/tests_repair_saves.rs +++ b/crates/engine/src/sidecar/tests_repair_saves.rs @@ -1,6 +1,7 @@ //! Real Session/serializer regressions for saves that overlap retained physical repairs. use super::*; use crate::rename::{MoveFs, RealFs}; +use crate::tests_xmp::{case_insensitive, same_path_or_file}; use lightcraft_catalog::{Op, Photo, Source}; use serde_json::json; @@ -19,14 +20,14 @@ impl MoveFs for PartialRename { RealFs.same_file(a, b) } fn rename_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> { - if a == self.sidecar || a == self.photo { + if same_path_or_file(a, &self.sidecar) || same_path_or_file(a, &self.photo) { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "transient rename fault")) } else { RealFs.rename_no_replace(a, b) } } fn copy_no_replace(&self, a: &Path, b: &Path) -> std::io::Result<()> { - if a == self.sidecar { + if same_path_or_file(a, &self.sidecar) { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "transient copy fault")) } else { RealFs.copy_no_replace(a, b) @@ -150,7 +151,8 @@ fn autowrite_repairs_partial_rename_before_merging_foreign_xmp_across_library_sw assert_eq!(parsed.rating, Some(4)); assert_eq!(parsed.title.as_deref(), Some("Final title")); assert!(parsed.keywords.as_ref().unwrap().contains(&"café".to_string())); - if upper { + // (on a case-insensitive volume the two spellings are one file) + if upper && !case_insensitive(&root) { assert!(!preferred.exists(), "merge uses the repaired uppercase packet"); } for (step, title) in [("edit.undo", "Fresh title"), ("edit.redo", "Final title")] { diff --git a/crates/engine/src/tests_xmp.rs b/crates/engine/src/tests_xmp.rs index c768078ce..4e7a0b326 100644 --- a/crates/engine/src/tests_xmp.rs +++ b/crates/engine/src/tests_xmp.rs @@ -7,6 +7,26 @@ use lightcraft_catalog::{ColorLabel, Flag, PhotoId}; use serde_json::json; use crate::Session; +use crate::rename::{MoveFs, RealFs}; + +/// Whether `dir` is on a case-insensitive volume (the default on macOS and Windows): its path in upper case still resolves. +pub(crate) fn case_insensitive(dir: &Path) -> bool { + Path::new(&dir.to_string_lossy().to_uppercase()).exists() +} + +/// The same path, or the same file under another spelling (`a.XMP` and `a.xmp` on a case-insensitive volume). Fault +/// injectors match this way, so an injected failure fires whichever spelling the code under test uses for a file; on a +/// case-sensitive volume only identical paths match. +pub(crate) fn same_path_or_file(a: &Path, b: &Path) -> bool { + a == b || RealFs.same_file(a, b) +} + +/// The same path, or on a case-insensitive volume a path that differs only in ASCII case: the code under test may spell +/// a sidecar `.XMP` for a file stored as `.xmp` there, while on a case-sensitive volume it offers only the spelling that +/// exists. +pub(crate) fn same_path_for_volume(a: &Path, b: &Path) -> bool { + a == b || (a.parent().is_some_and(case_insensitive) && a.to_string_lossy().eq_ignore_ascii_case(&b.to_string_lossy())) +} pub(crate) fn temp_dir(tag: &str) -> PathBuf { let d = std::env::temp_dir().join(format!("lc-xmp-{tag}-{}", std::process::id()));