Skip to content

Commit

Permalink
Clarify expect { }.not_to change { }.
Browse files Browse the repository at this point in the history
Fixes #154.
  • Loading branch information
myronmarston committed Dec 4, 2013
1 parent b5bec08 commit 953e3b3
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 10 deletions.
5 changes: 5 additions & 0 deletions Changelog.md
Expand Up @@ -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:

Expand Down
7 changes: 4 additions & 3 deletions lib/rspec/matchers.rb
Expand Up @@ -329,9 +329,10 @@ def be_within(delta)
# Evaluates <tt>receiver.message</tt> or <tt>block</tt> before and after it
# evaluates the block passed to <tt>expect</tt>.
#
# <tt>expect( ... ).not_to change</tt> only supports the form with no subsequent
# calls to <tt>by</tt>, <tt>by_at_least</tt>, <tt>by_at_most</tt>,
# <tt>to</tt> or <tt>from</tt>.
# `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
Expand Down
39 changes: 32 additions & 7 deletions lib/rspec/matchers/built_in/change.rb
Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
59 changes: 59 additions & 0 deletions spec/rspec/matchers/change_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit 953e3b3

Please sign in to comment.