Merge branch 'master' into trimming

CHANGELOG.md

@@ -4,6 +4,8 @@ All notable changes to insta and cargo-insta are documented here.
 
 ## 1.41.0
 
+- Experimental support for binary snapshots.  #610 (Florian Plattner)
+
 - We no longer trim starting newlines during assertions, which allows asserting
   the number of leading newlines match those in a snapshot value. Existing
   assertions with different leading newlines will pass with a warning suggesting
@@ -18,7 +20,16 @@ All notable changes to insta and cargo-insta are documented here.
 
 - Inline snapshots only use `#` characters as delimiters when required.  #603
 
-- Experimental support for binary snapshots.  #610 (Florian Plattner)
+- Warnings for undiscovered snapshots are more robust, and include files with
+  custom snapshot extensions.  #637
+
+- Insta runs correctly on packages which reference rust files in a parent path.  #626
+
+- Warnings are printed when any snapshot uses a legacy format.  #599
+
+- `insta` now internally uses `INSTA_UPDATE=force` rather than
+  `INSTA_FORCE_UPDATE`.  (This doesn't affect users of `cargo-insta`, which
+  handles this internally.)  #482
 
 ## 1.40.0
 

cargo-insta/Cargo.toml

@@ -38,4 +38,4 @@ walkdir = "2.3.1"
 similar= "2.2.1"
 itertools = "0.10.0"
 termcolor = "1.1.2"
-os_pipe = "0.9.0"
+os_pipe = "0.9.0"

cargo-insta/src/cli.rs

@@ -6,10 +6,11 @@ use std::{env, fs};
 use std::{io, process};
 
 use console::{set_colors_enabled, style, Key, Term};
-use insta::Snapshot;
 use insta::_cargo_insta_support::{
     is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots,
 };
+use insta::{internals::SnapshotContents, Snapshot};
+use itertools::Itertools;
 use semver::Version;
 use serde::Serialize;
 use uuid::Uuid;
@@ -468,7 +469,7 @@ fn handle_target_args<'a>(
         .iter()
         .map(|x| {
             if let Some(no_period) = x.strip_prefix(".") {
-                eprintln!("`{}` supplied as an extenstion. This will use `foo.{}` as file names; likely you want `{}` instead.", x, x, no_period)
+                eprintln!("`{}` supplied as an extension. This will use `foo.{}` as file names; likely you want `{}` instead.", x, x, no_period)
             };
             x.as_str()
         })
@@ -516,7 +517,15 @@ fn process_snapshots(
         if !quiet {
             println!("{}: no snapshots to review", style("done").bold());
             if loc.tool_config.review_warn_undiscovered() {
-                show_undiscovered_hint(loc.find_flags, &snapshot_containers, &roots, &loc.exts);
+                show_undiscovered_hint(
+                    loc.find_flags,
+                    &snapshot_containers
+                        .iter()
+                        .map(|x| x.0.clone())
+                        .collect_vec(),
+                    &roots,
+                    &loc.exts,
+                );
             }
         }
         return Ok(());
@@ -635,11 +644,12 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
             cmd.force_update_snapshots = true;
         }
     }
