Skip to content
Browse files

Update numericality comarison matcher more accurate

  • Loading branch information...
1 parent ce586c5 commit bc110f73b91db395467b91b222d76bc667a386f6 @untidy-hair untidy-hair committed Feb 26, 2014
View
4 NEWS.md
@@ -33,6 +33,10 @@
* Add `allow_nil` option to `validate_numericality_of` so that you can validate
that numeric values are validated only if a value is supplied.
+* Fix `validate_numericality_of` so that test fails when the value with
+ `greater_than`, `greater_than_or_equal_to`, `less_than`, `less_than_or_equal_
+ to` or `equal_to` is not appropriate.
+
# v 2.5.0
* Fix Rails/Test::Unit integration to ensure that the test case classes we are
View
39 lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb
@@ -7,7 +7,11 @@ module NumericalityMatchers
# is_greater_than(6).
# less_than(20)...(and so on) }
class ComparisonMatcher < ValidationMatcher
- def initialize(value, operator)
+ def initialize(numericality_matcher, value, operator)
+ unless numericality_matcher.respond_to? :diff_to_compare
+ raise ArgumentError, 'numericality_matcher is invalid'
+ end
+ @numericality_matcher = numericality_matcher
@value = value
@operator = operator
@message = nil
@@ -20,7 +24,7 @@ def for(attribute)
def matches?(subject)
@subject = subject
- disallows_value_of(value_to_compare, @message)
+ all_bounds_correct?
end
def with_message(message)
@@ -33,14 +37,23 @@ def comparison_description
private
- def value_to_compare
- case @operator
- when :> then [@value, @value - 1].sample
- when :>= then @value - 1
- when :== then @value + 1
- when :< then [@value, @value + 1].sample
- when :<= then @value + 1
- end
+ def comparison_combos
+ allow = :allows_value_of
+ disallow = :disallows_value_of
+ checker_types =
+ case @operator
+ when :> then [allow, disallow, disallow]
+ when :>= then [allow, allow, disallow]
+ when :== then [disallow, allow, disallow]
+ when :< then [disallow, disallow, allow]
+ when :<= then [disallow, allow, allow]
+ end
+ diffs_to_compare.zip(checker_types)
+ end
+
+ def diffs_to_compare
+ diff = @numericality_matcher.diff_to_compare
+ [diff, 0, -diff]
end
def expectation
@@ -52,6 +65,12 @@ def expectation
when :<= then "less than or equal to"
end
end
+
+ def all_bounds_correct?
+ comparison_combos.all? do |diff, checker_type|
+ __send__(checker_type, @value + diff, @message)
+ end
+ end
end
end
end
View
4 lib/shoulda/matchers/active_model/numericality_matchers/even_number_matcher.rb
@@ -15,6 +15,10 @@ def initialize(attribute, options = {})
def allowed_type
'even numbers'
end
+
+ def diff_to_compare
+ 2
+ end
end
end
end
View
4 lib/shoulda/matchers/active_model/numericality_matchers/numeric_type_matcher.rb
@@ -20,6 +20,10 @@ def allowed_type
raise NotImplementedError
end
+ def diff_to_compare
+ raise NotImplementedError
+ end
+
def failure_message
@disallow_value_matcher.failure_message
end
View
4 lib/shoulda/matchers/active_model/numericality_matchers/odd_number_matcher.rb
@@ -15,6 +15,10 @@ def initialize(attribute, options = {})
def allowed_type
'odd numbers'
end
+
+ def diff_to_compare
+ 2
+ end
end
end
end
View
4 lib/shoulda/matchers/active_model/numericality_matchers/only_integer_matcher.rb
@@ -14,6 +14,10 @@ def initialize(attribute)
def allowed_type
'integers'
end
+
+ def diff_to_compare
+ 1
+ end
end
end
end
View
72 lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb
@@ -26,58 +26,68 @@ def validate_numericality_of(attr)
class ValidateNumericalityOfMatcher
NUMERIC_NAME = 'numbers'
NON_NUMERIC_VALUE = 'abcd'
+ DEFAULT_DIFF_TO_COMPARE = 0.000_000_000_001
+ attr_reader :diff_to_compare
def initialize(attribute)
@attribute = attribute
@submatchers = []
-
+ @diff_to_compare = DEFAULT_DIFF_TO_COMPARE
add_disallow_value_matcher
end
def only_integer
- add_submatcher(NumericalityMatchers::OnlyIntegerMatcher.new(@attribute))
+ prepare_submatcher(
+ NumericalityMatchers::OnlyIntegerMatcher.new(@attribute)
+ )
self
end
def allow_nil
- add_submatcher(AllowValueMatcher.new(nil).for(@attribute).with_message(:not_a_number))
+ prepare_submatcher(
+ AllowValueMatcher.new(nil)
+ .for(@attribute)
+ .with_message(:not_a_number)
+ )
self
end
- def is_greater_than(value)
- add_submatcher(NumericalityMatchers::ComparisonMatcher.new(value, :>).for(@attribute))
+ def odd
+ prepare_submatcher(
+ NumericalityMatchers::OddNumberMatcher.new(@attribute)
+ )
self
end
- def is_greater_than_or_equal_to(value)
- add_submatcher(NumericalityMatchers::ComparisonMatcher.new(value, :>=).for(@attribute))
+ def even
+ prepare_submatcher(
+ NumericalityMatchers::EvenNumberMatcher.new(@attribute)
+ )
self
end
- def is_equal_to(value)
- add_submatcher(NumericalityMatchers::ComparisonMatcher.new(value, :==).for(@attribute))
+ def is_greater_than(value)
+ prepare_submatcher(comparison_matcher_for(value, :>).for(@attribute))
self
end
- def is_less_than(value)
- add_submatcher(NumericalityMatchers::ComparisonMatcher.new(value, :<).for(@attribute))
+ def is_greater_than_or_equal_to(value)
+ prepare_submatcher(comparison_matcher_for(value, :>=).for(@attribute))
self
end
- def is_less_than_or_equal_to(value)
- add_submatcher(NumericalityMatchers::ComparisonMatcher.new(value, :<=).for(@attribute))
+ def is_equal_to(value)
+ prepare_submatcher(comparison_matcher_for(value, :==).for(@attribute))
self
end
- def odd
- odd_number_matcher = NumericalityMatchers::OddNumberMatcher.new(@attribute)
- add_submatcher(odd_number_matcher)
+ def is_less_than(value)
+ prepare_submatcher(comparison_matcher_for(value, :<).for(@attribute))
self
end
- def even
- even_number_matcher = NumericalityMatchers::EvenNumberMatcher.new(@attribute)
- add_submatcher(even_number_matcher)
+ def is_less_than_or_equal_to(value)
+ prepare_submatcher(comparison_matcher_for(value, :<=).for(@attribute))
self
end
@@ -115,10 +125,27 @@ def add_disallow_value_matcher
add_submatcher(disallow_value_matcher)
end
+ def prepare_submatcher(submatcher)
+ add_submatcher(submatcher)
+ if submatcher.respond_to?(:diff_to_compare)
+ update_diff_to_compare(submatcher)
+ end
+ end
+
+ def comparison_matcher_for(value, operator)
+ NumericalityMatchers::ComparisonMatcher
+ .new(self, value, operator)
+ .for(@attribute)
+ end
+
def add_submatcher(submatcher)
@submatchers << submatcher
end
+ def update_diff_to_compare(matcher)
+ @diff_to_compare = [@diff_to_compare, matcher.diff_to_compare].max
+ end
+
def submatchers_match?
failing_submatchers.empty?
end
@@ -150,7 +177,12 @@ def comparison_descriptions
end
def submatcher_comparison_descriptions
- @submatchers.inject([]){|m, s| m << s.comparison_description if s.respond_to?(:comparison_description); m }
+ @submatchers.inject([]) do |arr, submatcher|
+ if submatcher.respond_to? :comparison_description
+ arr << submatcher.comparison_description
+ end
+ arr
+ end
end
end
end
View
125 spec/shoulda/matchers/active_model/numericality_matchers/comparison_matcher_spec.rb
@@ -1,33 +1,129 @@
require 'spec_helper'
describe Shoulda::Matchers::ActiveModel::NumericalityMatchers::ComparisonMatcher do
- it_behaves_like 'a numerical submatcher' do
- subject { described_class.new(0, :>) }
+ subject { described_class.new(matcher, 0, :>) }
+
+ it_behaves_like 'a numerical submatcher'
+
+ context 'when initialized without correct numerical matcher' do
+ it 'raises an argument error' do
+ fake_matcher = matcher
+ class << fake_matcher
+ undef_method :diff_to_compare
+ end
+ expect do
+ described_class.new(fake_matcher, 0, :>)
+ end.to raise_error ArgumentError
+ end
end
context 'is_greater_than' do
- it { expect(instance_with_validations(greater_than: 2)).to matcher.is_greater_than(2) }
- it { expect(instance_without_validations).not_to matcher.is_greater_than(2) }
+ it do
+ expect(instance_with_validations(greater_than: 2))
+ .to matcher.is_greater_than(2)
+ end
+
+ it do
+ expect(instance_with_validations(greater_than: 1.5))
+ .not_to matcher.is_greater_than(2)
+ end
+
+ it do
+ expect(instance_with_validations(greater_than: 2.5))
+ .not_to matcher.is_greater_than(2)
+ end
+
+ it do
+ expect(instance_without_validations).not_to matcher.is_greater_than(2)
+ end
end
context 'greater_than_or_equal_to' do
- it { expect(instance_with_validations(greater_than_or_equal_to: 2)).to matcher.is_greater_than_or_equal_to(2) }
- it { expect(instance_without_validations).not_to matcher.is_greater_than_or_equal_to(2) }
+ it do
+ expect(instance_with_validations(greater_than_or_equal_to: 2))
+ .to matcher.is_greater_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_with_validations(greater_than_or_equal_to: 1.5))
+ .not_to matcher.is_greater_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_with_validations(greater_than_or_equal_to: 2.5))
+ .not_to matcher.is_greater_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_without_validations)
+ .not_to matcher.is_greater_than_or_equal_to(2)
+ end
end
context 'less_than' do
- it { expect(instance_with_validations(less_than: 2)).to matcher.is_less_than(2) }
- it { expect(instance_without_validations).not_to matcher.is_less_than(2) }
+ it do
+ expect(instance_with_validations(less_than: 2))
+ .to matcher.is_less_than(2)
+ end
+
+ it do
+ expect(instance_with_validations(less_than: 1.5))
+ .not_to matcher.is_less_than(2)
+ end
+
+ it do
+ expect(instance_with_validations(less_than: 2.5))
+ .not_to matcher.is_less_than(2)
+ end
+
+ it do
+ expect(instance_without_validations)
+ .not_to matcher.is_less_than(2)
+ end
end
context 'less_than_or_equal_to' do
- it { expect(instance_with_validations(less_than_or_equal_to: 2)).to matcher.is_less_than_or_equal_to(2) }
- it { expect(instance_without_validations).not_to matcher.is_less_than_or_equal_to(2) }
+ it do
+ expect(instance_with_validations(less_than_or_equal_to: 2))
+ .to matcher.is_less_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_with_validations(less_than_or_equal_to: 1.5))
+ .not_to matcher.is_less_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_with_validations(less_than_or_equal_to: 2.5))
+ .not_to matcher.is_less_than_or_equal_to(2)
+ end
+
+ it do
+ expect(instance_without_validations)
+ .not_to matcher.is_less_than_or_equal_to(2)
+ end
end
context 'is_equal_to' do
- it { expect(instance_with_validations(equal_to: 0)).to matcher.is_equal_to(0) }
- it { expect(instance_without_validations).not_to matcher.is_equal_to(0) }
+ it do
+ expect(instance_with_validations(equal_to: 0))
+ .to matcher.is_equal_to(0)
+ end
+
+ it do
+ expect(instance_with_validations(equal_to: -0.5))
+ .not_to matcher.is_equal_to(0)
+ end
+
+ it do
+ expect(instance_with_validations(equal_to: 0.5))
+ .not_to matcher.is_equal_to(0)
+ end
+
+ it do
+ expect(instance_without_validations)
+ .not_to matcher.is_equal_to(0)
+ end
end
context 'with_message' do
@@ -45,7 +141,10 @@
{ operator: :<=, value: 4, expectation: 'less than or equal to 4' },
].each do |h|
context "with :#{h[:operator]} as operator and #{h[:value]} as value" do
- subject { described_class.new(h[:value], h[:operator]).comparison_description }
+ subject do
+ described_class.new(matcher, h[:value], h[:operator])
+ .comparison_description
+ end
it { should eq h[:expectation] }
end
end
View
4 spec/shoulda/matchers/active_model/numericality_matchers/even_number_matcher_spec.rb
@@ -10,6 +10,10 @@
expect(subject.allowed_type).to eq 'even numbers'
end
+ describe '#diff_to_compare' do
+ it { expect(subject.diff_to_compare).to eq 2 }
+ end
+
context 'when the model has an even validation' do
it 'matches' do
match = subject
View
4 spec/shoulda/matchers/active_model/numericality_matchers/odd_number_matcher_spec.rb
@@ -10,6 +10,10 @@
expect(subject.allowed_type).to eq 'odd numbers'
end
+ describe '#diff_to_compare' do
+ it { expect(subject.diff_to_compare).to eq 2 }
+ end
+
context 'when the model has an odd validation' do
it 'matches' do
match = subject
View
4 spec/shoulda/matchers/active_model/numericality_matchers/only_integer_matcher_spec.rb
@@ -10,6 +10,10 @@
expect(subject.allowed_type).to eq 'integers'
end
+ describe '#diff_to_compare' do
+ it { expect(subject.diff_to_compare).to eq 1 }
+ end
+
context 'given an attribute that only allows integer values' do
it 'matches' do
match = subject
View
184 spec/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb
@@ -22,28 +22,35 @@
the_matcher.matches?(define_model(:example, attr: :string).new)
- expect(the_matcher.failure_message_when_negated).to include 'Did not expect errors to include "is not a number"'
+ expect(the_matcher.failure_message_when_negated)
+ .to include 'Did not expect errors to include "is not a number"'
end
it 'rejects with the ActiveRecord :not_an_integer message' do
the_matcher = matcher.only_integer
- expect {
+ expect do
expect(not_validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be an integer"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be an integer"'
+ )
end
it 'rejects with the ActiveRecord :odd message' do
the_matcher = matcher.odd
- expect {
+ expect do
expect(not_validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be odd"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be odd"'
+ )
end
it 'rejects with the ActiveRecord :even message' do
the_matcher = matcher.even
- expect {
+ expect do
expect(not_validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be even"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be even"'
+ )
end
end
@@ -71,9 +78,11 @@
it 'rejects with the ActiveRecord :not_an_integer message' do
the_matcher = matcher.only_integer
- expect {
+ expect do
expect(validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be an integer"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be an integer"'
+ )
end
end
@@ -88,9 +97,11 @@
it 'rejects with the ActiveRecord :odd message' do
the_matcher = matcher.odd
- expect {
+ expect do
expect(validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be odd"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be odd"'
+ )
end
end
@@ -105,9 +116,123 @@
it 'rejects with the ActiveRecord :even message' do
the_matcher = matcher.even
- expect {
+ expect do
expect(validating_numericality).to the_matcher
- }.to fail_with_message_including('Expected errors to include "must be even"')
+ end.to fail_with_message_including(
+ 'Expected errors to include "must be even"'
+ )
+ end
+ end
+
+ context 'with multiple options together' do
+ context 'the success cases' do
+ it do
+ expect(validating_numericality(only_integer: true, greater_than: 18))
+ .to matcher.only_integer.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(even: true, greater_than: 18))
+ .to matcher.even.is_greater_than(18)
+ end
+ it do
+ expect(validating_numericality(odd: true, less_than_or_equal_to: 99))
+ .to matcher.odd.is_less_than_or_equal_to(99)
+ end
+
+ it do
+ expect(validating_numericality(
+ only_integer: true,
+ greater_than: 18,
+ less_than: 99)
+ ).to matcher.only_integer.is_greater_than(18).is_less_than(99)
+ end
+ end
+
+ context 'the failure cases with different validators' do
+ it do
+ expect(validating_numericality(even: true, greater_than: 18))
+ .not_to matcher.only_integer.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(greater_than: 18))
+ .not_to matcher.only_integer.is_greater_than(18)
+ end
+
+ it do
+ expect(
+ validating_numericality(even: true, greater_than_or_equal_to: 18)
+ ).not_to matcher.even.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(odd: true, greater_than: 18))
+ .not_to matcher.even.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(
+ odd: true,
+ greater_than_or_equal_to: 99
+ )
+ ).not_to matcher.odd.is_less_than_or_equal_to(99)
+ end
+
+ it do
+ expect(validating_numericality(
+ only_integer: true,
+ greater_than_or_equal_to: 18,
+ less_than: 99
+ )
+ ).not_to matcher.only_integer.is_greater_than(18).is_less_than(99)
+ end
+ end
+
+ context 'the failure cases with wrong values' do
+ it do
+ expect(validating_numericality(only_integer: true, greater_than: 19))
+ .not_to matcher.only_integer.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(only_integer: true, greater_than: 17))
+ .not_to matcher.only_integer.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(even: true, greater_than: 20))
+ .not_to matcher.even.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(even: true, greater_than: 16))
+ .not_to matcher.even.is_greater_than(18)
+ end
+
+ it do
+ expect(validating_numericality(odd: true, less_than_or_equal_to: 101))
+ .not_to matcher.odd.is_less_than_or_equal_to(99)
+ end
+
+ it do
+ expect(validating_numericality(odd: true, less_than_or_equal_to: 97))
+ .not_to matcher.odd.is_less_than_or_equal_to(99)
+ end
+
+ it do
+ expect(validating_numericality(only_integer: true,
+ greater_than: 19,
+ less_than: 99))
+ .not_to matcher.only_integer.is_greater_than(18).is_less_than(99)
+ end
+
+ it do
+ expect(validating_numericality(only_integer: true,
+ greater_than: 18,
+ less_than: 100))
+ .not_to matcher.only_integer.is_greater_than(18).is_less_than(99)
+ end
end
end
@@ -142,26 +267,45 @@ def set_attr!; self.attr = 5 end
end
context 'with only integer option' do
- it { expect(matcher.only_integer.description).to eq 'only allow integers for attr' }
+ it do
+ expect(matcher.only_integer.description)
+ .to eq 'only allow integers for attr'
+ end
end
[:odd, :even].each do |type|
context "with #{type} option" do
- it { expect(matcher.__send__(type).description).to eq "only allow #{type} numbers for attr" }
+ it do
+ expect(matcher.__send__(type).description)
+ .to eq "only allow #{type} numbers for attr"
+ end
end
end
- [:is_greater_than, :is_greater_than_or_equal_to, :is_less_than, :is_less_than_or_equal_to,
+ [:is_greater_than,
+ :is_greater_than_or_equal_to,
+ :is_less_than,
+ :is_less_than_or_equal_to,
:is_equal_to ].each do |comparison|
context "with #{comparison} option" do
- it { expect(matcher.__send__(comparison, 18).description).
- to eq "only allow numbers for attr which are #{comparison.to_s.sub('is_','').gsub('_', ' ')} 18" }
+ it do
+ expect(matcher.__send__(comparison, 18).description)
+ .to eq(
+ 'only allow numbers for attr which are ' +
+ "#{comparison.to_s.sub('is_', '').gsub('_', ' ')} 18"
+ )
+ end
end
end
context 'with odd, is_greater_than_or_equal_to option' do
- it { expect(matcher.odd.is_greater_than_or_equal_to(18).description).
- to eq "only allow odd numbers for attr which are greater than or equal to 18" }
+ it do
+ expect(matcher.odd.is_greater_than_or_equal_to(18).description)
+ .to eq(
+ 'only allow odd numbers for attr ' +
+ 'which are greater than or equal to 18'
+ )
+ end
end
context 'with only integer, is_greater_than and less_than_or_equal_to option' do
View
7 spec/support/shared_examples/numerical_type_submatcher.rb
@@ -3,7 +3,14 @@
shared_examples 'a numerical type submatcher' do
it 'implements the allowed_type method' do
expect(subject).to respond_to(:allowed_type).with(0).arguments
+ expect { subject.allowed_type }.not_to raise_error
end
+
+ it 'implements the diff_to_compare' do
+ expect(subject).to respond_to(:diff_to_compare).with(0).arguments
+ expect { subject.diff_to_compare }.not_to raise_error
+ end
+
it 'returns itself when given a message' do
expect(subject.with_message('some message')).to eq subject
end

0 comments on commit bc110f7

Please sign in to comment.
Something went wrong with that request. Please try again.