From c1947bc98684f6b50323577c18e275ff7ae3f9fb Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 3 Jan 2023 19:20:12 +0100 Subject: [PATCH] Add new --unreferenced option to control unreferenced snapshot behavior (#328) --- CHANGELOG.md | 4 + cargo-insta/src/cli.rs | 183 ++++++++++++++++++++++++++++------------- src/env.rs | 49 ++++++++++- src/lib.rs | 8 +- 4 files changed, 180 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f390ba7..f7bce53c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ All notable changes to insta and cargo-insta are documented here. - Renamed `--no-ignore` to `--include-ignored`. - Added `--include-hidden` to instruct insta to also walk into hidden paths. +- Added new `--unreferenced` option to `cargo-insta test` which allows + fine tuning of what should happen with unreferenced files. It's now + possible to ignore (default), warn, reject or delete unreferenced + snapshots. (#328) ## 1.23.0 diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index cb40c413..537496a1 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -10,7 +10,8 @@ use console::{set_colors_enabled, style, Key, Term}; use ignore::{Walk, WalkBuilder}; use insta::Snapshot; use insta::_cargo_insta_support::{ - print_snapshot, print_snapshot_diff, SnapshotUpdate, TestRunner, ToolConfig, + is_ci, print_snapshot, print_snapshot_diff, SnapshotUpdate, TestRunner, ToolConfig, + UnreferencedSnapshots, }; use serde::Serialize; use structopt::clap::AppSettings; @@ -172,8 +173,11 @@ pub struct TestCommand { /// Update all snapshots even if they are still matching. #[structopt(long)] pub force_update_snapshots: bool, - /// Delete unreferenced snapshots after the test run. + /// Controls what happens with unreferenced snapshots. #[structopt(long)] + pub unreferenced: Option, + /// Delete unreferenced snapshots after the test run. + #[structopt(long, hidden = true)] pub delete_unreferenced_snapshots: bool, /// Filters to apply to the insta glob feature. #[structopt(long)] @@ -402,7 +406,7 @@ fn load_snapshot_containers<'a>( fn process_snapshots( quiet: bool, snapshot_filter: Option<&[String]>, - loc: LocationInfo<'_>, + loc: &LocationInfo<'_>, op: Option, ) -> Result<(), Box> { let term = Term::stdout(); @@ -590,6 +594,11 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { cmd.review = true; } + // Legacy command + if cmd.delete_unreferenced_snapshots { + cmd.unreferenced = Some("delete".into()); + } + let test_runner = match cmd.test_runner { Some(ref test_runner) => test_runner .parse() @@ -597,10 +606,18 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { None => loc.tool_config.test_runner(), }; - let (mut proc, snapshot_ref_file) = prepare_test_runner(test_runner, &cmd, color, &[], None)?; + let unreferenced = match cmd.unreferenced { + Some(ref value) => value + .parse() + .map_err(|_| err_msg("invalid value for --unreferenced"))?, + None => loc.tool_config.test_unreferenced(), + }; + + let (mut proc, snapshot_ref_file) = + prepare_test_runner(test_runner, unreferenced, &cmd, color, &[], None)?; if !cmd.keep_pending { - process_snapshots(true, None, loc, Some(Operation::Reject))?; + process_snapshots(true, None, &loc, Some(Operation::Reject))?; } let status = proc.status()?; @@ -610,6 +627,7 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { if matches!(test_runner, TestRunner::Nextest) { let (mut proc, _) = prepare_test_runner( TestRunner::CargoTest, + unreferenced, &cmd, color, &["--doc"], @@ -633,65 +651,16 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { return Err(QuietExit(1).into()); } - // delete unreferenced snapshots if we were instructed to do so + // handle unreferenced snapshots if we were instructed to do so if let Some(ref path) = snapshot_ref_file { - let mut files = HashSet::new(); - match fs::read_to_string(path) { - Ok(s) => { - for line in s.lines() { - if let Ok(path) = fs::canonicalize(line) { - files.insert(path); - } - } - } - Err(err) => { - // if the file was not created, no test referenced - // snapshots. - if err.kind() != io::ErrorKind::NotFound { - return Err(err.into()); - } - } - } - - if let Ok(loc) = handle_target_args(&cmd.target_args) { - let mut deleted_any = false; - for entry in make_deletion_walker(&loc, cmd.package.as_deref()) { - let rel_path = match entry { - Ok(ref entry) => entry.path(), - _ => continue, - }; - if !rel_path.is_file() - || !rel_path - .file_name() - .map_or(false, |x| x.to_str().unwrap_or("").ends_with(".snap")) - { - continue; - } - - if let Ok(path) = fs::canonicalize(rel_path) { - if !files.contains(&path) { - if !deleted_any { - eprintln!("{}: deleted unreferenced snapshots:", style("info").bold()); - deleted_any = true; - } - eprintln!(" {}", rel_path.display()); - fs::remove_file(path).ok(); - } - } - } - if !deleted_any { - eprintln!("{}: no unreferenced snapshots found", style("info").bold()); - } - } - - fs::remove_file(&path).ok(); + handle_unreferenced_snapshots(path, &loc, unreferenced, cmd.package.as_deref())?; } if cmd.review || cmd.accept { process_snapshots( false, None, - handle_target_args(&cmd.target_args)?, + &handle_target_args(&cmd.target_args)?, if cmd.accept { Some(Operation::Accept) } else { @@ -720,8 +689,103 @@ fn test_run(mut cmd: TestCommand, color: &str) -> Result<(), Box> { Ok(()) } +fn handle_unreferenced_snapshots( + path: &Cow, + loc: &LocationInfo<'_>, + unreferenced: UnreferencedSnapshots, + package: Option<&str>, +) -> Result<(), Box> { + enum Action { + Delete, + Reject, + Warn, + } + + let action = match unreferenced { + UnreferencedSnapshots::Auto => { + if is_ci() { + Action::Reject + } else { + Action::Delete + } + } + UnreferencedSnapshots::Reject => Action::Reject, + UnreferencedSnapshots::Delete => Action::Delete, + UnreferencedSnapshots::Warn => Action::Warn, + UnreferencedSnapshots::Ignore => return Ok(()), + }; + + let mut files = HashSet::new(); + match fs::read_to_string(path) { + Ok(s) => { + for line in s.lines() { + if let Ok(path) = fs::canonicalize(line) { + files.insert(path); + } + } + } + Err(err) => { + // if the file was not created, no test referenced + // snapshots. + if err.kind() != io::ErrorKind::NotFound { + return Err(err.into()); + } + } + } + + let mut encountered_any = false; + for entry in make_deletion_walker(&loc, package) { + let rel_path = match entry { + Ok(ref entry) => entry.path(), + _ => continue, + }; + if !rel_path.is_file() + || !rel_path + .file_name() + .map_or(false, |x| x.to_str().unwrap_or("").ends_with(".snap")) + { + continue; + } + + if let Ok(path) = fs::canonicalize(rel_path) { + if files.contains(&path) { + continue; + } + if !encountered_any { + match action { + Action::Delete => { + eprintln!("{}: deleted unreferenced snapshots:", style("info").bold()); + } + _ => { + eprintln!( + "{}: encountered unreferenced snapshots:", + style("warning").bold() + ); + } + } + encountered_any = true; + } + eprintln!(" {}", rel_path.display()); + if matches!(action, Action::Delete) { + fs::remove_file(path).ok(); + } + } + } + + fs::remove_file(&path).ok(); + + if !encountered_any { + eprintln!("{}: no unreferenced snapshots found", style("info").bold()); + } else if matches!(action, Action::Reject) { + return Err(err_msg("aborting because of unreferenced snapshots")); + } + + Ok(()) +} + fn prepare_test_runner<'snapshot_ref>( test_runner: TestRunner, + unreferenced: UnreferencedSnapshots, cmd: &TestCommand, color: &str, extra_args: &[&str], @@ -740,7 +804,8 @@ fn prepare_test_runner<'snapshot_ref>( proc } }; - let snapshot_ref_file = if cmd.delete_unreferenced_snapshots { + + let snapshot_ref_file = if unreferenced != UnreferencedSnapshots::Ignore { match snapshot_ref_file { Some(path) => Some(Cow::Borrowed(path)), None => { @@ -933,7 +998,7 @@ pub fn run() -> Result<(), Box> { process_snapshots( cmd.quiet, cmd.snapshot_filter.as_deref(), - handle_target_args(&cmd.target_args)?, + &handle_target_args(&cmd.target_args)?, match opts.command { Command::Review(_) => None, Command::Accept(_) => Some(Operation::Accept), diff --git a/src/env.rs b/src/env.rs index 88ed8293..19ca9cb6 100644 --- a/src/env.rs +++ b/src/env.rs @@ -45,6 +45,18 @@ pub enum OutputBehavior { Nothing, } +/// Unreferenced snapshots flag +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg(feature = "_cargo_insta_internal")] +pub enum UnreferencedSnapshots { + Auto, + Reject, + Delete, + Warn, + Ignore, +} + +/// Snapshot update flag #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum SnapshotUpdate { Always, @@ -59,6 +71,8 @@ pub enum Error { Deserialize(crate::content::Error), Io(std::io::Error), Env(&'static str), + #[allow(unused)] + Config(&'static str), } impl fmt::Display for Error { @@ -66,7 +80,8 @@ impl fmt::Display for Error { match self { Error::Deserialize(_) => write!(f, "failed to deserialize tool config"), Error::Io(_) => write!(f, "io error while reading tool config"), - Error::Env(msg) => write!(f, "invalid value for env var '{}'", msg), + Error::Env(var) => write!(f, "invalid value for env var '{}'", var), + Error::Config(var) => write!(f, "invalid value for config '{}'", var), } } } @@ -76,7 +91,7 @@ impl std::error::Error for Error { match self { Error::Deserialize(ref err) => Some(err), Error::Io(ref err) => Some(err), - Error::Env(_) => None, + _ => None, } } } @@ -93,6 +108,8 @@ pub struct ToolConfig { #[cfg(feature = "_cargo_insta_internal")] test_runner: TestRunner, #[cfg(feature = "_cargo_insta_internal")] + test_unreferenced: UnreferencedSnapshots, + #[cfg(feature = "_cargo_insta_internal")] auto_review: bool, #[cfg(feature = "_cargo_insta_internal")] auto_accept_unseen: bool, @@ -196,6 +213,14 @@ impl ToolConfig { .map_err(|_| Error::Env("INSTA_TEST_RUNNER"))? }, #[cfg(feature = "_cargo_insta_internal")] + test_unreferenced: { + resolve(&cfg, &["test", "unreferenced"]) + .and_then(|x| x.as_str()) + .unwrap_or("ignore") + .parse::() + .map_err(|_| Error::Config("unreferenced"))? + }, + #[cfg(feature = "_cargo_insta_internal")] auto_review: resolve(&cfg, &["test", "auto_review"]) .and_then(|x| x.as_bool()) .unwrap_or(false), @@ -248,6 +273,10 @@ impl ToolConfig { self.test_runner } + pub fn test_unreferenced(&self) -> UnreferencedSnapshots { + self.test_unreferenced + } + /// Returns the auto review flag. pub fn auto_review(&self) -> bool { self.auto_review @@ -355,6 +384,22 @@ impl std::str::FromStr for TestRunner { } } +#[cfg(feature = "_cargo_insta_internal")] +impl std::str::FromStr for UnreferencedSnapshots { + type Err = (); + + fn from_str(value: &str) -> Result { + match value { + "auto" => Ok(UnreferencedSnapshots::Auto), + "reject" | "error" => Ok(UnreferencedSnapshots::Reject), + "delete" => Ok(UnreferencedSnapshots::Delete), + "warn" => Ok(UnreferencedSnapshots::Warn), + "ignore" => Ok(UnreferencedSnapshots::Ignore), + _ => Err(()), + } + } +} + /// Memoizes a snapshot file in the reference file. pub fn memoize_snapshot_file(snapshot_file: &Path) { if let Ok(path) = env::var("INSTA_SNAPSHOT_REFERENCES_FILE") { diff --git a/src/lib.rs b/src/lib.rs index a27b4ad1..5ae07141 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -287,13 +287,15 @@ pub mod internals { #[doc(hidden)] #[cfg(feature = "_cargo_insta_internal")] pub mod _cargo_insta_support { - pub use crate::env::{ - Error as ToolConfigError, OutputBehavior, SnapshotUpdate, TestRunner, ToolConfig, - }; pub use crate::{ + env::{ + Error as ToolConfigError, OutputBehavior, SnapshotUpdate, TestRunner, ToolConfig, + UnreferencedSnapshots, + }, output::{print_snapshot, print_snapshot_diff}, snapshot::PendingInlineSnapshot, snapshot::SnapshotContents, + utils::is_ci, }; }