Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improve `change` failure messages. #422

Merged
merged 1 commit into from

2 participants

@myronmarston

There were a couple of cases that produced puzzling failure messages:

k = "foo"
expect { }.to change { k }.to("foo")

..used to fail with:

expected #some_value to have changed to "foo", but is now "foo"

..and now fails with:

expected #some_value to have changed to "foo", but did not change

k = "foo"
expect { }.to change { k }.from("foo")

..used to fail with:

expected result to have changed to BasicObject, but is now "foo"

..and now fails with:

expected result to have changed from "foo", but did not change

@myronmarston myronmarston Improve `change` failure messages.
There were a couple of cases that produced puzzling failure messages:

k = "foo"
expect { }.to change { k }.to("foo")

..used to fail with:

  expected #some_value to have changed to "foo", but is now "foo"

..and now fails with:

  expected #some_value to have changed to "foo", but did not change

k = "foo"
expect { }.to change { k }.from("foo")

..used to fail with:

  expected result to have changed to BasicObject, but is now "foo"

..and now fails with:

  expected result to have changed from "foo", but did not change
a60a8f8
@JonRowe
Owner

LGTM

@JonRowe JonRowe merged commit 7edf388 into master
@JonRowe JonRowe deleted the fix-change-failure-message branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 12, 2014
  1. @myronmarston

    Improve `change` failure messages.

    myronmarston authored
    There were a couple of cases that produced puzzling failure messages:
    
    k = "foo"
    expect { }.to change { k }.to("foo")
    
    ..used to fail with:
    
      expected #some_value to have changed to "foo", but is now "foo"
    
    ..and now fails with:
    
      expected #some_value to have changed to "foo", but did not change
    
    k = "foo"
    expect { }.to change { k }.from("foo")
    
    ..used to fail with:
    
      expected result to have changed to BasicObject, but is now "foo"
    
    ..and now fails with:
    
      expected result to have changed from "foo", but did not change
This page is out of date. Refresh to see the latest.
View
47 lib/rspec/matchers/built_in/change.rb
@@ -119,12 +119,14 @@ def matches?(event_proc)
@change_details.changed? && matches_before? && matches_after?
end
+ def description
+ "change #{@change_details.message} #{change_description}"
+ end
+
def failure_message
- if !matches_before?
- "expected #{@change_details.message} to have initially been #{description_of @expected_before}, but was #{description_of @change_details.actual_before}"
- else
- "expected #{@change_details.message} to have changed to #{description_of @expected_after}, but is now #{description_of @change_details.actual_after}"
- end
+ return before_value_failure unless matches_before?
+ return did_not_change_failure unless @change_details.changed?
+ after_value_failure
end
private
@@ -136,6 +138,22 @@ def matches_before?
def matches_after?
values_match?(@expected_after, @change_details.actual_after)
end
+
+ def before_value_failure
+ "expected #{@change_details.message} to have initially been #{description_of @expected_before}, but was #{description_of @change_details.actual_before}"
+ end
+
+ def after_value_failure
+ "expected #{@change_details.message} to have changed to #{description_of @expected_after}, but is now #{description_of @change_details.actual_after}"
+ end
+
+ def did_not_change_failure
+ "expected #{@change_details.message} to have changed #{change_description}, but did not change"
+ end
+
+ def did_change_failure
+ "expected #{@change_details.message} not to have changed, but did change from #{description_of @change_details.actual_before} to #{description_of @change_details.actual_after}"
+ end
end
# Used to specify a change from a specific value
@@ -163,15 +181,14 @@ def does_not_match?(event_proc)
end
def failure_message_when_negated
- if !matches_before?
- "expected #{@change_details.message} to have initially been #{description_of @expected_before}, but was #{description_of @change_details.actual_before}"
- else
- "expected #{@change_details.message} not to have changed, but did change from #{description_of @change_details.actual_before} to #{description_of @change_details.actual_after}"
- end
+ return before_value_failure unless matches_before?
+ did_change_failure
end
- def description
- "change #{@change_details.message} from #{description_of @expected_before}#{@description_suffix}"
+ private
+
+ def change_description
+ "from #{description_of @expected_before}#{@description_suffix}"
end
end
@@ -194,8 +211,10 @@ def does_not_match?(event_proc)
raise NotImplementedError, "`expect { }.not_to change { }.to()` is not supported"
end
- def description
- "change #{@change_details.message} to #{description_of @expected_after}#{@description_suffix}"
+ private
+
+ def change_description
+ "to #{description_of @expected_after}#{@description_suffix}"
end
end
View
12 spec/rspec/matchers/built_in/change_spec.rb
@@ -527,6 +527,12 @@ def ==(other)
end.to fail_with("expected result to have initially been \"cat\", but was \"string\"")
end
+ it "fails when attribute does not change" do
+ expect do
+ expect { }.to change { @instance.some_value }.from("string")
+ end.to fail_with('expected result to have changed from "string", but did not change')
+ end
+
it "provides a #description" do
expect(change { }.from(3).description).to eq "change result from 3"
end
@@ -565,6 +571,12 @@ def ==(other)
expect { @instance.some_value = "cat" }.to change(@instance, :some_value).from("string").to("dog")
end.to fail_with("expected #some_value to have changed to \"dog\", but is now \"cat\"")
end
+
+ it "fails with a clear message when it ends with the right value but did not change" do
+ expect {
+ expect { }.to change(@instance, :some_value).to("string")
+ }.to fail_with('expected #some_value to have changed to "string", but did not change')
+ end
end
end
Something went wrong with that request. Please try again.