Merge PR #2587: test: handle concurrent fixture publication on Windows

crates/av1/tests/it/common/mod.rs

@@ -76,7 +76,7 @@ pub fn make(ff: &Path, spec: &Spec) -> PathBuf {
         args.extend(spec.enc.iter().map(String::as_str));
         args.extend(["-f", "ivf", t.as_str()]);
         run(ff, &args);
-        std::fs::rename(&tmp, &path).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &path).unwrap();
     }
     path
 }

crates/dnx/tests/it/common/mod.rs

@@ -49,7 +49,7 @@ pub fn make(ff: &Path, spec: &Spec) -> PathBuf {
         let t = tmp.to_str().unwrap().to_string();
         args.push(&t);
         run(ff, &args);
-        std::fs::rename(&tmp, &path).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &path).unwrap();
     }
     path
 }
@@ -278,7 +278,7 @@ pub fn source_frames(ff: &Path, spec: SourceFrameSpec<'_>) -> Vec<Frame> {
         let tmp = filmcraft_testkit::temp_path(&path);
         let f = frames.to_string();
         run(ff, &["-f", "lavfi", "-i", lavfi, "-frames:v", &f, "-f", "rawvideo", "-pix_fmt", pix, tmp.to_str().unwrap()]);
-        std::fs::rename(&tmp, &path).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &path).unwrap();
     }
     let bytes = std::fs::read(&path).unwrap();
     let s: Vec<u16> = if depth == 8 {

crates/h264/tests/it/common/mod.rs

@@ -364,7 +364,7 @@ pub fn ensure(f: &Fixture) -> Option<(PathBuf, PathBuf)> {
             eprintln!("SKIPPED ({}): encoder unavailable", f.name);
             return None;
         }
-        std::fs::rename(&tmp, &h264).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &h264).unwrap();
     }
     if !yuv.exists() {
         let tmp = filmcraft_testkit::temp_path(&yuv);
@@ -372,7 +372,7 @@ pub fn ensure(f: &Fixture) -> Option<(PathBuf, PathBuf)> {
         c.args(["-hide_banner", "-loglevel", "error", "-y", "-i"]).arg(&h264);
         c.args(["-fps_mode", "passthrough", "-f", "rawvideo", "-pix_fmt", "yuv420p"]).arg(&tmp);
         assert!(run(&mut c), "reference decode failed for {}", f.name);
-        std::fs::rename(&tmp, &yuv).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &yuv).unwrap();
     }
     Some((h264, yuv))
 }

crates/hevc/tests/it/common/mod.rs

@@ -142,7 +142,7 @@ pub fn ensure(f: &Fixture) -> Option<(PathBuf, PathBuf)> {
             eprintln!("SKIPPED ({}): encoder unavailable", f.name);
             return None;
         }
-        std::fs::rename(&tmp, &hevc).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &hevc).unwrap();
     }
     if !yuv.exists() {
         let tmp = filmcraft_testkit::temp_path(&yuv);
@@ -150,7 +150,7 @@ pub fn ensure(f: &Fixture) -> Option<(PathBuf, PathBuf)> {
         c.args(["-hide_banner", "-loglevel", "error", "-y", "-i"]).arg(&hevc);
         c.args(["-fps_mode", "passthrough", "-f", "rawvideo", "-pix_fmt", pix_fmt(f.bit_depth)]).arg(&tmp);
         assert!(run(&mut c), "reference decode failed for {}", f.name);
-        std::fs::rename(&tmp, &yuv).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &yuv).unwrap();
     }
     Some((hevc, yuv))
 }

crates/isobmff/tests/it/common/mod.rs

