From ffa7a23c8290713c5cff4bb050a2c70aecff6cf3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 1 Mar 2024 13:17:14 -0600 Subject: [PATCH] fix(snap): Provide more stable snapshots Before our auto-named snapshots used line numbers to make them unique. This would fail within macros. They could easily change for no good reason, requiring deleting the entire snapshot directory and restarting. Our paths were relative to the test file. I reconsidered this when doing the above because the function name made things less unique. In making them unique, I'd need to extend the path a lot (`src//snapshots/.txt`). So I moved them to the root to all be together. --- crates/snapbox/src/data/mod.rs | 27 ++++++++++++------ crates/snapbox/src/data/runtime.rs | 45 ++++++++++++++++++++++++++++++ crates/snapbox/src/macros.rs | 42 ++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/crates/snapbox/src/data/mod.rs b/crates/snapbox/src/data/mod.rs index 20e9cca3..d4c7941f 100644 --- a/crates/snapbox/src/data/mod.rs +++ b/crates/snapbox/src/data/mod.rs @@ -40,18 +40,12 @@ impl ToDebug for D { #[macro_export] macro_rules! file { [_] => {{ - let stem = ::std::path::Path::new(::std::file!()).file_stem().unwrap(); - let rel_path = ::std::format!("snapshots/{}-{}.txt", stem.to_str().unwrap(), line!()); - let mut path = $crate::current_dir!(); - path.push(rel_path); + let path = $crate::data::generate_snapshot_path($crate::fn_path!(), None); $crate::Data::read_from(&path, None) }}; [_ : $type:ident] => {{ - let stem = ::std::path::Path::new(::std::file!()).file_stem().unwrap(); - let ext = $crate::data::DataFormat:: $type.ext(); - let rel_path = ::std::format!("snapshots/{}-{}.{ext}", stem.to_str().unwrap(), line!()); - let mut path = $crate::current_dir!(); - path.push(rel_path); + let format = $crate::data::DataFormat:: $type; + let path = $crate::data::generate_snapshot_path($crate::fn_path!(), Some(format)); $crate::Data::read_from(&path, Some($crate::data::DataFormat:: $type)) }}; [$path:literal] => {{ @@ -568,3 +562,18 @@ fn is_binary(data: &[u8]) -> bool { fn is_binary(_data: &[u8]) -> bool { false } + +#[doc(hidden)] +pub fn generate_snapshot_path(fn_path: &str, format: Option) -> std::path::PathBuf { + use std::fmt::Write as _; + + let fn_path_normalized = fn_path.replace("::", "__"); + let mut path = format!("tests/snapshots/{fn_path_normalized}"); + let count = runtime::get().count(&path); + if 0 < count { + write!(&mut path, "@{count}").unwrap(); + } + path.push('.'); + path.push_str(format.unwrap_or(DataFormat::Text).ext()); + path.into() +} diff --git a/crates/snapbox/src/data/runtime.rs b/crates/snapbox/src/data/runtime.rs index 5c1ade1a..52dad661 100644 --- a/crates/snapbox/src/data/runtime.rs +++ b/crates/snapbox/src/data/runtime.rs @@ -10,12 +10,29 @@ pub(crate) fn get() -> std::sync::MutexGuard<'static, Runtime> { #[derive(Default)] pub(crate) struct Runtime { per_file: Vec, + path_count: Vec, } impl Runtime { const fn new() -> Self { Self { per_file: Vec::new(), + path_count: Vec::new(), + } + } + + pub(crate) fn count(&mut self, path_prefix: &str) -> usize { + if let Some(entry) = self + .path_count + .iter_mut() + .find(|entry| entry.is(path_prefix)) + { + entry.next() + } else { + let entry = PathRuntime::new(path_prefix); + let next = entry.count(); + self.path_count.push(entry); + next } } @@ -324,6 +341,34 @@ impl StrLitKind { } } +#[derive(Clone)] +struct PathRuntime { + path_prefix: String, + count: usize, +} + +impl PathRuntime { + fn new(path_prefix: &str) -> Self { + Self { + path_prefix: path_prefix.to_owned(), + count: 0, + } + } + + fn is(&self, path_prefix: &str) -> bool { + self.path_prefix == path_prefix + } + + fn next(&mut self) -> usize { + self.count += 1; + self.count + } + + fn count(&self) -> usize { + self.count + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/snapbox/src/macros.rs b/crates/snapbox/src/macros.rs index 3baf82ba..10a434fd 100644 --- a/crates/snapbox/src/macros.rs +++ b/crates/snapbox/src/macros.rs @@ -39,3 +39,45 @@ macro_rules! cargo_rustc_current_dir { } }}; } + +/// Path to the current function +/// +/// Closures are ignored +#[doc(hidden)] +#[macro_export] +macro_rules! fn_path { + () => {{ + fn f() {} + fn type_name_of_val(_: T) -> &'static str { + std::any::type_name::() + } + let mut name = type_name_of_val(f).strip_suffix("::f").unwrap_or(""); + while let Some(rest) = name.strip_suffix("::{{closure}}") { + name = rest; + } + name + }}; +} + +#[cfg(test)] +mod test { + #[test] + fn direct_fn_path() { + assert_eq!(fn_path!(), "snapbox::macros::test::direct_fn_path"); + } + + #[test] + fn closure_fn_path() { + (|| { + assert_eq!(fn_path!(), "snapbox::macros::test::closure_fn_path"); + })() + } + + #[test] + fn nested_fn_path() { + fn nested() { + assert_eq!(fn_path!(), "snapbox::macros::test::nested_fn_path::nested"); + } + nested() + } +}