From a2e70ae221df0f52f1d0b63c3f49b2b580d99316 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Fri, 27 Sep 2024 05:24:33 +0200 Subject: [PATCH 1/2] docs: improved monospace quoting and gramar fixes (#619) --- cargo-insta/src/cli.rs | 4 ++-- cargo-insta/src/utils.rs | 2 +- insta/src/content/yaml/vendored/emitter.rs | 16 ++++++++-------- insta/src/glob.rs | 4 ++-- insta/src/lib.rs | 5 +++-- insta/src/settings.rs | 4 ++-- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index e3eafafe..ac00787a 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -83,7 +83,7 @@ enum Command { #[derive(Args, Debug, Clone)] struct TargetArgs { - /// Path to Cargo.toml + /// Path to `Cargo.toml` #[arg(long, value_name = "PATH")] manifest_path: Option, /// Explicit path to the workspace root @@ -95,7 +95,7 @@ struct TargetArgs { /// Work on all packages in the workspace #[arg(long)] workspace: bool, - /// Alias for --workspace (deprecated) + /// Alias for `--workspace` (deprecated) #[arg(long)] all: bool, /// Also walk into ignored paths. diff --git a/cargo-insta/src/utils.rs b/cargo-insta/src/utils.rs index 74930394..53d5b621 100644 --- a/cargo-insta/src/utils.rs +++ b/cargo-insta/src/utils.rs @@ -48,7 +48,7 @@ lazy_static! { pub static ref INSTA_VERSION: Version = read_insta_version().unwrap(); } -/// cargo-insta version +/// `cargo-insta` version // We could put this in a lazy_static pub(crate) fn cargo_insta_version() -> String { env!("CARGO_PKG_VERSION").to_string() diff --git a/insta/src/content/yaml/vendored/emitter.rs b/insta/src/content/yaml/vendored/emitter.rs index eddcaae4..7c8260f8 100644 --- a/insta/src/content/yaml/vendored/emitter.rs +++ b/insta/src/content/yaml/vendored/emitter.rs @@ -1,6 +1,5 @@ use crate::content::yaml::vendored::yaml::{Hash, Yaml}; -use std::convert::From; use std::error::Error; use std::fmt::{self, Display}; @@ -250,19 +249,20 @@ impl<'a> YamlEmitter<'a> { } /// Check if the string requires quoting. +/// /// Strings starting with any of the following characters must be quoted. -/// :, &, *, ?, |, -, <, >, =, !, %, @ +/// `:`, `&`, `*`, `?`, `|`, `-`, `<`, `>`, `=`, `!`, `%`, `@` /// Strings containing any of the following characters must be quoted. -/// {, }, [, ], ,, #, ` +/// `{`, `}`, `[`, `]`, `,`, `#`, `\`` /// /// If the string contains any of the following control characters, it must be escaped with double quotes: -/// \0, \x01, \x02, \x03, \x04, \x05, \x06, \a, \b, \t, \n, \v, \f, \r, \x0e, \x0f, \x10, \x11, \x12, \x13, \x14, \x15, \x16, \x17, \x18, \x19, \x1a, \e, \x1c, \x1d, \x1e, \x1f, \N, \_, \L, \P +/// `\0`, `\x01`, `\x02`, `\x03`, `\x04`, `\x05`, `\x06`, `\a`, `\b`, `\t`, `\n, `\v, `\f`, `\r`, `\x0e`, `\x0f`, `\x10`, `\x11`, `\x12`, `\x13`, `\x14`, `\x15`, `\x16`, `\x17`, `\x18`, `\x19`, `\x1a`, `\e`, `\x1c`, `\x1d`, `\x1e`, `\x1f`, `\N`, `\_`, `\L`, `\P` /// /// Finally, there are other cases when the strings must be quoted, no matter if you're using single or double quotes: -/// * When the string is true or false (otherwise, it would be treated as a boolean value); -/// * When the string is null or ~ (otherwise, it would be considered as a null value); -/// * When the string looks like a number, such as integers (e.g. 2, 14, etc.), floats (e.g. 2.6, 14.9) and exponential numbers (e.g. 12e7, etc.) (otherwise, it would be treated as a numeric value); -/// * When the string looks like a date (e.g. 2014-12-31) (otherwise it would be automatically converted into a Unix timestamp). +/// * When the string is `true` or `false` (otherwise, it would be treated as a boolean value); +/// * When the string is `null` or `~` (otherwise, it would be considered as a null value); +/// * When the string looks like a number, such as integers (e.g. `2`, `14`, etc.), floats (e.g. `2.6`, `14.9`) and exponential numbers (e.g. `12e7`, etc.) (otherwise, it would be treated as a numeric value); +/// * When the string looks like a date (e.g. `2014-12-31`) (otherwise it would be automatically converted into a Unix timestamp). fn need_quotes(string: &str) -> bool { fn need_quotes_spaces(string: &str) -> bool { string.starts_with(' ') || string.ends_with(' ') diff --git a/insta/src/glob.rs b/insta/src/glob.rs index 53579de4..e901702d 100644 --- a/insta/src/glob.rs +++ b/insta/src/glob.rs @@ -15,9 +15,9 @@ pub(crate) struct GlobCollector { pub(crate) show_insta_hint: bool, } -// the glob stack holds failure count + an indication if cargo insta review -// should be run. lazy_static::lazy_static! { + /// the glob stack holds failure count and an indication if `cargo insta review` + /// should be run. pub(crate) static ref GLOB_STACK: Mutex> = Mutex::default(); } diff --git a/insta/src/lib.rs b/insta/src/lib.rs index 574d3807..0d115ecf 100644 --- a/insta/src/lib.rs +++ b/insta/src/lib.rs @@ -46,7 +46,8 @@ //! ``` //! //! The recommended flow is to run the tests once, have them fail and check -//! if the result is okay. By default the new snapshots are stored next +//! if the result is okay. +//! By default, the new snapshots are stored next //! to the old ones with the extra `.new` extension. Once you are satisfied //! move the new files over. To simplify this workflow you can use //! `cargo insta review` (requires @@ -190,7 +191,7 @@ //! # Dependencies //! //! `insta` tries to be light in dependencies but this is tricky to accomplish -//! given what it tries to do. By default it currently depends on `serde` for +//! given what it tries to do. By default, it currently depends on `serde` for //! the [`assert_toml_snapshot!`] and [`assert_yaml_snapshot!`] macros. In the //! future this default dependencies will be removed. To already benefit from //! this optimization you can disable the default features and manually opt into diff --git a/insta/src/settings.rs b/insta/src/settings.rs index fbf5b3a8..74048818 100644 --- a/insta/src/settings.rs +++ b/insta/src/settings.rs @@ -210,7 +210,7 @@ impl Settings { /// Disables prepending of modules to the snapshot filename. /// - /// By default the filename of a snapshot is `__.snap`. + /// By default, the filename of a snapshot is `__.snap`. /// Setting this flag to `false` changes the snapshot filename to just /// `.snap`. /// @@ -226,7 +226,7 @@ impl Settings { /// Allows the [`glob!`] macro to succeed if it matches no files. /// - /// By default the glob macro will fail the test if it does not find + /// By default, the glob macro will fail the test if it does not find /// any files to prevent accidental typos. This can be disabled when /// fixtures should be conditional. /// From ed63aa9b20d7242669cb4545d138b12c81835251 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Fri, 27 Sep 2024 00:16:30 -0700 Subject: [PATCH 2/2] Move a few functions onto context object (#622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small refactor — standardizes functions operating on a snapshot context as methods --- insta/src/runtime.rs | 208 +++++++++++++++++++++---------------------- 1 file changed, 104 insertions(+), 104 deletions(-) diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 32bc4292..c2c363ed 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -42,6 +42,17 @@ macro_rules! elog { writeln!(std::io::stderr(), $($arg)*).ok(); }) } +#[cfg(feature = "glob")] +macro_rules! print_or_panic { + ($fail_fast:expr, $($tokens:tt)*) => {{ + if (!$fail_fast) { + eprintln!($($tokens)*); + eprintln!(); + } else { + panic!($($tokens)*); + } + }} +} /// Special marker to use an automatic name. /// @@ -440,127 +451,116 @@ impl<'a> SnapshotAssertionContext<'a> { Ok(snapshot_update) } -} -fn prevent_inline_duplicate(function_name: &str, assertion_file: &str, assertion_line: u32) { - let key = format!("{}|{}|{}", function_name, assertion_file, assertion_line); - let mut set = INLINE_DUPLICATES.lock().unwrap(); - if set.contains(&key) { - // drop the lock so we don't poison it - drop(set); - panic!( - "Insta does not allow inline snapshot assertions in loops. \ - Wrap your assertions in allow_duplicates! to change this." + /// This prints the information about the snapshot + fn print_snapshot_info(&self, new_snapshot: &Snapshot) { + let mut printer = SnapshotPrinter::new( + self.cargo_workspace.as_path(), + self.old_snapshot.as_ref(), + new_snapshot, ); - } - set.insert(key); -} - -/// This prints the information about the snapshot -fn print_snapshot_info(ctx: &SnapshotAssertionContext, new_snapshot: &Snapshot) { - let mut printer = SnapshotPrinter::new( - ctx.cargo_workspace.as_path(), - ctx.old_snapshot.as_ref(), - new_snapshot, - ); - printer.set_line(Some(ctx.assertion_line)); - printer.set_snapshot_file(ctx.snapshot_file.as_deref()); - printer.set_title(Some("Snapshot Summary")); - printer.set_show_info(true); - match ctx.tool_config.output_behavior() { - OutputBehavior::Summary => { - printer.print(); - } - OutputBehavior::Diff => { - printer.set_show_diff(true); - printer.print(); + printer.set_line(Some(self.assertion_line)); + printer.set_snapshot_file(self.snapshot_file.as_deref()); + printer.set_title(Some("Snapshot Summary")); + printer.set_show_info(true); + match self.tool_config.output_behavior() { + OutputBehavior::Summary => { + printer.print(); + } + OutputBehavior::Diff => { + printer.set_show_diff(true); + printer.print(); + } + _ => {} } - _ => {} } -} - -#[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: SnapshotUpdateBehavior) { - // if we are in glob mode, we want to adjust the finalization - // so that we do not show the hints immediately. - let fail_fast = { - #[cfg(feature = "glob")] - { - if let Some(top) = crate::glob::GLOB_STACK.lock().unwrap().last() { - top.fail_fast - } else { + /// Finalizes the assertion when the snapshot comparison fails, potentially + /// panicking to fail the test + fn finalize(&self, update_result: SnapshotUpdateBehavior) { + // if we are in glob mode, we want to adjust the finalization + // so that we do not show the hints immediately. + let fail_fast = { + #[cfg(feature = "glob")] + { + if let Some(top) = crate::glob::GLOB_STACK.lock().unwrap().last() { + top.fail_fast + } else { + true + } + } + #[cfg(not(feature = "glob"))] + { true } - } - #[cfg(not(feature = "glob"))] + }; + + if fail_fast + && update_result == SnapshotUpdateBehavior::NewFile + && self.tool_config.output_behavior() != OutputBehavior::Nothing + && !self.is_doctest { - true + println!( + "{hint}", + hint = style("To update snapshots run `cargo insta review`").dim(), + ); } - }; - if fail_fast - && update_result == SnapshotUpdateBehavior::NewFile - && ctx.tool_config.output_behavior() != OutputBehavior::Nothing - && !ctx.is_doctest - { - println!( - "{hint}", - hint = style("To update snapshots run `cargo insta review`").dim(), - ); - } + if update_result != SnapshotUpdateBehavior::InPlace && !self.tool_config.force_pass() { + if fail_fast && self.tool_config.output_behavior() != OutputBehavior::Nothing { + let msg = if env::var("INSTA_CARGO_INSTA") == Ok("1".to_string()) { + "Stopped on the first failure." + } else { + "Stopped on the first failure. Run `cargo insta test` to run all snapshots." + }; + println!("{hint}", hint = style(msg).dim(),); + } - if update_result != SnapshotUpdateBehavior::InPlace && !ctx.tool_config.force_pass() { - if fail_fast && ctx.tool_config.output_behavior() != OutputBehavior::Nothing { - let msg = if env::var("INSTA_CARGO_INSTA") == Ok("1".to_string()) { - "Stopped on the first failure." - } else { - "Stopped on the first failure. Run `cargo insta test` to run all snapshots." - }; - println!("{hint}", hint = style(msg).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 == SnapshotUpdateBehavior::NewFile + && self.tool_config.output_behavior() != OutputBehavior::Nothing + { + glob_collector.show_insta_hint = true; + } - // 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 == SnapshotUpdateBehavior::NewFile - && ctx.tool_config.output_behavior() != OutputBehavior::Nothing - { - glob_collector.show_insta_hint = true; + print_or_panic!( + fail_fast, + "snapshot assertion from glob for '{}' failed in line {}", + self.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), + self.assertion_line + ); + return; } - - 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 - ); - return; } + + panic!( + "snapshot assertion for '{}' failed in line {}", + self.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), + self.assertion_line + ); } + } +} +fn prevent_inline_duplicate(function_name: &str, assertion_file: &str, assertion_line: u32) { + let key = format!("{}|{}|{}", function_name, assertion_file, assertion_line); + let mut set = INLINE_DUPLICATES.lock().unwrap(); + if set.contains(&key) { + // drop the lock so we don't poison it + drop(set); panic!( - "snapshot assertion for '{}' failed in line {}", - ctx.snapshot_name.as_deref().unwrap_or("unnamed snapshot"), - ctx.assertion_line + "Insta does not allow inline snapshot assertions in loops. \ + Wrap your assertions in allow_duplicates! to change this." ); } + set.insert(key); } fn record_snapshot_duplicate( @@ -698,9 +698,9 @@ pub fn assert_snapshot( } // otherwise print information and update snapshots. } else { - print_snapshot_info(&ctx, &new_snapshot); + ctx.print_snapshot_info(&new_snapshot); let update_result = ctx.update_snapshot(new_snapshot)?; - finalize_assertion(&ctx, update_result); + ctx.finalize(update_result); } Ok(())