aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport
diff options
context:
space:
mode:
authorDaniel Ma <drailskid@yahoo.com>2017-10-30 11:21:28 -0700
committerDaniel Ma <drailskid@yahoo.com>2017-11-13 11:49:35 -0800
commitaf0361da0ac7e5b7703e772ce69c21c3315a54d0 (patch)
tree765fa913f758f7dfc67eaac2e37bbc5c154e0bd0 /activesupport
parent705cf47033afabf4530a209f907ff4bf35acf2c2 (diff)
downloadrails-af0361da0ac7e5b7703e772ce69c21c3315a54d0.tar.gz
rails-af0361da0ac7e5b7703e772ce69c21c3315a54d0.tar.bz2
rails-af0361da0ac7e5b7703e772ce69c21c3315a54d0.zip
`assert_changes` should always assert some change
While using `assert_changes`, I came across some unexpected behavior: if you provide a `to:` argument, and the expression matches but didn't actually change, the assertion will pass. The way `assert_changes` reads, I assumed that it would both assert that there was any change at all, _and_ that the expression changed to match my `to:` argument. In the case of just a `from:` argument, `assert_changes` does what I expect as well. It asserts that the before value `=== from` and that the after value changed. My key change is that `assert_changes` will now _always_ assert that expression changes, no matter what combination of `from:` and `to:` arguments
Diffstat (limited to 'activesupport')
-rw-r--r--activesupport/CHANGELOG.md5
-rw-r--r--activesupport/lib/active_support/testing/assertions.rb11
-rw-r--r--activesupport/test/test_case_test.rb11
3 files changed, 22 insertions, 5 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 889919855c..a4c90c8b44 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+* `assert_changes` will always assert that the expression changes,
+ regardless of `from:` and `to:` argument combinations.
+
+ *Daniel Ma*
+
* Allow `Range#include?` on TWZ ranges
In #11474 we prevented TWZ ranges being iterated over which matched
diff --git a/activesupport/lib/active_support/testing/assertions.rb b/activesupport/lib/active_support/testing/assertions.rb
index b24aa36ede..6f69c48674 100644
--- a/activesupport/lib/active_support/testing/assertions.rb
+++ b/activesupport/lib/active_support/testing/assertions.rb
@@ -156,11 +156,12 @@ module ActiveSupport
after = exp.call
- if to == UNTRACKED
- error = "#{expression.inspect} didn't change"
- error = "#{message}.\n#{error}" if message
- assert before != after, error
- else
+ error = "#{expression.inspect} didn't change"
+ error = "#{error}. It was already #{to}" if before == to
+ error = "#{message}.\n#{error}" if message
+ assert before != after, error
+
+ unless to == UNTRACKED
error = "#{expression.inspect} didn't change to #{to}"
error = "#{message}.\n#{error}" if message
assert to === after, error
diff --git a/activesupport/test/test_case_test.rb b/activesupport/test/test_case_test.rb
index 84e4953fe2..79b75a001a 100644
--- a/activesupport/test/test_case_test.rb
+++ b/activesupport/test/test_case_test.rb
@@ -156,6 +156,16 @@ class AssertDifferenceTest < ActiveSupport::TestCase
end
end
+ def test_assert_changes_with_to_option_but_no_change_has_special_message
+ error = assert_raises Minitest::Assertion do
+ assert_changes "@object.num", to: 0 do
+ # no changes
+ end
+ end
+
+ assert_equal "\"@object.num\" didn't change. It was already 0", error.message
+ end
+
def test_assert_changes_with_wrong_to_option
assert_raises Minitest::Assertion do
assert_changes "@object.num", to: 2 do
@@ -218,6 +228,7 @@ class AssertDifferenceTest < ActiveSupport::TestCase
def test_assert_changes_with_message
error = assert_raises Minitest::Assertion do
assert_changes "@object.num", "@object.num should 1", to: 1 do
+ @object.decrement
end
end