Fix failure message for numericality matcher

Secondary author: Mauro George <maurogot@gmail.com>

Sometimes the failure message did not always provide the reason for
failure. For instance, given this validation:

    validates :ano, numericality: { only_integer: true, greater_than_or_equal_to: 1900 }

this test:

    it { should validate_numericality_of(:ano).is_greater_than_or_equal_to(1900) }

would fail with the following:

    Did not expect errors  when ano is set to 1900.000000000001, got error:

as you can see, the failure message doesn't contain the validation
error.

In the process of making this fix, we changed how the matcher fails so
that it will so when the first submatcher fails, not the last. This
changes what the failure message looks like, but not the basic
functionality of the matcher itself.

NEWS.md

@@ -154,6 +154,9 @@
 * Fix `validate_uniqueness_of` so that it allows you to test against scoped
   attributes that are boolean columns. ([#457], [#694])
 
+* Fix failure message for `validate_numericality_of` as it sometimes didn't
+  provide the reason for failure. ([#699])
+
 ### Features
 
 * Add `on` qualifier to `permit`. This allows you to make an assertion that
@@ -196,6 +199,7 @@
 [#457]: https://github.com/thoughtbot/shoulda-matchers/pull/457
 [#694]: https://github.com/thoughtbot/shoulda-matchers/pull/694
 [#692]: https://github.com/thoughtbot/shoulda-matchers/pull/692
+[#699]: https://github.com/thoughtbot/shoulda-matchers/pull/699
 
 # 2.8.0
 

lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb

@@ -391,7 +391,7 @@ module Shoulda
 
         def matches?(subject)
           @subject = subject
-          failing_submatchers.empty?
+          first_failing_submatcher.nil?
         end
 
         def description
@@ -410,12 +410,12 @@ module Shoulda
         end
 
         def failure_message
-          last_failing_submatcher.failure_message
+          first_failing_submatcher.failure_message
         end
         alias failure_message_for_should failure_message
 
         def failure_message_when_negated
-          last_failing_submatcher.failure_message_when_negated
+          first_failing_submatcher.failure_message_when_negated
         end
         alias failure_message_for_should_not failure_message_when_negated
 
@@ -450,21 +450,10 @@ module Shoulda
           @diff_to_compare = [@diff_to_compare, matcher.diff_to_compare].max
         end
 
-        def submatchers_and_results
-          @_submatchers_and_results ||=
-            @submatchers.map do |matcher|
-              { matcher: matcher, matched: matcher.matches?(@subject) }
-            end
-        end
-
-        def failing_submatchers
-          submatchers_and_results.
-            select { |x| !x[:matched] }.
-            map { |x| x[:matcher] }
-        end
-
-        def last_failing_submatcher
-          failing_submatchers.last
+        def first_failing_submatcher
+          @_first_failing_submatcher ||= @submatchers.detect do |submatcher|
+            !submatcher.matches?(@subject)
+          end
         end
 
         def allowed_types

spec/support/unit/helpers/rails_versions.rb

@@ -20,5 +20,9 @@ module UnitTests
     def rails_gte_4_1?
       rails_version >= 4.1
     end
+
+    def rails_gte_4_2?
+      rails_version >= 4.2
+    end
   end
 end

spec/support/unit/matchers/fail_with_message_matcher.rb

@@ -16,7 +16,7 @@ module UnitTests
           @actual = ex.message
         end
 
-        @actual && @actual == expected
+        @actual && @actual == expected.sub(/\n\z/, '')
       end
 
       def failure_message

spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb

@@ -500,163 +500,297 @@ describe Shoulda::Matchers::ActiveModel::ValidateNumericalityOfMatcher, type: :m
     end
 
     context 'when the qualifiers do not match the validation options' do
-      it do
+      specify 'such as validating even but testing that only_integer is validated' do
         record = build_record_validating_numericality(
           even: true,
           greater_than: 18
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be an integer" when attr is set to 0.1,
+            got errors:
+            * "must be greater than 18" (attribute: attr, value: "0.1")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as not validating only_integer but testing that only_integer is validated' do
         record = build_record_validating_numericality(greater_than: 18)
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be an integer" when attr is set to 0.1,
+            got errors:
+            * "must be greater than 18" (attribute: attr, value: "0.1")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as validating greater_than_or_equal_to (+ even) but testing that greater_than is validated' do
         record = build_record_validating_numericality(
           even: true,
           greater_than_or_equal_to: 18
         )
-        expect(record).
-          not_to validate_numericality.
-          even.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            even.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as validating odd (+ greater_than) but testing that even is validated' do
         record = build_record_validating_numericality(
           odd: true,
           greater_than: 18
         )
-        expect(record).
-          not_to validate_numericality.
-          even.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            even.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be even" when attr is set to 1,
+            got errors:
+            * "must be greater than 18" (attribute: attr, value: "1")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as validating greater_than_or_equal_to (+ odd) but testing that is_less_than_or_equal_to is validated' do
         record = build_record_validating_numericality(
           odd: true,
           greater_than_or_equal_to: 99
         )
-        expect(record).
-          not_to validate_numericality.
-          odd.
-          is_less_than_or_equal_to(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            odd.
+            is_less_than_or_equal_to(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be less than or equal to 99" when attr is set to 101,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as validating greater_than_or_equal_to (+ only_integer + less_than) but testing that greater_than is validated' do
         record = build_record_validating_numericality(
           only_integer: true,
           greater_than_or_equal_to: 18,
           less_than: 99
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18).
-          is_less_than(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18).
+            is_less_than(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
     end
 
     context 'when qualifiers match the validation options but the values are different' do
-      it do
+      specify 'such as testing greater_than (+ only_integer) with lower value' do
         record = build_record_validating_numericality(
           only_integer: true,
           greater_than: 19
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got errors:
+            * "must be greater than 19" (attribute: attr, value: "19")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing greater_than (+ only_integer) with higher value' do
         record = build_record_validating_numericality(
           only_integer: true,
           greater_than: 17
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing greater_than (+ even) with lower value' do
         record = build_record_validating_numericality(
           even: true,
           greater_than: 20
         )
-        expect(record).
-          not_to validate_numericality.
-          even.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            even.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got errors:
+            * "must be greater than 20" (attribute: attr, value: "20")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing greater than (+ even) with higher value' do
         record = build_record_validating_numericality(
           even: true,
           greater_than: 16
         )
-        expect(record).
-          not_to validate_numericality.
-          even.
-          is_greater_than(18)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            even.
+            is_greater_than(18)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing less_than_or_equal_to (+ odd) with lower value' do
         record = build_record_validating_numericality(
           odd: true,
           less_than_or_equal_to: 101
         )
-        expect(record).
-          not_to validate_numericality.
-          odd.
-          is_less_than_or_equal_to(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            odd.
+            is_less_than_or_equal_to(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be less than or equal to 99" when attr is set to 101,
+            got no errors
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing less_than_or_equal_to (+ odd) with higher value' do
         record = build_record_validating_numericality(
           odd: true,
           less_than_or_equal_to: 97
         )
-        expect(record).
-          not_to validate_numericality.
-          odd.
-          is_less_than_or_equal_to(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            odd.
+            is_less_than_or_equal_to(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be less than or equal to 99" when attr is set to 101,
+            got errors:
+            * "must be less than or equal to 97" (attribute: attr, value: "101")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing greater_than (+ only_integer + less_than) with lower value' do
         record = build_record_validating_numericality(
           only_integer: true,
           greater_than: 19,
           less_than: 99
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18).
-          is_less_than(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18).
+            is_less_than(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be greater than 18" when attr is set to 18,
+            got errors:
+            * "must be greater than 19" (attribute: attr, value: "19")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
 
-      it do
+      specify 'such as testing less_than (+ only_integer + greater_than) with higher value' do
         record = build_record_validating_numericality(
           only_integer: true,
           greater_than: 18,
           less_than: 100
         )
-        expect(record).
-          not_to validate_numericality.
-          only_integer.
-          is_greater_than(18).
-          is_less_than(99)
+        assertion = lambda do
+          expect(record).
+            to validate_numericality.
+            only_integer.
+            is_greater_than(18).
+            is_less_than(99)
+        end
+        message = format_message_according_to_rails_version(
+          <<-MESSAGE.strip_heredoc
+            Expected errors to include "must be less than 99" when attr is set to 100,
+            got errors:
+            * "must be less than 100" (attribute: attr, value: "100")
+          MESSAGE
+        )
+        expect(&assertion).to fail_with_message(message)
       end
     end
   end
@@ -847,4 +981,12 @@ describe Shoulda::Matchers::ActiveModel::ValidateNumericalityOfMatcher, type: :m
   def build_record_with_integer_column_of_limit(limit, validation_options = {})
     define_model_with_integer_column_of_limit(limit, validation_options).new
   end
+
+  def format_message_according_to_rails_version(message)
+    if rails_gte_4_2?
+      message
+    else
+      message.gsub(/"((?:\d+)(?:\.\d+)?)"/, '\1')
+    end
+  end
 end