@@ -113,7 +113,7 @@ fn run_ffmpeg(ff: &Path, args: &[&str], out: &Path) -> bool {
     let tmp = filmcraft_testkit::temp_path(out);
     let st = Command::new(ff).args(["-v", "error", "-y", "-nostdin"]).args(args).arg(&tmp).output();
     match st {
-        Ok(o) if o.status.success() => std::fs::rename(&tmp, out).is_ok(),
+        Ok(o) if o.status.success() => filmcraft_testkit::fixtures::publish(&tmp, out).is_ok(),
         Ok(o) => {
             eprintln!("ffmpeg failed for {}: {}", out.display(), String::from_utf8_lossy(&o.stderr));
             let _ = std::fs::remove_file(&tmp);

crates/matroska/tests/common/mod.rs

@@ -114,7 +114,7 @@ pub fn fixture(ffmpeg: &Path, name: &str) -> Option<PathBuf> {
         eprintln!("could not generate {name} (encoder unavailable?); skipping");
         return None;
     }
-    std::fs::rename(&tmp, &out).unwrap(); // atomic: concurrent generators race harmlessly
+    filmcraft_testkit::fixtures::publish(&tmp, &out).unwrap(); // atomic: concurrent generators race harmlessly
     Some(out)
 }
 

crates/prores/tests/it/common/mod.rs

@@ -47,7 +47,7 @@ pub fn make(ff: &Path, spec: &Spec) -> PathBuf {
         let t = tmp.to_str().unwrap().to_string();
         args.push(&t);
         run(ff, &args);
-        std::fs::rename(&tmp, &path).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &path).unwrap();
     }
     path
 }
@@ -265,10 +265,10 @@ pub fn source_frames(ff: &Path, spec: SourceFrameSpec<'_>) -> Vec<Frame> {
             (ChromaFormat::Yuv444, false) => "yuv444p10le",
             (ChromaFormat::Yuv444, true) => "yuva444p10le",
         };
-        let tmp = fixture_dir().join(format!("{name}.tmp.raw"));
+        let tmp = filmcraft_testkit::temp_path(&path);
         let f = frames.to_string();
         run(ff, &["-f", "lavfi", "-i", lavfi, "-frames:v", &f, "-f", "rawvideo", "-pix_fmt", pix, tmp.to_str().unwrap()]);
-        std::fs::rename(&tmp, &path).unwrap();
+        filmcraft_testkit::fixtures::publish(&tmp, &path).unwrap();
     }
     let bytes = std::fs::read(&path).unwrap();
     let s: Vec<u16> = bytes.as_chunks::<2>().0.iter().map(|c| u16::from_le_bytes([c[0], c[1]])).collect();

crates/testkit/src/fixtures.rs

@@ -46,11 +46,34 @@ pub fn temp_path(out: &Path) -> PathBuf {
     out.with_file_name(name)
 }
 
