Remove duplicate write check (#583)

Stacks on #581, merge that first

Now that we've consolidated the checks for whether to write snapshots
into `matches_fully`, I think we can remove the check on whether the
file contents match.

insta/src/runtime.rs

@@ -389,8 +389,8 @@ impl<'a> SnapshotAssertionContext<'a> {
         match snapshot_update {
             SnapshotUpdateBehavior::InPlace => {
                 if let Some(ref snapshot_file) = self.snapshot_file {
-                    let saved = new_snapshot.save(snapshot_file)?;
-                    if should_print && saved {
+                    new_snapshot.save(snapshot_file)?;
+                    if should_print {
                         elog!(
                             "{} {}",
                             if unseen {
@@ -409,14 +409,13 @@ impl<'a> SnapshotAssertionContext<'a> {
             SnapshotUpdateBehavior::NewFile => {
                 if let Some(ref snapshot_file) = self.snapshot_file {
                     // File snapshot
-                    if let Some(new_path) = new_snapshot.save_new(snapshot_file)? {
-                        if should_print {
-                            elog!(
-                                "{} {}",
-                                style("stored new snapshot").green(),
-                                style(new_path.display()).cyan().underlined(),
-                            );
-                        }
+                    let new_path = new_snapshot.save_new(snapshot_file)?;
+                    if should_print {
+                        elog!(
+                            "{} {}",
+                            style("stored new snapshot").green(),
+                            style(new_path.display()).cyan().underlined(),
+                        );
                     }
                 } else if self.is_doctest {
                     if should_print {
@@ -690,11 +689,6 @@ pub fn assert_snapshot(
             // Avoid creating new files if contents match exactly. In
             // particular, this would otherwise create lots of unneeded files
             // for inline snapshots
-            //
-            // Note that there's a check down the stack on whether the file
-            // contents match exactly for file snapshots; probably we should
-            // combine that check with `matches_fully` and then use a single
-            // check for whether we force update snapshots.
             let matches_fully = &ctx
                 .old_snapshot
                 .as_ref()

insta/src/snapshot.rs

@@ -484,63 +484,33 @@ impl Snapshot {
         buf
     }
 
-    fn save_with_metadata(
-        &self,
-        path: &Path,
-        ref_file: Option<&Path>,
-        md: &MetaData,
-    ) -> Result<bool, Box<dyn Error>> {
+    fn save_with_metadata(&self, path: &Path, md: &MetaData) -> Result<(), Box<dyn Error>> {
         if let Some(folder) = path.parent() {
             fs::create_dir_all(folder)?;
         }
 
         let serialized_snapshot = self.serialize_snapshot(md);
-
-        // check the reference file for contents.  Note that we always want to
-        // compare snapshots that were trimmed to persistence here.  This is a
-        // stricter check than even `matches_fully`, since it's comparing the
-        // exact contents of the file.
-        if let Ok(old) = fs::read_to_string(ref_file.unwrap_or(path)) {
-            let persisted = match md.trim_for_persistence() {
-                Cow::Owned(trimmed) => Cow::Owned(self.serialize_snapshot(&trimmed)),
-                Cow::Borrowed(trimmed) => {
-                    // This condition needs to hold, otherwise we need to
-                    // compare the old value to a newly trimmed serialized snapshot
-                    debug_assert_eq!(trimmed, md);
-
-                    Cow::Borrowed(&serialized_snapshot)
-                }
-            };
-            if old == persisted.as_str() {
-                return Ok(false);
-            }
-        }
-
         fs::write(path, serialized_snapshot)?;
-        Ok(true)
+        Ok(())
     }
 
     /// Saves the snapshot.
     ///
     /// Returns `true` if the snapshot was saved.  This will return `false` if there
     /// was already a snapshot with matching contents.
     #[doc(hidden)]
-    pub fn save(&self, path: &Path) -> Result<bool, Box<dyn Error>> {
-        self.save_with_metadata(path, None, &self.metadata.trim_for_persistence())
+    pub fn save(&self, path: &Path) -> Result<(), Box<dyn Error>> {
+        self.save_with_metadata(path, &self.metadata.trim_for_persistence())
     }
 
     /// Same as `save` but instead of writing a normal snapshot file this will write
     /// a `.snap.new` file with additional information.
     ///
-    /// If the existing snapshot matches the new file, then `None` is returned, otherwise
-    /// the name of the new snapshot file.
-    pub(crate) fn save_new(&self, path: &Path) -> Result<Option<PathBuf>, Box<dyn Error>> {
+    /// The name of the new snapshot file is returned.
+    pub(crate) fn save_new(&self, path: &Path) -> Result<PathBuf, Box<dyn Error>> {
         let new_path = path.to_path_buf().with_extension("snap.new");
-        if self.save_with_metadata(&new_path, Some(path), &self.metadata)? {
-            Ok(Some(new_path))
-        } else {
-            Ok(None)
-        }
+        self.save_with_metadata(&new_path, &self.metadata)?;
+        Ok(new_path)
     }
 }