From af0361da0ac7e5b7703e772ce69c21c3315a54d0 Mon Sep 17 00:00:00 2001
From: Daniel Ma <drailskid@yahoo.com>
Date: Mon, 30 Oct 2017 11:21:28 -0700
Subject: `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
---
 activesupport/CHANGELOG.md                             |  5 +++++
 activesupport/lib/active_support/testing/assertions.rb | 11 ++++++-----
 activesupport/test/test_case_test.rb                   | 11 +++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

(limited to 'activesupport')

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
 
-- 
cgit v1.2.3