don't fail when file won't change

levinmr · Dec 8, 2022 · 17b1567 · 17b1567
1 parent 88caa84
commit 17b1567
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 66 deletions.
diff --git a/lib/rspec/snapshot/matchers/match_snapshot.rb b/lib/rspec/snapshot/matchers/match_snapshot.rb
@@ -50,7 +50,7 @@ def matches?(actual)
 
           @expected = read_snapshot
 
-          if should_write?
+          if @wrote
             false
           else
             @actual == @expected
@@ -69,6 +69,7 @@ def matches?(actual)
 
         private def write_snapshot
           return unless should_write?
+          @wrote = true
 
           RSpec.configuration.reporter.message(
             "Snapshot written: #{@snapshot_path}"
@@ -79,7 +80,7 @@ def matches?(actual)
         end
 
         private def should_write?
-          update_snapshots? || !File.exist?(@snapshot_path)
+          !File.exist?(@snapshot_path) || (update_snapshots? && read_snapshot != @actual)
         end
 
         private def update_snapshots?
@@ -102,7 +103,7 @@ def diffable?
         end
 
         def failure_message
-          if should_write?
+          if @wrote
             "failing because we wrote a snapshot"
           else
             "\nexpected: #{@expected}\n     got: #{@actual}\n"

diff --git a/spec/rspec/snapshot/matchers/match_snapshot_spec.rb b/spec/rspec/snapshot/matchers/match_snapshot_spec.rb
@@ -210,35 +210,20 @@ def dump(object)
             expect(serializer).not_to have_received(:dump)
           end
 
-          it 'opens the snapshot file for writing' do
-            expect(File).to have_received(:new).with(snapshot_filepath, 'w+')
-          end
-
-          it 'writes the snapshot file with the value' do
-            expect(file).to have_received(:write).with(value_to_match)
-          end
-
-          it 'logs the snapshot write with the RSpec reporter' do
-            expect(RSpec.configuration.reporter).to(
-              have_received(:message)
-                .with("Snapshot written: #{snapshot_filepath}")
-            )
-          end
-
           it 'opens the snapshot file for reading' do
-            expect(File).to have_received(:new).with(snapshot_filepath)
+            expect(File).to have_received(:new).with(snapshot_filepath).twice
           end
 
           it 'reads the snapshot file' do
-            expect(file).to have_received(:read)
+            expect(file).to have_received(:read).twice
           end
 
           it 'closes the snapshot file after reading and writing' do
             expect(file).to have_received(:close).twice
           end
 
-          it 'returns false' do
-            expect(@actual).to be(false)
+          it 'returns true' do
+            expect(@actual).to be(true)
           end
         end
 
@@ -258,35 +243,20 @@ def dump(object)
             expect(serializer).to have_received(:dump).with(value_to_match)
           end
 
-          it 'opens the snapshot file for writing' do
-            expect(File).to have_received(:new).with(snapshot_filepath, 'w+')
-          end
-
-          it 'writes the snapshot file with the serialized value' do
-            expect(file).to have_received(:write).with(serialized_value)
-          end
-
-          it 'logs the snapshot write with the RSpec reporter' do
-            expect(RSpec.configuration.reporter).to(
-              have_received(:message)
-                .with("Snapshot written: #{snapshot_filepath}")
-            )
-          end
-
           it 'opens the snapshot file for reading' do
-            expect(File).to have_received(:new).with(snapshot_filepath)
+            expect(File).to have_received(:new).with(snapshot_filepath).twice
           end
 
           it 'reads the snapshot file' do
-            expect(file).to have_received(:read)
+            expect(file).to have_received(:read).twice
           end
 
           it 'closes the snapshot file after reading and writing' do
             expect(file).to have_received(:close).twice
           end
 
-          it 'returns false' do
-            expect(@actual).to be(false)
+          it 'returns true' do
+            expect(@actual).to be(true)
           end
         end
       end
@@ -590,13 +560,6 @@ def dump(object)
             expect(file).to have_received(:write).with(value_to_match)
           end
 
-          it 'logs the snapshot write with the RSpec reporter' do
-            expect(RSpec.configuration.reporter).to(
-              have_received(:message)
-                .with("Snapshot written: #{snapshot_filepath}")
-            )
-          end
-
           it 'opens the snapshot file for reading' do
             expect(File).to have_received(:new).with(snapshot_filepath)
           end
@@ -700,24 +663,14 @@ def dump(object)
       subject.instance_variable_set(:@actual, actual)
     end
 
-    context 'when should_write? is true' do
-      it 'returns a failure message including the actual and expected' do
-        expect(subject.failure_message).to(
-          eq("failing because we wrote a snapshot")
-        )
-      end
-    end
-
-    context 'when should_write? is false' do
-      before {
-        allow(subject).to receive(:should_write?).and_return(false)
-      }
+    before {
+      allow(subject).to receive(:should_write?).and_return(false)
+    }
 
-      it 'returns a failure message including the actual and expected' do
-        expect(subject.failure_message).to(
-          eq("\nexpected: #{expected}\n     got: #{actual}\n")
-        )
-      end
+    it 'returns a failure message including the actual and expected' do
+      expect(subject.failure_message).to(
+        eq("\nexpected: #{expected}\n     got: #{actual}\n")
+      )
     end
   end
 

diff --git a/spec/rspec/snapshot/matchers_spec.rb b/spec/rspec/snapshot/matchers_spec.rb
@@ -165,7 +165,9 @@ class TestClass
             FileUtils.rm_rf(snapshot_dir)
 
             # rubocop:disable RSpec/ExpectInHook
-            expect(expected).to match_snapshot(snapshot_name)
+            expect {
+              expect(expected).to match_snapshot(snapshot_name)
+            }.to raise_error
             # rubocop:enable RSpec/ExpectInHook
             file = File.new(snapshot_path)
             @actual = file.read
@@ -355,7 +357,9 @@ def dump(object)
         before do
           File.unlink(snapshot_path) if File.exist?(snapshot_path)
           # rubocop:disable RSpec/ExpectInHook
+          expect {
           expect(snapshot_value).to match_snapshot(snapshot_name)
+          }.to raise_error
           # rubocop:enable RSpec/ExpectInHook
           file = File.new(snapshot_path)
           @actual = file.read