From a6d3002ff4751e4fc9d909f34cd695c9e7618868 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 4 Sep 2022 13:44:08 +0200 Subject: [PATCH 1/4] Defer snapshot failures from glob! until the end of the block --- src/glob.rs | 30 ++++++++++++++++++++++++++++++ src/runtime.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/glob.rs b/src/glob.rs index 6c247e5e..c24e1cf4 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -1,11 +1,22 @@ use std::env; use std::path::Path; +use std::sync::Mutex; use globset::{GlobBuilder, GlobMatcher}; use once_cell::sync::Lazy; use walkdir::WalkDir; use crate::settings::Settings; +use crate::utils::style; + +pub(crate) struct GlobCollector { + pub(crate) failed: usize, + pub(crate) show_insta_hint: bool, +} + +// the glob stack holds failure count + an indication if cargo insta review +// should be run. +pub(crate) static GLOB_STACK: Lazy>> = Lazy::new(Mutex::default); static GLOB_FILTER: Lazy> = Lazy::new(|| { env::var("INSTA_GLOB_FILTER") @@ -34,6 +45,11 @@ pub fn glob_exec(base: &Path, pattern: &str, mut f: F) { let mut glob_found_matches = false; let mut settings = Settings::clone_current(); + GLOB_STACK.lock().unwrap().push(GlobCollector { + failed: 0, + show_insta_hint: false, + }); + for file in walker { let file = file.unwrap(); let path = file.path(); @@ -58,7 +74,21 @@ pub fn glob_exec(base: &Path, pattern: &str, mut f: F) { }); } + let top = GLOB_STACK.lock().unwrap().pop().unwrap(); if !glob_found_matches && !settings.allow_empty_glob() { panic!("the glob! macro did not match any files."); } + if top.failed > 0 { + if top.show_insta_hint { + println!( + "{hint}", + hint = style("To update snapshots run `cargo insta review`").dim(), + ); + } + panic!( + "glob! resulted in {} snapshot assertion failure{}s", + top.failed, + if top.failed == 1 { "" } else { "s" }, + ); + } } diff --git a/src/runtime.rs b/src/runtime.rs index 07f0d16b..c692fcd2 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -438,7 +438,21 @@ fn print_snapshot_info(ctx: &SnapshotAssertionContext, new_snapshot: &Snapshot) /// Finalizes the assertion based on the update result. fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpdate) { - if update_result == SnapshotUpdate::NewFile + // if we are in glob mode, we want to adjust the finalization + // so that we do not show the hints immediately. + let multi_assert = { + #[cfg(feature = "glob")] + { + !crate::glob::GLOB_STACK.lock().unwrap().is_empty() + } + #[cfg(not(feature = "glob"))] + { + false + } + }; + + if !multi_assert + && update_result == SnapshotUpdate::NewFile && get_output_behavior() != OutputBehavior::Nothing && !ctx.is_doctest { @@ -449,7 +463,7 @@ fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpd } if update_result != SnapshotUpdate::InPlace && !force_pass() { - if get_output_behavior() != OutputBehavior::Nothing { + if !multi_assert && get_output_behavior() != OutputBehavior::Nothing { println!( "{hint}", hint = style( @@ -458,6 +472,30 @@ fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpd .dim(), ); } + + // if we are in glob mode, count the failures and print the + // errors instead of panicking. The glob will then panic at + // the end. + #[cfg(feature = "glob")] + { + let mut stack = crate::glob::GLOB_STACK.lock().unwrap(); + if let Some(glob_collector) = stack.last_mut() { + glob_collector.failed += 1; + if update_result == SnapshotUpdate::NewFile + && get_output_behavior() != OutputBehavior::Nothing + { + glob_collector.show_insta_hint = true; + } + eprintln!( + "snapshot assertion from glob for '{}' failed in line {}", + ctx.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), + ctx.assertion_line + ); + eprintln!(); + return; + } + } + panic!( "snapshot assertion for '{}' failed in line {}", ctx.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), From 44fb4f3b96f59a40ee838b90321ec56b86558858 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 4 Sep 2022 13:45:21 +0200 Subject: [PATCH 2/4] Added note on snapshot failures in glob --- src/macros.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/macros.rs b/src/macros.rs index d420e98c..6e434835 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -488,6 +488,10 @@ macro_rules! with_settings { /// `INSTA_GLOB_FILTER` to `foo-*txt;bar-*.txt` only files starting with `foo-` or `bar-` /// end ending in `.txt` will be executed. When using `cargo-insta` the `--glob-filter` /// option can be used instead. +/// +/// Another effect of the globbing system is that snapshot failures within the glob macro +/// are deferred until the end of of it. In other words this means that each snapshot +/// assertion within the `glob!` block are reported. #[cfg(feature = "glob")] #[cfg_attr(docsrs, doc(cfg(feature = "glob")))] #[macro_export] From f5856e4276f640c839ef88a21698523dbc47fc2e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 11 Sep 2022 22:10:30 +0200 Subject: [PATCH 3/4] Couple glob behavior to --fail-fast flag --- cargo-insta/src/cli.rs | 4 +++- src/glob.rs | 3 +++ src/runtime.rs | 31 ++++++++++++++++++++++++------- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 54412945..d434bc8a 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -548,7 +548,9 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { proc.arg("--manifest-path"); proc.arg(manifest_path); } - if !cmd.fail_fast { + if cmd.fail_fast { + proc.env("INSTA_GLOB_FAIL_FAST", "1"); + } else { proc.arg("--no-fail-fast"); } if !cmd.no_force_pass { diff --git a/src/glob.rs b/src/glob.rs index c24e1cf4..1a0e9215 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -10,6 +10,7 @@ use crate::settings::Settings; use crate::utils::style; pub(crate) struct GlobCollector { + pub(crate) fail_fast: bool, pub(crate) failed: usize, pub(crate) show_insta_hint: bool, } @@ -48,6 +49,7 @@ pub fn glob_exec(base: &Path, pattern: &str, mut f: F) { GLOB_STACK.lock().unwrap().push(GlobCollector { failed: 0, show_insta_hint: false, + fail_fast: std::env::var("INSTA_GLOB_FAIL_FAST").as_deref() == Ok("1"), }); for file in walker { @@ -78,6 +80,7 @@ pub fn glob_exec(base: &Path, pattern: &str, mut f: F) { if !glob_found_matches && !settings.allow_empty_glob() { panic!("the glob! macro did not match any files."); } + if top.failed > 0 { if top.show_insta_hint { println!( diff --git a/src/runtime.rs b/src/runtime.rs index c692fcd2..c8353257 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -436,22 +436,38 @@ fn print_snapshot_info(ctx: &SnapshotAssertionContext, new_snapshot: &Snapshot) } } +#[cfg(feature = "glob")] +macro_rules! print_or_panic { + ($fail_fast:expr, $($tokens:tt)*) => {{ + if ($fail_fast) { + eprintln!($($tokens)*); + eprintln!(); + } else { + panic!($($tokens)*); + } + }} +} + /// Finalizes the assertion based on the update result. fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpdate) { // if we are in glob mode, we want to adjust the finalization // so that we do not show the hints immediately. - let multi_assert = { + let fail_fast = { #[cfg(feature = "glob")] { - !crate::glob::GLOB_STACK.lock().unwrap().is_empty() + if let Some(top) = crate::glob::GLOB_STACK.lock().unwrap().last() { + top.fail_fast + } else { + true + } } #[cfg(not(feature = "glob"))] { - false + true } }; - if !multi_assert + if fail_fast && update_result == SnapshotUpdate::NewFile && get_output_behavior() != OutputBehavior::Nothing && !ctx.is_doctest @@ -463,7 +479,7 @@ fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpd } if update_result != SnapshotUpdate::InPlace && !force_pass() { - if !multi_assert && get_output_behavior() != OutputBehavior::Nothing { + if fail_fast && get_output_behavior() != OutputBehavior::Nothing { println!( "{hint}", hint = style( @@ -486,12 +502,13 @@ fn finalize_assertion(ctx: &SnapshotAssertionContext, update_result: SnapshotUpd { glob_collector.show_insta_hint = true; } - eprintln!( + + print_or_panic!( + fail_fast, "snapshot assertion from glob for '{}' failed in line {}", ctx.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), ctx.assertion_line ); - eprintln!(); return; } } From 0b271bf0abe4505e0f61c31e96ad669c1cc2309a Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 11 Sep 2022 22:19:54 +0200 Subject: [PATCH 4/4] Update behavior of glob! once more since combing with cargo test doesn't make sense --- cargo-insta/src/cli.rs | 4 +--- src/glob.rs | 6 ++++++ src/macros.rs | 3 ++- src/runtime.rs | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index d434bc8a..54412945 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -548,9 +548,7 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { proc.arg("--manifest-path"); proc.arg(manifest_path); } - if cmd.fail_fast { - proc.env("INSTA_GLOB_FAIL_FAST", "1"); - } else { + if !cmd.fail_fast { proc.arg("--no-fail-fast"); } if !cmd.no_force_pass { diff --git a/src/glob.rs b/src/glob.rs index 1a0e9215..b30a5724 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -88,6 +88,12 @@ pub fn glob_exec(base: &Path, pattern: &str, mut f: F) { hint = style("To update snapshots run `cargo insta review`").dim(), ); } + if top.failed > 1 { + println!( + "{hint}", + hint = style("To enable fast failing for glob! export INSTA_GLOB_FAIL_FAST=1 as environment variable.").dim() + ); + } panic!( "glob! resulted in {} snapshot assertion failure{}s", top.failed, diff --git a/src/macros.rs b/src/macros.rs index 6e434835..63b9ac77 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -491,7 +491,8 @@ macro_rules! with_settings { /// /// Another effect of the globbing system is that snapshot failures within the glob macro /// are deferred until the end of of it. In other words this means that each snapshot -/// assertion within the `glob!` block are reported. +/// assertion within the `glob!` block are reported. It can be disabled by setting +/// `INSTA_GLOB_FAIL_FAST` environment variable to `1`. #[cfg(feature = "glob")] #[cfg_attr(docsrs, doc(cfg(feature = "glob")))] #[macro_export] diff --git a/src/runtime.rs b/src/runtime.rs index c8353257..e7e606bc 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -439,7 +439,7 @@ fn print_snapshot_info(ctx: &SnapshotAssertionContext, new_snapshot: &Snapshot) #[cfg(feature = "glob")] macro_rules! print_or_panic { ($fail_fast:expr, $($tokens:tt)*) => {{ - if ($fail_fast) { + if (!$fail_fast) { eprintln!($($tokens)*); eprintln!(); } else {