From 2dec89bf8c35ab230b424e9e7cdf74b3398a593d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 14:24:52 -0400 Subject: [PATCH] Core tests make a throwaway blob store with blobs::scratch blobs, store and portable each built a BlobStore in a temp directory their own way: pid+tag twice, a uuid once. blobs::scratch(tag) is that, with a counter, so two tests can never share a directory even if they pick the same tag. The desktop's and ffi's temp-dir helpers stay, one per crate: sharing them would need a test-util feature on the core crate. DRY pass #2, batch 2, F9 (#5372). Co-Authored-By: Claude Opus 5.5 --- core/src/local/portable.rs | 4 ++-- core/src/local/store.rs | 12 +++--------- core/src/sync/blobs.rs | 33 +++++++++++++++++++-------------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/core/src/local/portable.rs b/core/src/local/portable.rs index 2c5a5b8..a770cbe 100644 --- a/core/src/local/portable.rs +++ b/core/src/local/portable.rs @@ -596,8 +596,8 @@ mod tests { fn store() -> (Connection, BlobStore, std::path::PathBuf) { let conn = crate::local::memory_conn().unwrap(); - let dir = std::env::temp_dir().join(format!("inkwell-portable-{}", uuid::Uuid::new_v4())); - let blobs = BlobStore::new(dir.clone()).unwrap(); + let blobs = crate::sync::blobs::scratch("portable"); + let dir = blobs.root().to_path_buf(); (conn, blobs, dir) } diff --git a/core/src/local/store.rs b/core/src/local/store.rs index b8e49af..b937387 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -1810,12 +1810,6 @@ mod tests { assert_eq!(list_labels(&conn).expect("labels")[0].count, Some(1)); } - fn blobs(tag: &str) -> crate::sync::blobs::BlobStore { - let dir = std::env::temp_dir().join(format!("ts-store-blobs-{}-{tag}", std::process::id())); - let _ = std::fs::remove_dir_all(&dir); - crate::sync::blobs::BlobStore::new(dir).expect("blobs") - } - fn tombstones(conn: &Connection) -> Vec<(String, String)> { let mut stmt = conn .prepare("SELECT entity, id FROM pending_deletes ORDER BY entity, id") @@ -1829,7 +1823,7 @@ mod tests { #[test] fn an_attached_file_is_kept_here_and_queued_to_upload() { let conn = db(); - let blobs = blobs("attach"); + let blobs = crate::sync::blobs::scratch("attach"); let n = note(&conn, "with a receipt"); conn.execute("UPDATE notes SET dirty = 0", []) .expect("clean"); @@ -1873,7 +1867,7 @@ mod tests { #[test] fn attaching_to_a_note_that_is_gone_writes_nothing() { let conn = db(); - let blobs = blobs("gone"); + let blobs = crate::sync::blobs::scratch("gone"); assert!(add_attachment(&conn, &blobs, "missing", "a.txt", "text/plain", b"hi").is_err()); let rows: i64 = conn .query_row("SELECT COUNT(*) FROM attachments", [], |r| r.get(0)) @@ -1886,7 +1880,7 @@ mod tests { #[test] fn only_a_file_the_server_holds_leaves_a_tombstone_when_removed() { let conn = db(); - let blobs = blobs("remove"); + let blobs = crate::sync::blobs::scratch("remove"); let n = note(&conn, "two files"); conn.execute( "INSERT INTO attachments (id, note_id, url) VALUES ('synced', ?1, '/x')", diff --git a/core/src/sync/blobs.rs b/core/src/sync/blobs.rs index 9a889ee..57675b7 100644 --- a/core/src/sync/blobs.rs +++ b/core/src/sync/blobs.rs @@ -215,18 +215,23 @@ pub fn serve(path: &str, query: Option<&str>) -> (u16, String, Vec) { (200, content_type_for(&claimed), bytes) } +/// A blob store in a fresh throwaway directory, for any core test that needs one. +/// No tempfile dependency for one fixture: the process id and a counter keep +/// concurrent tests and runs apart, and `tag` says whose it was. +#[cfg(test)] +pub(crate) fn scratch(tag: &str) -> BlobStore { + use std::sync::atomic::{AtomicU32, Ordering}; + static SEQ: AtomicU32 = AtomicU32::new(0); + let n = SEQ.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("ts-blobs-{}-{n}-{tag}", std::process::id())); + let _ = fs::remove_dir_all(&dir); + BlobStore::new(dir).expect("scratch blob store") +} + #[cfg(test)] mod tests { use super::*; - /// A blob store in a throwaway directory. No tempfile dependency for one test - /// fixture — the process id keeps concurrent runs apart. - fn store(tag: &str) -> BlobStore { - let dir = std::env::temp_dir().join(format!("ts-blobs-{}-{tag}", std::process::id())); - let _ = fs::remove_dir_all(&dir); - BlobStore::new(dir).expect("store") - } - /// sha256("hello") — a fixed vector, so a broken digest can't agree with itself. const HELLO: &str = "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"; @@ -237,7 +242,7 @@ mod tests { #[test] fn stores_and_reads_back() { - let store = store("roundtrip"); + let store = scratch("roundtrip"); assert!(!store.has(HELLO)); store.store(HELLO, b"hello").expect("store"); assert!(store.has(HELLO)); @@ -248,7 +253,7 @@ mod tests { fn refuses_bytes_that_dont_match_the_hash() { // A corrupted or substituted transfer must never be filed under a name that // claims it's genuine. - let store = store("mismatch"); + let store = scratch("mismatch"); let err = store.store(HELLO, b"goodbye").expect_err("must reject"); assert!(err.contains("integrity"), "got {err}"); assert!(!store.has(HELLO), "nothing should have been written"); @@ -257,7 +262,7 @@ mod tests { #[test] fn rejects_a_hash_that_could_escape_the_directory() { // The hash arrives from a server response and becomes a filename. - let store = store("traversal"); + let store = scratch("traversal"); assert!(store.path("../../etc/passwd").is_none()); assert!(store.store("../../etc/passwd", b"x").is_err()); assert!(store.path("").is_none()); @@ -267,7 +272,7 @@ mod tests { #[test] fn accepts_an_uppercase_hash() { // The wire format isn't guaranteed to be lowercase; the filename is. - let store = store("case"); + let store = scratch("case"); store .store(&HELLO.to_ascii_uppercase(), b"hello") .expect("store"); @@ -329,14 +334,14 @@ mod tests { #[test] fn put_files_bytes_under_their_own_hash() { - let store = store("put"); + let store = scratch("put"); assert_eq!(store.put(b"hello").expect("put"), HELLO); assert_eq!(store.read(HELLO).as_deref(), Some(&b"hello"[..])); } #[test] fn missing_blob_reads_as_none() { - let store = store("missing"); + let store = scratch("missing"); assert!(store.read(HELLO).is_none()); assert!(!store.has(HELLO)); }