-/// Return `out` if it already exists (non-empty); otherwise run `make(tmp)` on a [`temp_path`]
+fn complete_file(path: &Path) -> bool {
+    std::fs::metadata(path).is_ok_and(|m| m.is_file() && m.len() > 0)
+}
+
+/// Publish a complete temporary file, or reuse a complete concurrently published
+/// winner. On failure, attempt to remove only the caller's temporary file.
+/// This is for files, not directories; callers must use a unique [`temp_path`].
+pub fn publish(tmp: &Path, out: &Path) -> std::io::Result<()> {
+    if !complete_file(tmp) {
+        let winner = complete_file(out);
+        let _ = std::fs::remove_file(tmp);
+        return if winner { Ok(()) } else { Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "temporary fixture is not a non-empty file")) };
+    }
+    match std::fs::rename(tmp, out) {
+        Ok(()) => Ok(()),
+        Err(error) => {
+            let winner = complete_file(out);
+            let _ = std::fs::remove_file(tmp);
+            if winner { Ok(()) } else { Err(error) }
+        }
+    }
+}
+
+/// Return `out` if it is a non-empty file; otherwise run `make(tmp)` on a [`temp_path`]
 /// and atomically rename the result into place. Concurrent generators race harmlessly.
 /// `None` when `make` reports failure (the temporary file is removed).
 pub fn generate(out: &Path, make: impl FnOnce(&Path) -> bool) -> Option<PathBuf> {
-    if std::fs::metadata(out).is_ok_and(|m| m.len() > 0) {
+    if complete_file(out) {
         return Some(out.to_path_buf());
     }
     if let Some(dir) = out.parent() {
@@ -61,7 +84,7 @@ pub fn generate(out: &Path, make: impl FnOnce(&Path) -> bool) -> Option<PathBuf>
         let _ = std::fs::remove_file(&tmp);
         return None;
     }
-    std::fs::rename(&tmp, out).ok()?;
+    publish(&tmp, out).ok()?;
     Some(out.to_path_buf())
 }
 
@@ -103,6 +126,154 @@ pub fn generate_and_report<T>(name: &str, outputs: &[PathBuf], generate: impl Fn
 mod tests {
     use super::*;
 
+    #[cfg(windows)]
+    #[test]
+    fn direct_publication_reuses_a_locked_complete_winner() {
+        use std::os::windows::fs::OpenOptionsExt;
+        let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-direct-publish"));
+        std::fs::create_dir_all(&dir).unwrap();
+        let out = dir.join("fixture.bin");
+        let tmp = temp_path(&out);
+        std::fs::write(&out, b"winner").unwrap();
+        std::fs::write(&tmp, b"candidate").unwrap();
+        let reader = std::fs::OpenOptions::new().read(true).share_mode(1).open(&out).unwrap();
+        assert!(publish(&tmp, &out).is_ok());
+        assert_eq!(std::fs::read(&out).unwrap(), b"winner");
+        assert!(!tmp.exists());
+        drop(reader);
+        std::fs::remove_dir_all(dir).unwrap();
+    }
+
+    #[test]
+    fn direct_publication_rejects_invalid_temporary_without_output() {
+        for kind in ["empty", "missing", "directory"] {
+            let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-invalid-publish"));
+            std::fs::create_dir_all(&dir).unwrap();
+            let out = dir.join("fixture.bin");
+            let tmp = temp_path(&out);
+            match kind {
+                "empty" => std::fs::write(&tmp, b"").unwrap(),
+                "directory" => {
+                    std::fs::create_dir(&tmp).unwrap();
+                    std::fs::write(tmp.join("child"), b"caller owned").unwrap();
+                }
+                _ => {}
+            }
+            assert!(publish(&tmp, &out).is_err(), "accepted {kind} temporary");
+            assert!(!out.exists(), "published {kind} temporary");
+            if kind == "directory" {
+                assert_eq!(std::fs::read(tmp.join("child")).unwrap(), b"caller owned");
+                std::fs::remove_file(tmp.join("child")).unwrap();
+                std::fs::remove_dir(&tmp).unwrap();
+            } else {
+                assert!(!tmp.exists(), "left {kind} temporary file");
+            }
+            std::fs::remove_dir(&dir).unwrap();
+        }
+    }
+
+    #[test]
+    fn direct_publication_rejects_invalid_temporary_with_empty_output() {
+        for kind in ["empty", "missing", "directory"] {
+            let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-invalid-empty-out"));
+            std::fs::create_dir_all(&dir).unwrap();
+            let out = dir.join("fixture.bin");
+            let tmp = temp_path(&out);
+            std::fs::write(&out, b"").unwrap();
+            match kind {
+                "empty" => std::fs::write(&tmp, b"").unwrap(),
+                "directory" => {
+                    std::fs::create_dir(&tmp).unwrap();
+                    std::fs::write(tmp.join("child"), b"caller owned").unwrap();
+                }
+                _ => {}
+            }
+            assert!(publish(&tmp, &out).is_err(), "accepted {kind} temporary");
+            assert!(out.is_file());
+            assert_eq!(std::fs::read(&out).unwrap(), b"");
+            if kind == "directory" {
+                assert_eq!(std::fs::read(tmp.join("child")).unwrap(), b"caller owned");
+                std::fs::remove_file(tmp.join("child")).unwrap();
+                std::fs::remove_dir(&tmp).unwrap();
+            } else {
+                assert!(!tmp.exists(), "left {kind} temporary file");
+            }
+            std::fs::remove_file(&out).unwrap();
+            std::fs::remove_dir(&dir).unwrap();
+        }
+    }
+
+    #[test]
+    fn direct_publication_preserves_complete_winner_with_invalid_temporary() {
+        for kind in ["empty", "missing", "directory"] {
+            let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-invalid-winner"));
+            std::fs::create_dir_all(&dir).unwrap();
+            let out = dir.join("fixture.bin");
+            let tmp = temp_path(&out);
+            std::fs::write(&out, b"complete winner").unwrap();
+            match kind {
+                "empty" => std::fs::write(&tmp, b"").unwrap(),
+                "directory" => {
+                    std::fs::create_dir(&tmp).unwrap();
+                    std::fs::write(tmp.join("child"), b"caller owned").unwrap();
+                }
+                _ => {}
+            }
+            assert!(publish(&tmp, &out).is_ok(), "did not reuse winner for {kind} temporary");
+            assert_eq!(std::fs::read(&out).unwrap(), b"complete winner", "changed winner for {kind} temporary");
+            if kind == "directory" {
+                assert_eq!(std::fs::read(tmp.join("child")).unwrap(), b"caller owned");
+                std::fs::remove_file(tmp.join("child")).unwrap();
+                std::fs::remove_dir(&tmp).unwrap();
+            } else {
+                assert!(!tmp.exists(), "left {kind} temporary file");
+            }
+            std::fs::remove_file(&out).unwrap();
+            std::fs::remove_dir(&dir).unwrap();
+        }
+    }
+
+    // These callbacks expose an inherited generate validation gap: success alone
+    // does not establish that the generated path is a complete fixture file.
+    #[test]
+    fn generate_rejects_successful_callback_producing_empty_file() {
+        let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-generate-empty"));
+        std::fs::create_dir_all(&dir).unwrap();
+        let out = dir.join("fixture.bin");
+        let mut temporary = None;
+        let result = generate(&out, |tmp| {
+            temporary = Some(tmp.to_path_buf());
+            std::fs::write(tmp, b"").unwrap();
+            true
+        });
+        assert!(result.is_none());
+        assert!(!out.exists());
+        assert!(!temporary.unwrap().exists());
+        std::fs::remove_dir(&dir).unwrap();
+    }
+
+    #[test]
+    fn generate_rejects_successful_callback_producing_directory() {
+        let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-generate-directory"));
+        std::fs::create_dir_all(&dir).unwrap();
+        let out = dir.join("fixture.bin");
+        let mut temporary = None;
+        let result = generate(&out, |tmp| {
+            temporary = Some(tmp.to_path_buf());
+            std::fs::create_dir(tmp).unwrap();
+            std::fs::write(tmp.join("child"), b"caller owned").unwrap();
+            true
+        });
+        assert!(result.is_none());
+        assert!(!out.exists());
+        let tmp = temporary.unwrap();
+        assert!(tmp.is_dir());
+        assert_eq!(std::fs::read(tmp.join("child")).unwrap(), b"caller owned");
+        std::fs::remove_file(tmp.join("child")).unwrap();
+        std::fs::remove_dir(&tmp).unwrap();
+        std::fs::remove_dir(&dir).unwrap();
+    }
+
     #[test]
     fn root_is_the_workspace() {
         assert!(workspace_root().join("Cargo.toml").exists());
@@ -133,4 +304,54 @@ mod tests {
         let p2 = generate(&out, |_| panic!("regenerated")).unwrap();
         assert_eq!(p, p2);
     }
+
+    #[cfg(windows)]
+    #[test]
+    fn concurrent_complete_fixture_survives_blocked_publication() {
+        use std::os::windows::fs::OpenOptionsExt;
+
+        let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-publish"));
+        std::fs::create_dir_all(&dir).unwrap();
+        let out = dir.join("fixture.bin");
+        let mut reader = None;
+        let mut temporary = None;
+        let result = generate(&out, |tmp| {
+            temporary = Some(tmp.to_path_buf());
+            std::fs::write(&out, b"complete winner").unwrap();
+            // Simulate another test reading the winner without delete sharing.
+            reader = Some(std::fs::OpenOptions::new().read(true).share_mode(1).open(&out).unwrap());
+            std::fs::write(tmp, b"complete candidate").is_ok()
+        });
+        assert_eq!(result.as_deref(), Some(out.as_path()));
+        assert_eq!(std::fs::read(&out).unwrap(), b"complete winner");
+        assert!(!temporary.unwrap().exists());
+        drop(reader);
+        std::fs::remove_dir_all(&dir).unwrap();
+    }
+
+    #[test]
+    fn failed_publication_does_not_accept_a_directory_or_leave_temporary() {
+        let dir = temp_path(&std::env::temp_dir().join("filmcraft-testkit-publish-directory"));
+        std::fs::create_dir_all(&dir).unwrap();
+        let out = dir.join("fixture.bin");
+        let mut temporary = None;
+        let result = generate(&out, |tmp| {
+            temporary = Some(tmp.to_path_buf());
+            std::fs::create_dir_all(&out).unwrap();
+            std::fs::write(out.join("child"), b"not a fixture").unwrap();
+            std::fs::write(tmp, b"candidate").is_ok()
+        });
+        assert!(result.is_none());
+        assert!(!temporary.unwrap().exists());
+        let mut called = false;
+        assert!(
+            generate(&out, |_| {
+                called = true;
+                false
+            })
+            .is_none()
+        );
+        assert!(called, "a directory must not be reused as a cached file");
+        std::fs::remove_dir_all(&dir).unwrap();
+    }
 }