From e7087897143dfd5f59e3c512c31a8c7976ad96dd Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 9 Apr 2015 11:27:26 -0300 Subject: [PATCH] Fix failure message for numericality matcher Secondary author: Mauro George 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 | 4 + .../validate_numericality_of_matcher.rb | 25 +- spec/support/unit/helpers/rails_versions.rb | 4 + .../matchers/fail_with_message_matcher.rb | 2 +- .../validate_numericality_of_matcher_spec.rb | 288 +++++++++++++----- 5 files changed, 231 insertions(+), 92 deletions(-) diff --git a/NEWS.md b/NEWS.md index 289288ba..81a3be53 100644 --- a/NEWS.md +++ b/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 diff --git a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb index 571a7ff9..1aeeaf1e 100644 --- a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb +++ b/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 diff --git a/spec/support/unit/helpers/rails_versions.rb b/spec/support/unit/helpers/rails_versions.rb index 38d6d02d..7df93f65 100644 --- a/spec/support/unit/helpers/rails_versions.rb +++ b/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 diff --git a/spec/support/unit/matchers/fail_with_message_matcher.rb b/spec/support/unit/matchers/fail_with_message_matcher.rb index 2f7aff32..1efabdc2 100644 --- a/spec/support/unit/matchers/fail_with_message_matcher.rb +++ b/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 diff --git a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb index 8e4c9549..2bd2df3f 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb +++ b/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