-
-    // --check always implies --no-force-pass as otherwise this command does not
-    // make a lot of sense.
-    if cmd.check {
-        cmd.no_force_pass = true
+    if cmd.no_force_pass {
+        cmd.check = true;
+        eprintln!(
+            "{}: `--no-force-pass` is deprecated. Please use --check to immediately raise an error on any non-matching snapshots.",
+            style("warning").bold().yellow()
+        )
     }
 
     // the tool config can also indicate that --accept-unseen should be picked
@@ -732,7 +742,8 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box<dyn Error>
         )?
     } else {
         let (snapshot_containers, roots) = load_snapshot_containers(&loc)?;
-        let snapshot_count = snapshot_containers.iter().map(|x| x.0.len()).sum::<usize>();
+        let snapshot_containers = snapshot_containers.into_iter().map(|x| x.0).collect_vec();
+        let snapshot_count = snapshot_containers.iter().map(|x| x.len()).sum::<usize>();
         if snapshot_count > 0 {
             eprintln!(
                 "{}: {} snapshot{} to review",
@@ -988,15 +999,6 @@ fn prepare_test_runner<'snapshot_ref>(
     }
     if !cmd.check {
         proc.env("INSTA_FORCE_PASS", "1");
-    } else if !cmd.no_force_pass {
-        proc.env("INSTA_FORCE_PASS", "1");
-        // If we're not running under cargo insta, raise a warning that this option
-        // is deprecated. (cargo insta still uses it when running under `--check`,
-        // but this will stop soon too)
-        eprintln!(
-            "{}: `--no-force-pass` is deprecated. Please use --check to immediately raise an error on any non-matching snapshots.",
-            style("warning").bold().yellow()
-    );
     }
 
     proc.env(
@@ -1130,11 +1132,15 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
         let is_inline = snapshot_container.snapshot_file().is_none();
         for snapshot_ref in snapshot_container.iter_snapshots() {
             if cmd.as_json {
-                let old_snapshot = snapshot_ref
-                    .old
-                    .as_ref()
-                    .map(|x| x.contents_string().unwrap());
-                let new_snapshot = snapshot_ref.new.contents_string().unwrap();
+                let old_snapshot = snapshot_ref.old.as_ref().map(|x| match x.contents() {
+                    SnapshotContents::Text(x) => x.to_string(),
+                    _ => unreachable!(),
+                });
+                let new_snapshot = match snapshot_ref.new.contents() {
+                    SnapshotContents::Text(x) => x.to_string(),
+                    _ => unreachable!(),
+                };
+
                 let info = if is_inline {
                     SnapshotKey::InlineSnapshot {
                         path: &target_file,
@@ -1160,7 +1166,7 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
 
 fn show_undiscovered_hint(
     find_flags: FindFlags,
-    snapshot_containers: &[(SnapshotContainer, &Package)],
+    snapshot_containers: &[SnapshotContainer],
     roots: &HashSet<PathBuf>,
     extensions: &[&str],
 ) {
@@ -1169,56 +1175,64 @@ fn show_undiscovered_hint(
         return;
     }
 
-    let mut found_extra = false;
     let found_snapshots = snapshot_containers
         .iter()
-        .filter_map(|x| x.0.snapshot_file())
+        .filter_map(|x| x.snapshot_file())
+        .map(|x| x.to_path_buf())
         .collect::<HashSet<_>>();
 
-    for root in roots {
-        for snapshot in make_snapshot_walker(
-            root,
-            extensions,
-            FindFlags {
-                include_ignored: true,
-                include_hidden: true,
-            },
-        )
-        .filter_map(|e| e.ok())
-        .filter(|x| {
-            let fname = x.file_name().to_string_lossy();
-            // TODO: use `extensions` here
-            fname.ends_with(".snap.new") || fname.ends_with(".pending-snap")
-        }) {
-            if !found_snapshots.contains(snapshot.path()) {
-                found_extra = true;
-                break;
-            }
-        }
-    }
+    let all_snapshots: HashSet<_> = roots
+        .iter()
+        .flat_map(|root| {
+            make_snapshot_walker(
+                root,
+                extensions,
+                FindFlags {
+                    include_ignored: true,
+                    include_hidden: true,
+                },
+            )
+            .filter_map(|e| e.ok())
+            .filter(|x| {
+                let fname = x.file_name().to_string_lossy();
+                extensions
+                    .iter()
+                    .any(|ext| fname.ends_with(&format!(".{}.new", ext)))
+                    || fname.ends_with(".pending-snap")
+            })
+            .map(|x| x.path().to_path_buf())
+        })
+        .collect();
+
+    let missed_snapshots = all_snapshots.difference(&found_snapshots).collect_vec();
 
     // we did not find any extra snapshots
-    if !found_extra {
+    if missed_snapshots.is_empty() {
         return;
     }
 
     let (args, paths) = match (find_flags.include_ignored, find_flags.include_hidden) {
-        (true, false) => ("--include-ignored", "ignored"),
-        (false, true) => ("--include-hidden", "hidden"),
+        (false, true) => ("--include-ignored", "ignored"),
+        (true, false) => ("--include-hidden", "hidden"),
         (false, false) => (
             "--include-ignored and --include-hidden",
             "ignored or hidden",
         ),
         (true, true) => unreachable!(),
     };
 
-    println!(
+    eprintln!(
         "{}: {}",
         style("warning").yellow().bold(),
         format_args!(
-            "found undiscovered snapshots in some paths which are not picked up by cargo \
-            insta. Use {} if you have snapshots in {} paths.",
-            args, paths,
+            "found undiscovered pending snapshots in some paths which are not picked up by cargo \
+            insta. Use {} if you have snapshots in {} paths. Files:\n{}",
+            args,
+            paths,
+            missed_snapshots
+                .iter()
+                .map(|x| x.display().to_string())
+                .join("\n")
         )
     );
 }

cargo-insta/src/container.rs

@@ -2,9 +2,9 @@ use std::error::Error;
 use std::fs;
 use std::path::{Path, PathBuf};
 
-use insta::Snapshot;
 pub(crate) use insta::TextSnapshotKind;
 use insta::_cargo_insta_support::{ContentError, PendingInlineSnapshot};
+use insta::{internals::SnapshotContents, Snapshot};
 
 use crate::inline::FilePatcher;
 
@@ -15,7 +15,7 @@ pub(crate) enum Operation {
     Skip,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub(crate) struct PendingSnapshot {
     #[allow(dead_code)]
     id: usize,
@@ -45,9 +45,9 @@ impl PendingSnapshot {
 /// A snapshot and its immediate context, which loads & saves the snapshot. It
 /// holds either a single file snapshot, or all the inline snapshots from a
 /// single rust file.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub(crate) struct SnapshotContainer {
-    // Path of the pending snapshot file (generally a `.new` or `.pending-snap` file)
+    // Path of the pending snapshot file (generally a `.snap.new` or `.pending-snap` file)
     pending_path: PathBuf,
     // Path of the target snapshot file (generally a `.snap` file)
     target_path: PathBuf,
@@ -186,7 +186,10 @@ impl SnapshotContainer {
                     Operation::Accept => {
                         patcher.set_new_content(
                             idx,
-                            snapshot.new.contents().as_string_contents().unwrap(),
+                            match snapshot.new.contents() {
+                                SnapshotContents::Text(c) => c,
+                                _ => unreachable!(),
+                            },
                         );
                         did_accept = true;
                     }

cargo-insta/src/inline.rs

@@ -10,13 +10,14 @@ use proc_macro2::TokenTree;
 use syn::__private::ToTokens;
 use syn::spanned::Spanned;
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 struct InlineSnapshot {
     start: (usize, usize),
     end: (usize, usize),
     indentation: usize,
 }
 
+#[derive(Clone)]
 pub(crate) struct FilePatcher {
     filename: PathBuf,
     lines: Vec<String>,