From 14004e447e9742e89dd6476d6832829ed9c30c31 Mon Sep 17 00:00:00 2001 From: Michael Glass Date: Thu, 8 Dec 2022 16:40:24 +0100 Subject: [PATCH 1/3] fail the spec whenever we write a snapshot --- lib/rspec/snapshot/matchers/match_snapshot.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/rspec/snapshot/matchers/match_snapshot.rb b/lib/rspec/snapshot/matchers/match_snapshot.rb index 921a83c..9dabefe 100644 --- a/lib/rspec/snapshot/matchers/match_snapshot.rb +++ b/lib/rspec/snapshot/matchers/match_snapshot.rb @@ -50,7 +50,11 @@ def matches?(actual) @expected = read_snapshot - @actual == @expected + if should_write? + false + else + @actual == @expected + end end # === is the method called when matching an argument @@ -98,7 +102,11 @@ def diffable? end def failure_message - "\nexpected: #{@expected}\n got: #{@actual}\n" + if should_write? + "failing because we wrote a snapshot" + else + "\nexpected: #{@expected}\n got: #{@actual}\n" + end end def failure_message_when_negated From 88caa847438948f84cc325436dc10af460bf6ed6 Mon Sep 17 00:00:00 2001 From: Michael Glass Date: Thu, 8 Dec 2022 16:52:46 +0100 Subject: [PATCH 2/3] fail the spec whenever we write a snapshot --- .../snapshot/matchers/match_snapshot_spec.rb | 46 ++++++++++++------- spec/rspec/snapshot/matchers_spec.rb | 6 ++- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/spec/rspec/snapshot/matchers/match_snapshot_spec.rb b/spec/rspec/snapshot/matchers/match_snapshot_spec.rb index af030f9..d497b47 100644 --- a/spec/rspec/snapshot/matchers/match_snapshot_spec.rb +++ b/spec/rspec/snapshot/matchers/match_snapshot_spec.rb @@ -237,8 +237,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end @@ -285,8 +285,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end end @@ -335,8 +335,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end @@ -383,8 +383,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end end @@ -609,8 +609,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end @@ -657,8 +657,8 @@ def dump(object) expect(file).to have_received(:close).twice end - it 'returns true' do - expect(@actual).to be(true) + it 'returns false' do + expect(@actual).to be(false) end end end @@ -700,10 +700,24 @@ def dump(object) subject.instance_variable_set(:@actual, actual) end - it 'returns a failure message including the actual and expected' do - expect(subject.failure_message).to( - eq("\nexpected: #{expected}\n got: #{actual}\n") - ) + 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) + } + + 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 end diff --git a/spec/rspec/snapshot/matchers_spec.rb b/spec/rspec/snapshot/matchers_spec.rb index 48a5dea..67105ea 100644 --- a/spec/rspec/snapshot/matchers_spec.rb +++ b/spec/rspec/snapshot/matchers_spec.rb @@ -272,7 +272,9 @@ def dump(object) file.write(original_snapshot_value) file.close # rubocop:disable RSpec/ExpectInHook - expect(updated_snapshot_value).to match_snapshot(snapshot_name) + expect { + expect(updated_snapshot_value).to match_snapshot(snapshot_name) + }.to raise_error # rubocop:enable RSpec/ExpectInHook file = File.new(snapshot_path) @actual = file.read @@ -295,7 +297,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 From 17b1567a5d073fd4cee207cf388b835329fb5b88 Mon Sep 17 00:00:00 2001 From: Michael Glass Date: Thu, 8 Dec 2022 17:42:31 +0100 Subject: [PATCH 3/3] don't fail when file won't change --- lib/rspec/snapshot/matchers/match_snapshot.rb | 7 +- .../snapshot/matchers/match_snapshot_spec.rb | 77 ++++--------------- spec/rspec/snapshot/matchers_spec.rb | 6 +- 3 files changed, 24 insertions(+), 66 deletions(-) diff --git a/lib/rspec/snapshot/matchers/match_snapshot.rb b/lib/rspec/snapshot/matchers/match_snapshot.rb index 9dabefe..cc9e4d9 100644 --- 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 index d497b47..39c0d0f 100644 --- 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 index 67105ea..b0b3e38 100644 --- 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