From e9deac1da21e197393620f6821dbea9387c8c17c Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Tue, 24 Sep 2024 00:36:12 -0700 Subject: [PATCH] Add a couple of docstrings / small refactors Helpful to me, hopefully helpful to others --- cargo-insta/src/container.rs | 30 ++++++++++++++++-------------- insta/src/output.rs | 18 +++++++----------- insta/src/runtime.rs | 5 +++-- insta/src/settings.rs | 8 ++++---- insta/src/snapshot.rs | 2 ++ 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/cargo-insta/src/container.rs b/cargo-insta/src/container.rs index 6a2ded50..125b2fec 100644 --- a/cargo-insta/src/container.rs +++ b/cargo-insta/src/container.rs @@ -47,7 +47,9 @@ impl PendingSnapshot { /// single rust file. #[derive(Debug)] pub(crate) struct SnapshotContainer { - snapshot_path: PathBuf, + // Path of the pending snapshot file (generally a `.new` or `.pending-snap` file) + pending_path: PathBuf, + // Path of the target snapshot file (generally a `.snap` file) target_path: PathBuf, kind: SnapshotKind, snapshots: Vec, @@ -56,7 +58,7 @@ pub(crate) struct SnapshotContainer { impl SnapshotContainer { pub(crate) fn load( - snapshot_path: PathBuf, + pending_path: PathBuf, target_path: PathBuf, kind: SnapshotKind, ) -> Result> { @@ -68,7 +70,7 @@ impl SnapshotContainer { } else { Some(Snapshot::from_file(&target_path)?) }; - let new = Snapshot::from_file(&snapshot_path)?; + let new = Snapshot::from_file(&pending_path)?; snapshots.push(PendingSnapshot { id: 0, old, @@ -79,7 +81,7 @@ impl SnapshotContainer { None } SnapshotKind::Inline => { - let mut pending_vec = PendingInlineSnapshot::load_batch(&snapshot_path)?; + let mut pending_vec = PendingInlineSnapshot::load_batch(&pending_path)?; let mut have_new = false; let rv = if fs::metadata(&target_path).is_ok() { @@ -111,8 +113,8 @@ impl SnapshotContainer { // The runtime code will issue something like this: // PendingInlineSnapshot::new(None, None, line).save(pending_snapshots)?; if !have_new { - fs::remove_file(&snapshot_path) - .map_err(|e| ContentError::FileIo(e, snapshot_path.to_path_buf()))?; + fs::remove_file(&pending_path) + .map_err(|e| ContentError::FileIo(e, pending_path.to_path_buf()))?; } rv @@ -120,7 +122,7 @@ impl SnapshotContainer { }; Ok(SnapshotContainer { - snapshot_path, + pending_path, target_path, kind, snapshots, @@ -141,7 +143,7 @@ impl SnapshotContainer { pub(crate) fn snapshot_sort_key(&self) -> impl Ord + '_ { let path = self - .snapshot_path + .pending_path .file_name() .and_then(|x| x.to_str()) .unwrap_or_default(); @@ -201,9 +203,9 @@ impl SnapshotContainer { patcher.save()?; } if did_skip { - PendingInlineSnapshot::save_batch(&self.snapshot_path, &new_pending)?; + PendingInlineSnapshot::save_batch(&self.pending_path, &new_pending)?; } else { - try_removing_snapshot(&self.snapshot_path); + try_removing_snapshot(&self.pending_path); } } else { // should only be one or this is weird @@ -211,22 +213,22 @@ impl SnapshotContainer { for snapshot in self.snapshots.iter() { match snapshot.op { Operation::Accept => { - let snapshot = Snapshot::from_file(&self.snapshot_path).map_err(|e| { + let snapshot = Snapshot::from_file(&self.pending_path).map_err(|e| { // If it's an IO error, pass a ContentError back so // we get a slightly clearer error message match e.downcast::() { Ok(io_error) => Box::new(ContentError::FileIo( *io_error, - self.snapshot_path.to_path_buf(), + self.pending_path.to_path_buf(), )), Err(other_error) => other_error, } })?; snapshot.save(&self.target_path)?; - try_removing_snapshot(&self.snapshot_path); + try_removing_snapshot(&self.pending_path); } Operation::Reject => { - try_removing_snapshot(&self.snapshot_path); + try_removing_snapshot(&self.pending_path); } Operation::Skip => {} } diff --git a/insta/src/output.rs b/insta/src/output.rs index 5c9d103b..089af78f 100644 --- a/insta/src/output.rs +++ b/insta/src/output.rs @@ -226,13 +226,8 @@ pub fn print_snapshot_summary( workspace_root: &Path, snapshot: &Snapshot, snapshot_file: Option<&Path>, - mut line: Option, + line: Option, ) { - // default to old assertion line from snapshot. - if line.is_none() { - line = snapshot.metadata().assertion_line(); - } - if let Some(snapshot_file) = snapshot_file { let snapshot_file = workspace_root .join(snapshot_file) @@ -255,11 +250,12 @@ pub fn print_snapshot_summary( println!( "Source: {}{}", style(value.display()).cyan(), - if let Some(line) = line { - format!(":{}", style(line).bold()) - } else { - "".to_string() - } + line.or( + // default to old assertion line from snapshot. + snapshot.metadata().assertion_line() + ) + .map(|line| format!(":{}", style(line).bold())) + .unwrap_or_default() ); } diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 73392edf..22dca6e3 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -201,7 +201,8 @@ fn get_snapshot_filename( }) } -/// A single snapshot including surrounding context which asserts and save the +/// The context around a snapshot, such as the reference value, location, etc. +/// (but not including the generated value). Responsible for saving the /// snapshot. #[derive(Debug)] struct SnapshotAssertionContext<'a> { @@ -620,7 +621,7 @@ where /// assertion with a panic if needed. #[allow(clippy::too_many_arguments)] pub fn assert_snapshot( - refval: ReferenceValue<'_>, + refval: ReferenceValue, new_snapshot_value: &str, manifest_dir: &str, function_name: &str, diff --git a/insta/src/settings.rs b/insta/src/settings.rs index 7128cfe3..fbf5b3a8 100644 --- a/insta/src/settings.rs +++ b/insta/src/settings.rs @@ -269,10 +269,10 @@ impl Settings { /// Sets the input file reference. /// - /// This value is completely unused by the snapshot testing system but - /// it lets you store some meta data with a snapshot that refers you back - /// to the input file. The path stored here is made relative to the - /// workspace root before storing with the snapshot. + /// This value is completely unused by the snapshot testing system but it + /// allows storing some metadata with a snapshot that refers back to the + /// input file. The path stored here is made relative to the workspace root + /// before storing with the snapshot. pub fn set_input_file>(&mut self, p: P) { self._private_inner_mut().input_file(p); } diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 45d3f0df..74e6ac12 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -418,6 +418,8 @@ impl Snapshot { fn as_content(&self) -> Content { let mut fields = vec![("module_name", Content::from(self.module_name.as_str()))]; + // Note this is currently never used, since this method is only used for + // inline snapshots if let Some(name) = self.snapshot_name.as_deref() { fields.push(("snapshot_name", Content::from(name))); }