diff --git a/Cargo.lock b/Cargo.lock index 3061f8f6..097fba72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -66,7 +66,7 @@ dependencies = [ [[package]] name = "cargo-insta" -version = "1.40.0" +version = "1.41.0" dependencies = [ "cargo_metadata", "clap", @@ -74,7 +74,9 @@ dependencies = [ "ignore", "insta", "itertools", + "lazy_static", "proc-macro2", + "semver", "serde", "serde_json", "similar", @@ -349,7 +351,7 @@ dependencies = [ [[package]] name = "insta" -version = "1.40.0" +version = "1.41.0" dependencies = [ "clap", "console", diff --git a/Cargo.toml b/Cargo.toml index e025974d..7164d075 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,5 +16,5 @@ inherits = "release" lto = "thin" [workspace.dependencies] -# Locking because of MSRV; wait for MSRV bump or msrv-resolver +# Needs pinning in Cargo.lock because of MSRV clap = {version = "4.1", features = ["derive", "env"]} diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index 36c789f9..9e95c6fd 100644 --- a/cargo-insta/Cargo.toml +++ b/cargo-insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-insta" -version = "1.40.0" +version = "1.41.0" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A review tool for the insta snapshot testing library for Rust" @@ -11,24 +11,25 @@ keywords = ["snapshot", "testing", "jest"] categories = ["development-tools::cargo-plugins"] edition = "2021" readme = "README.md" -rust-version = "1.64.0" +rust-version = "1.65.0" [dependencies] -insta = { version = "=1.40.0", path = "../insta", features = ["json", "yaml", "redactions", "_cargo_insta_internal"] } +insta = { version = "=1.41.0", path = "../insta", features = ["json", "yaml", "redactions", "_cargo_insta_internal"] } cargo_metadata = { version = "0.18.0", default-features = false } console = "0.15.4" serde = { version = "1.0.117", features = ["derive"] } serde_json = "1.0.59" proc-macro2 = { version = "1.0.60", features = ["span-locations"] } -# Pinned because of MSRV; wait for MSRV bump or msrv-resolver +# Needs pinning in Cargo.lock because of MSRV syn = { version = "2.0.8", features = ["full", "visit", "extra-traits"] } +# Needs pinning in Cargo.lock because of MSRV ignore = "0.4.17" uuid = { version = "1.0.0", features = ["v4"] } tempfile = "3.5.0" -# Not yet supported in our MSRV of 1.60.0 -# clap = { workspace=true } -clap = {version = "4.1", features = ["derive", "env"]} - +# Needs pinning in Cargo.lock because of MSRV +semver = {version = "1.0.7", features = ["serde"]} +lazy_static = "1.4.0" +clap = { workspace=true } [dev-dependencies] walkdir = "2.3.1" diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 8f7046e9..295cdaf5 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -10,12 +10,14 @@ use insta::Snapshot; use insta::_cargo_insta_support::{ is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots, }; +use semver::Version; use serde::Serialize; use uuid::Uuid; use crate::cargo::{find_snapshot_roots, Package}; use crate::container::{Operation, SnapshotContainer}; use crate::utils::cargo_insta_version; +use crate::utils::INSTA_VERSION; use crate::utils::{err_msg, QuietExit}; use crate::walk::{find_snapshots, make_snapshot_walker, FindFlags}; @@ -592,6 +594,9 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box cmd.accept = false; } } + SnapshotUpdate::Force => { + cmd.force_update_snapshots = true; + } } // --check always implies --no-force-pass as otherwise this command does not @@ -943,16 +948,27 @@ fn prepare_test_runner<'snapshot_ref>( proc.env( "INSTA_UPDATE", - match (cmd.check, cmd.accept_unseen) { - (true, _) => "no", - (_, true) => "unseen", - (_, false) => "new", - }, + // Don't set `INSTA_UPDATE=force` for `--force-update-snapshots` on + // older versions + if *INSTA_VERSION >= Version::new(1,41,0) { + match (cmd.check, cmd.accept_unseen, cmd.force_update_snapshots) { + (true, false, false) => "no", + (false, true, false) => "unseen", + (false, false, false) => "new", + (false, _, true) => "force", + _ => return Err(err_msg(format!("invalid combination of flags: check={}, accept-unseen={}, force-update-snapshots={}", cmd.check, cmd.accept_unseen, cmd.force_update_snapshots))), + } + } else { + match (cmd.check, cmd.accept_unseen) { + (true, _) => "no", + (_, true) => "unseen", + (_, false) => "new", + } + } ); - if cmd.force_update_snapshots { - // for old versions of insta + if cmd.force_update_snapshots && *INSTA_VERSION < Version::new(1, 40, 0) { + // Currently compatible with older versions of insta. proc.env("INSTA_FORCE_UPDATE_SNAPSHOTS", "1"); - // for newer versions of insta proc.env("INSTA_FORCE_UPDATE", "1"); } if cmd.require_full_match { diff --git a/cargo-insta/src/utils.rs b/cargo-insta/src/utils.rs index b5b22315..74930394 100644 --- a/cargo-insta/src/utils.rs +++ b/cargo-insta/src/utils.rs @@ -1,6 +1,10 @@ use std::error::Error; use std::fmt; +use cargo_metadata::MetadataCommand; +use lazy_static::lazy_static; +use semver::Version; + /// Close without message but exit code. #[derive(Debug)] pub(crate) struct QuietExit(pub(crate) i32); @@ -28,6 +32,22 @@ pub(crate) fn err_msg>(s: S) -> Box { Box::new(ErrMsg(s.into())) } +/// The insta version in the current workspace (i.e. not the `cargo-insta` +/// binary that's running). +fn read_insta_version() -> Result> { + MetadataCommand::new() + .exec()? + .packages + .iter() + .find(|package| package.name == "insta") + .map(|package| package.version.clone()) + .ok_or("insta not found in cargo metadata".into()) +} + +lazy_static! { + pub static ref INSTA_VERSION: Version = read_insta_version().unwrap(); +} + /// cargo-insta version // We could put this in a lazy_static pub(crate) fn cargo_insta_version() -> String { diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index a87dafa0..57708511 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -729,12 +729,6 @@ fn test_virtual_manifest_single_crate() { #[test] fn test_force_update_snapshots() { - // We test with both 1.39 and `master`. These currently test the same code! - // But I copied the test from - // https://github.com/mitsuhiko/insta/pull/482/files where they'll test - // different code. If we don't end up merging that, we can remove one of the - // tests (but didn't think it was worthwhile to do the work to then undo it) - fn create_test_force_update_project(name: &str, insta_dependency: &str) -> TestProject { TestFiles::new() .add_file( @@ -784,7 +778,7 @@ Hello, world! let test_current_insta = create_test_force_update_project("current", "{ path = '$PROJECT_PATH' }"); - let test_insta_1_39_0 = create_test_force_update_project("1_39_0", "\"1.39.0\""); + let test_insta_1_40_0 = create_test_force_update_project("1_40_0", "\"1.40.0\""); // Test with current insta version let output_current = test_current_insta @@ -795,14 +789,14 @@ Hello, world! assert_success(&output_current); - // Test with insta 1.39.0 - let output_1_39_0 = test_insta_1_39_0 + // Test with insta 1.40.0 + let output_1_40_0 = test_insta_1_40_0 .cmd() .args(["test", "--accept", "--force-update-snapshots"]) .output() .unwrap(); - assert_success(&output_1_39_0); + assert_success(&output_1_40_0); // Check that both versions updated the snapshot correctly assert_snapshot!(test_current_insta.diff("src/snapshots/test_force_update_current__force_update.snap"), @r#" @@ -820,9 +814,9 @@ Hello, world! - "#); - assert_snapshot!(test_insta_1_39_0.diff("src/snapshots/test_force_update_1_39_0__force_update.snap"), @r#" - --- Original: src/snapshots/test_force_update_1_39_0__force_update.snap - +++ Updated: src/snapshots/test_force_update_1_39_0__force_update.snap + assert_snapshot!(test_insta_1_40_0.diff("src/snapshots/test_force_update_1_40_0__force_update.snap"), @r#" + --- Original: src/snapshots/test_force_update_1_40_0__force_update.snap + +++ Updated: src/snapshots/test_force_update_1_40_0__force_update.snap @@ -1,8 +1,5 @@ - --- diff --git a/insta/Cargo.toml b/insta/Cargo.toml index 5df915f0..6759e9a3 100644 --- a/insta/Cargo.toml +++ b/insta/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "insta" -version = "1.40.0" +version = "1.41.0" license = "Apache-2.0" authors = ["Armin Ronacher "] description = "A snapshot testing library for Rust" diff --git a/insta/src/env.rs b/insta/src/env.rs index 9082b120..807f8d36 100644 --- a/insta/src/env.rs +++ b/insta/src/env.rs @@ -4,8 +4,11 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::{env, fmt, fs}; -use crate::content::{yaml, Content}; use crate::utils::is_ci; +use crate::{ + content::{yaml, Content}, + elog, +}; lazy_static::lazy_static! { static ref WORKSPACES: Mutex>> = Mutex::new(BTreeMap::new()); @@ -64,6 +67,7 @@ pub enum SnapshotUpdate { Unseen, New, No, + Force, } #[derive(Debug)] @@ -96,7 +100,6 @@ impl std::error::Error for Error { /// Represents a tool configuration. #[derive(Debug)] pub struct ToolConfig { - force_update_snapshots: bool, force_pass: bool, require_full_match: bool, output: OutputBehavior, @@ -146,26 +149,44 @@ impl ToolConfig { } let cfg = cfg.unwrap_or_else(|| Content::Map(Default::default())); - // support for the deprecated environment variable. This is implemented in a way that - // cargo-insta can support older and newer insta versions alike. It will set both - // variables. However if only `INSTA_FORCE_UPDATE_SNAPSHOTS` is set, we will emit - // a deprecation warning. - if env::var("INSTA_FORCE_UPDATE").is_err() { - if let Ok("1") = env::var("INSTA_FORCE_UPDATE_SNAPSHOTS").as_deref() { - eprintln!("INSTA_FORCE_UPDATE_SNAPSHOTS is deprecated, use INSTA_FORCE_UPDATE"); - env::set_var("INSTA_FORCE_UPDATE", "1"); - } + // Support for the deprecated environment variables. This is + // implemented in a way that cargo-insta can support older and newer + // insta versions alike. Versions of `cargo-insta` <= 1.39 will set + // `INSTA_FORCE_UPDATE_SNAPSHOTS` & `INSTA_FORCE_UPDATE`. + // + // If `INSTA_FORCE_UPDATE_SNAPSHOTS` is the only env var present we emit + // a deprecation warning, later to be expanded to `INSTA_FORCE_UPDATE`. + // + // Another approach would be to pass the version of `cargo-insta` in a + // `INSTA_CARGO_INSTA_VERSION` env var, and then raise a warning unless + // running under cargo-insta <= 1.39. Though it would require adding a + // `semver` dependency to this crate or doing the version comparison + // ourselves (a tractable task...). + let force_update_old_env_vars = if let Ok("1") = env::var("INSTA_FORCE_UPDATE").as_deref() { + // Don't raise a warning yet, because recent versions of + // `cargo-insta` use this, so that it's compatible with older + // versions of `insta`. + // + // elog!("INSTA_FORCE_UPDATE is deprecated, use + // INSTA_UPDATE=force"); + true + } else if let Ok("1") = env::var("INSTA_FORCE_UPDATE_SNAPSHOTS").as_deref() { + // Warn on an old envvar. + // + // There's some possibility that we're running from within an fairly + // old version of `cargo-insta` (before we added an + // `INSTA_CARGO_INSTA` env var, so we can't pick that up.) So offer + // a caveat in that case. + elog!("INSTA_FORCE_UPDATE_SNAPSHOTS is deprecated, use INSTA_UPDATE=force. (If running from `cargo insta`, no action is required; upgrading `cargo-insta` will silence this warning.)"); + true + } else { + false + }; + if force_update_old_env_vars { + env::set_var("INSTA_UPDATE", "force"); } Ok(ToolConfig { - force_update_snapshots: match env::var("INSTA_FORCE_UPDATE").as_deref() { - Err(_) | Ok("") => resolve(&cfg, &["behavior", "force_update"]) - .and_then(|x| x.as_bool()) - .unwrap_or(false), - Ok("0") => false, - Ok("1") => true, - _ => return Err(Error::Env("INSTA_FORCE_UPDATE")), - }, require_full_match: match env::var("INSTA_REQUIRE_FULL_MATCH").as_deref() { Err(_) | Ok("") => resolve(&cfg, &["behavior", "require_full_match"]) .and_then(|x| x.as_bool()) @@ -203,6 +224,14 @@ impl ToolConfig { let val = match env_var.as_deref() { Err(_) | Ok("") => resolve(&cfg, &["behavior", "update"]) .and_then(|x| x.as_str()) + // Legacy support for the old force update config + .or(resolve(&cfg, &["behavior", "force_update"]).and_then(|x| { + elog!("`force_update: true` is deprecated in insta config files, use `update: force`"); + match x.as_bool() { + Some(true) => Some("force"), + _ => None, + } + })) .unwrap_or("auto"), Ok(val) => val, }; @@ -212,6 +241,7 @@ impl ToolConfig { "new" => SnapshotUpdate::New, "unseen" => SnapshotUpdate::Unseen, "no" => SnapshotUpdate::No, + "force" => SnapshotUpdate::Force, _ => return Err(Error::Env("INSTA_UPDATE")), } }, @@ -278,11 +308,6 @@ impl ToolConfig { // TODO: Do we want all these methods, vs. just allowing access to the fields? - /// Is insta told to force update snapshots? - pub fn force_update_snapshots(&self) -> bool { - self.force_update_snapshots - } - /// Should we fail if metadata doesn't match? pub fn require_full_match(&self) -> bool { self.require_full_match @@ -380,6 +405,7 @@ pub fn snapshot_update_behavior(tool_config: &ToolConfig, unseen: bool) -> Snaps } SnapshotUpdate::New => SnapshotUpdateBehavior::NewFile, SnapshotUpdate::No => SnapshotUpdateBehavior::NoUpdate, + SnapshotUpdate::Force => SnapshotUpdateBehavior::InPlace, } } diff --git a/insta/src/lib.rs b/insta/src/lib.rs index a146e705..4de7fc24 100644 --- a/insta/src/lib.rs +++ b/insta/src/lib.rs @@ -129,6 +129,7 @@ //! - `unseen`: `always` for previously unseen snapshots or `new` for existing //! snapshots //! - `no`: does not write to snapshot files at all; just runs tests +//! - `force`: forcibly updates snapshot files, even if assertions pass //! //! When `new`, `auto` or `unseen` is used, the //! [`cargo-insta`](https://crates.io/crates/cargo-insta) command can be used to @@ -207,8 +208,6 @@ //! //! ```yaml //! behavior: -//! # also set by INSTA_FORCE_UPDATE -//! force_update: true/false //! # also set by INSTA_REQUIRE_FULL_MATCH //! require_full_match: true/false //! # also set by INSTA_FORCE_PASS @@ -216,7 +215,7 @@ //! # also set by INSTA_OUTPUT //! output: "diff" | "summary" | "minimal" | "none" //! # also set by INSTA_UPDATE -//! update: "auto" | "always" | "new" | "unseen" | "no" +//! update: "auto" | "new" | "always" | "no" | "unseen" | "force" //! # also set by INSTA_GLOB_FAIL_FAST //! glob_fail_fast: true/false //! diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 90c96398..6a63d6be 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -32,6 +32,7 @@ thread_local! { // This macro is basically eprintln but without being captured and // hidden by the test runner. +#[macro_export] macro_rules! elog { () => (write!(std::io::stderr()).ok()); ($($arg:tt)*) => ({ @@ -368,6 +369,17 @@ impl<'a> SnapshotAssertionContext<'a> { let should_print = self.tool_config.output_behavior() != OutputBehavior::Nothing; let snapshot_update = snapshot_update_behavior(&self.tool_config, unseen); + // If snapshot_update is `InPlace` and we have an inline snapshot, then + // use `NewFile`, since we can't use `InPlace` for inline. `cargo-insta` + // then accepts all snapshots at the end of the test. + + let snapshot_update = + if snapshot_update == SnapshotUpdateBehavior::InPlace && self.snapshot_file.is_none() { + SnapshotUpdateBehavior::NewFile + } else { + snapshot_update + }; + match snapshot_update { SnapshotUpdateBehavior::InPlace => { if let Some(ref snapshot_file) = self.snapshot_file { @@ -383,16 +395,9 @@ impl<'a> SnapshotAssertionContext<'a> { style(snapshot_file.display()).cyan().underlined(), ); } - } else if should_print { - elog!( - "{}", - style( - "error: cannot update inline snapshots in-place \ - (https://github.com/mitsuhiko/insta/issues/272)" - ) - .red() - .bold(), - ); + } else { + // Checked self.snapshot_file.is_none() above + unreachable!() } } SnapshotUpdateBehavior::NewFile => { @@ -667,7 +672,10 @@ pub fn assert_snapshot( if pass { ctx.cleanup_passing()?; - if tool_config.force_update_snapshots() { + if matches!( + tool_config.snapshot_update(), + crate::env::SnapshotUpdate::Force + ) { ctx.update_snapshot(new_snapshot)?; } // otherwise print information and update snapshots. diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 8c8fafe4..d81c26ef 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -319,7 +319,7 @@ impl Snapshot { } } } - eprintln!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta --force-update-snapshots.\n\nSnapshot is at: {}", p.to_string_lossy()); + crate::elog!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", p.to_string_lossy()); rv }; @@ -717,7 +717,7 @@ fn get_inline_snapshot_value(frozen_value: &str) -> String { // (the only call site) if frozen_value.trim_start().starts_with('⋮') { - eprintln!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta --force-update-snapshots.\n\nValue: {}", frozen_value); + crate::elog!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", frozen_value); // legacy format - retain so old snapshots still work let mut buf = String::new();