diff --git a/Changelog.md b/Changelog.md index 2f3931de7..54196c339 100644 --- a/Changelog.md +++ b/Changelog.md @@ -21,10 +21,15 @@ Breaking Changes for 3.0.0: `Rspec` or `Spec`. (Myron Marston) * Remove deprecated `RSpec::Expectations.differ=`. (Myron Marston) * Remove support for deprecated `expect(...).should`. (Myron Marston) +* Explicitly disallow `expect { }.not_to change { }` with `by`, + `by_at_least`, `by_at_most` or `to`. These have never been supported + but did not raise explicit errors. (Myron Marston) Bug Fixes: * Fix wrong matcher descriptions with falsey expected value (yujinakayama) +* Fix `expect { }.not_to change { }.from(x)` so that the matcher only + passes if the starting value is `x`. (Tyler Rick, Myron Marston) Deprecations: diff --git a/lib/rspec/matchers.rb b/lib/rspec/matchers.rb index 0ca41c487..6152fcf4a 100644 --- a/lib/rspec/matchers.rb +++ b/lib/rspec/matchers.rb @@ -329,9 +329,10 @@ def be_within(delta) # Evaluates receiver.message or block before and after it # evaluates the block passed to expect. # - # expect( ... ).not_to change only supports the form with no subsequent - # calls to by, by_at_least, by_at_most, - # to or from. + # `expect( ... ).not_to change` supports the form that specifies `from` + # (which specifies what you expect the starting, unchanged value to be) + # but does not support forms with subsequent calls to `by`, `by_at_least`, + # `by_at_most` or `to`. def change(receiver=nil, message=nil, &block) BuiltIn::Change.new(receiver, message, &block) end diff --git a/lib/rspec/matchers/built_in/change.rb b/lib/rspec/matchers/built_in/change.rb index a6af6e025..6970caec6 100644 --- a/lib/rspec/matchers/built_in/change.rb +++ b/lib/rspec/matchers/built_in/change.rb @@ -5,17 +5,17 @@ module BuiltIn class Change # Specifies the delta of the expected change. def by(expected_delta) - ChangeRelatively.new(@change_details, expected_delta, :==, "by") + ChangeRelatively.new(@change_details, expected_delta, :==, :by) end # Specifies a minimum delta of the expected change. def by_at_least(minimum) - ChangeRelatively.new(@change_details, minimum, :>=, "by at least") + ChangeRelatively.new(@change_details, minimum, :>=, :by_at_least) end # Specifies a maximum delta of the expected change. def by_at_most(maximum) - ChangeRelatively.new(@change_details, maximum, :<=, "by at most") + ChangeRelatively.new(@change_details, maximum, :<=, :by_at_most) end # Specifies the new value you expect. @@ -66,15 +66,15 @@ def raise_block_syntax_error # Used to specify a relative change. # @api private class ChangeRelatively - def initialize(change_details, expected_delta, comparison, description) + def initialize(change_details, expected_delta, comparison, relativity) @change_details = change_details @expected_delta = expected_delta @comparison = comparison - @description = description + @relativity = relativity end def failure_message - "expected #{@change_details.message} to have changed #{@description} #{@expected_delta.inspect}, " + + "expected #{@change_details.message} to have changed #{@relativity.to_s.gsub("_", " ")} #{@expected_delta.inspect}, " + "but was changed by #{@change_details.actual_delta.inspect}" end @@ -83,9 +83,13 @@ def matches?(event_proc) @change_details.actual_delta.__send__(@comparison, @expected_delta) end + def does_not_match?(event_proc) + raise NotImplementedError, "`expect { }.not_to change { }.#{@relativity}()` is not supported" + end + # @api private def description - "change #{@change_details.message} #{@description} #{@expected_delta.inspect}" + "change #{@change_details.message} #{@relativity.to_s.gsub("_", " ")} #{@expected_delta.inspect}" end end @@ -151,6 +155,23 @@ def to(value) self end + def does_not_match?(event_proc) + if @description_suffix + raise NotImplementedError, "`expect { }.not_to change { }.to()` is not supported" + end + + @change_details.perform_change(event_proc) + !@change_details.changed? && matches_before? + end + + def failure_message_when_negated + if !matches_before? + "expected #{@change_details.message} to have initially been #{@expected_before.inspect}, but was #{@change_details.actual_before.inspect}" + else + "expected #{@change_details.message} not to have changed, but did change from #{@change_details.actual_before.inspect} to #{@change_details.actual_after.inspect}" + end + end + def description "change #{@change_details.message} from #{@expected_before.inspect}#{@description_suffix}" end @@ -171,6 +192,10 @@ def from(value) self end + def does_not_match?(event_proc) + raise NotImplementedError, "`expect { }.not_to change { }.to()` is not supported" + end + def description "change #{@change_details.message} to #{@expected_after.inspect}#{@description_suffix}" end diff --git a/spec/rspec/matchers/change_spec.rb b/spec/rspec/matchers/change_spec.rb index 40dd488c0..6934ff571 100644 --- a/spec/rspec/matchers/change_spec.rb +++ b/spec/rspec/matchers/change_spec.rb @@ -247,6 +247,65 @@ def ==(other) end end +describe "expect { ... }.not_to change { }.from" do + it 'passes when the value starts at the from value and does not change' do + k = 5 + expect { }.not_to change { k }.from(5) + end + + it 'fails when the value starts at a different value and does not change' do + expect { + k = 6 + expect { }.not_to change { k }.from(5) + }.to fail_with(/expected result to have initially been 5/) + end + + it 'fails when the value starts at the from value and changes' do + expect { + k = 5 + expect { k += 1 }.not_to change { k }.from(5) + }.to fail_with(/but did change from 5 to 6/) + end +end + +describe "expect { ... }.not_to change { }.to" do + it 'is not supported' do + expect { + expect { }.not_to change { }.to(3) + }.to raise_error(NotImplementedError) + end + + it 'is not supported when it comes after `from`' do + expect { + expect { }.not_to change { }.from(nil).to(3) + }.to raise_error(NotImplementedError) + end +end + +describe "expect { ... }.not_to change { }.by" do + it 'is not supported' do + expect { + expect { }.not_to change { }.by(3) + }.to raise_error(NotImplementedError) + end +end + +describe "expect { ... }.not_to change { }.by_at_least" do + it 'is not supported' do + expect { + expect { }.not_to change { }.by_at_least(3) + }.to raise_error(NotImplementedError) + end +end + +describe "expect { ... }.not_to change { }.by_at_most" do + it 'is not supported' do + expect { + expect { }.not_to change { }.by_at_most(3) + }.to raise_error(NotImplementedError) + end +end + describe "expect { ... }.to change(actual, message).by(expected)" do before(:each) do @instance = SomethingExpected.new