Detect snapshot names from function names (#213)

We used to detect the snapshot names form the current thread.  This has
been very unrealiable in a lot of cases.  This changes the behavior so
that the function name is used instead by using `std::any::type_name`.

This does change behavior slightly but most users should not notice this
change except if they relied on helper functions.  In that case using a
macro is a better solution most likely.

Refs #127

Fixes #105

Author: mitsuhiko

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to insta and cargo-insta are documented here.
 ## 1.12.0
 
 - Add support for sorting redactions (`sorted_redaction` and `Settings::sort_selector`). (#212)
+- Changed snapshot name detection to no longer use thread names but function names. (#213)
 
 ## 1.11.0
 

Cargo.toml

@@ -33,6 +33,9 @@ colors = ["console"]
 # This feature is now just always enabled because we use yaml internally now.
 serialization = []
 
+# This feature is no longer used as snapshot name detection was changed.
+backtrace = []
+
 [dependencies]
 csv = { version = "1.1.4", optional = true }
 serde = { version = "1.0.117", features = ["derive"] }
@@ -42,7 +45,6 @@ serde_json = "1.0.59"
 pest = { version = "2.1.3", optional = true }
 pest_derive = { version = "2.1.0", optional = true }
 ron = { version = "0.7.0", optional = true }
-backtrace = { version = "0.3.55", optional = true }
 toml = { version = "0.5.7", optional = true }
 globset = { version = "0.4.6", optional = true }
 walkdir = { version = "2.3.1", optional = true }

src/macros.rs

@@ -1,3 +1,17 @@
+/// Utility macro to return the name of the current function.
+#[doc(hidden)]
+#[macro_export]
+macro_rules! _function_name {
+    () => {{
+        fn f() {}
+        fn type_name_of_val<T>(_: T) -> &'static str {
+            std::any::type_name::<T>()
+        }
+        let name = type_name_of_val(f).strip_suffix("::f").unwrap_or("");
+        name.strip_suffix("::{{closure}}").unwrap_or(name)
+    }};
+}
+
 /// Asserts a `Serialize` snapshot in CSV format.
 ///
 /// **Feature:** `csv` (disabled by default)
@@ -384,6 +398,7 @@ macro_rules! assert_snapshot {
             $name.into(),
             &$value,
             env!("CARGO_MANIFEST_DIR"),
+            $crate::_function_name!(),
             module_path!(),
             file!(),
             line!(),

src/runtime.rs

@@ -6,7 +6,6 @@ use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::str;
 use std::sync::{Arc, Mutex};
-use std::thread;
 
 use once_cell::sync::Lazy;
 
@@ -76,59 +75,8 @@ pub enum ReferenceValue<'a> {
     Inline(&'a str),
 }
 
-#[cfg(feature = "backtrace")]
-fn test_name_from_backtrace(module_path: &str) -> Result<String, &'static str> {
-    let backtrace = backtrace::Backtrace::new();
-    let frames = backtrace.frames();
-    let mut found_run_wrapper = false;
-
-    for symbol in frames
-        .iter()
-        .rev()
-        .flat_map(|x| x.symbols())
-        .filter_map(|x| x.name())
-        .map(|x| format!("{}", x))
-    {
-        if !found_run_wrapper {
-            if symbol.starts_with("test::run_test::") {
-                found_run_wrapper = true;
-            }
-        } else if symbol.starts_with(module_path) {
-            let mut rv = &symbol[..symbol.len() - 19];
-            if rv.ends_with("::{{closure}}") {
-                rv = &rv[..rv.len() - 13];
-            }
-            return Ok(rv.to_string());
-        }
-    }
-
-    Err(
-        "Cannot determine test name from backtrace, no snapshot name \
-        can be generated. Did you forget to enable debug info?",
-    )
-}
-
-fn generate_snapshot_name_for_thread(module_path: &str) -> Result<String, &'static str> {
-    let thread = thread::current();
-    #[allow(unused_mut)]
-    let mut name = Cow::Borrowed(
-        thread
-            .name()
-            .ok_or("test thread is unnamed, no snapshot name can be generated.")?,
-    );
-    if name == "main" {
-        #[cfg(feature = "backtrace")]
-        {
-            name = Cow::Owned(test_name_from_backtrace(module_path)?);
-        }
-        #[cfg(not(feature = "backtrace"))]
-        {
-            return Err("tests run with disabled concurrency, automatic snapshot \
-                 name generation is not supported.  Consider using the \
-                 \"backtrace\" feature of insta which tries to recover test \
-                 names from the call stack.");
-        }
-    }
+fn detect_snapshot_name(function_name: &str, module_path: &str) -> Result<String, &'static str> {
+    let name = Cow::Borrowed(function_name);
 
     // clean test name first
     let mut name = name.rsplit("::").next().unwrap();
@@ -230,6 +178,7 @@ impl<'a> SnapshotAssertionContext<'a> {
     fn prepare(
         refval: ReferenceValue<'a>,
         manifest_dir: &'a str,
+        function_name: &'a str,
         module_path: &'a str,
         assertion_file: &'a str,
         assertion_line: u32,
@@ -244,7 +193,7 @@ impl<'a> SnapshotAssertionContext<'a> {
             ReferenceValue::Named(name) => {
                 let name = match name {
                     Some(name) => add_suffix_to_snapshot_name(name),
-                    None => generate_snapshot_name_for_thread(module_path)
+                    None => detect_snapshot_name(function_name, module_path)
                         .unwrap()
                         .into(),
                 };
@@ -257,7 +206,7 @@ impl<'a> SnapshotAssertionContext<'a> {
                 snapshot_file = Some(file);
             }
             ReferenceValue::Inline(contents) => {
-                snapshot_name = generate_snapshot_name_for_thread(module_path)
+                snapshot_name = detect_snapshot_name(function_name, module_path)
                     .ok()
                     .map(Cow::Owned);
                 let mut pending_file = cargo_workspace.join(assertion_file);
@@ -460,6 +409,7 @@ pub fn assert_snapshot(
     refval: ReferenceValue<'_>,
     new_snapshot_value: &str,
     manifest_dir: &str,
+    function_name: &str,
     module_path: &str,
     assertion_file: &str,
     assertion_line: u32,
@@ -468,6 +418,7 @@ pub fn assert_snapshot(
     let ctx = SnapshotAssertionContext::prepare(
         refval,
         manifest_dir,
+        function_name,
         module_path,
         assertion_file,
         assertion_line,

tests/snapshots/test_inline__something-2.snap renamed to tests/snapshots/test_inline__unnamed_thread_single_line-2.snap

@@ -1,5 +1,7 @@
 ---
 source: tests/test_inline.rs
+assertion_line: 41
 expression: "\"Testing-thread-2\""
+
 ---
 Testing-thread-2

tests/snapshots/test_inline__something.snap renamed to tests/snapshots/test_inline__unnamed_thread_single_line.snap

@@ -1,5 +1,7 @@
 ---
 source: tests/test_inline.rs
+assertion_line: 40
 expression: "\"Testing-thread\""
+
 ---
 Testing-thread

tests/test_clash_detection.rs

@@ -1,7 +1,13 @@
 use std::env;
 use std::thread;
 
-use similar_asserts::assert_eq;
+fn test_foo_always_missing() {
+    insta::assert_debug_snapshot!(42);
+}
+
+fn foo_always_missing() {
+    insta::assert_debug_snapshot!(42);
+}
 
 #[test]
 fn test_clash_detection() {
@@ -11,17 +17,15 @@ fn test_clash_detection() {
     env::set_var("INSTA_FORCE_PASS", "0");
 
     let err1 = thread::Builder::new()
-        .name("test_foo_always_missing".into())
         .spawn(|| {
-            insta::assert_debug_snapshot!(42);
+            test_foo_always_missing();
         })
         .unwrap()
         .join()
         .unwrap_err();
     let err2 = thread::Builder::new()
-        .name("foo_always_missing".into())
         .spawn(|| {
-            insta::assert_debug_snapshot!(42);
+            foo_always_missing();
         })
         .unwrap()
         .join()
@@ -44,6 +48,6 @@ fn test_clash_detection() {
     values.sort();
     assert_eq!(&values[..], &vec![
         "Insta snapshot name clash detected between \'foo_always_missing\' and \'test_foo_always_missing\' in \'test_clash_detection\'. Rename one function.",
-        "snapshot assertion for \'foo_always_missing\' failed in line 16",
+        "snapshot assertion for \'foo_always_missing\' failed in line 5",
     ][..]);
 }

tests/test_inline.rs

@@ -31,6 +31,8 @@ fn test_unnamed_single_line() {
     assert_snapshot!("Testing-2");
 }
 
+// We used to use the thread name for snapshot name detection.  This is unreliable
+// so this test now basically does exactly the same as `test_unnamed_single_line`.
 #[test]
 fn test_unnamed_thread_single_line() {
     let builder = thread::Builder::new().name("foo::lol::something".into());