From ef80d2be42d140355bdc610445d388748e5ef4c1 Mon Sep 17 00:00:00 2001
From: Elliot Winkler <elliot.winkler@gmail.com>
Date: Thu, 24 Sep 2015 16:49:58 -0600
Subject: [PATCH] Rewrite the tests for ComparisonMatcher

Why:

* The tests for ComparisonMatcher don't actually test
  ComparisonMatcher -- they test the comparison qualifiers for
  NumericalityMatcher. Not only is this misleading, but it also creates
  a problem as `validate_numericality_of` is no longer available in any
  example group, but only ones that have been specially tagged.

To satisfy the above:

* Ensure that the tests build an instance of ComparisonMatcher and test
  against that.
* Fix style issues with the tests.
* Additionally, add a #description method to ComparisonMatcher while
  we're at it.
---
 .../comparison_matcher.rb                     |  29 +-
 .../comparison_matcher_spec.rb                | 275 ++++++++++--------
 2 files changed, 171 insertions(+), 133 deletions(-)

diff --git a/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb b/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb
index 61075910..90d442f3 100644
--- a/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb
+++ b/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb
@@ -24,24 +24,31 @@ module Shoulda
             @strict = false
           end
 
+          def description
+            message = "validate that #{@attribute} is #{comparison_expectation} #{@value}"
+
+            if @strict
+              message << " strictly"
+            end
+
+            message
+          end
+
           def for(attribute)
             @attribute = attribute
             self
           end
 
+          def with_message(message)
+            @message = message
+            self
+          end
+
           def matches?(subject)
             @subject = subject
             all_bounds_correct?
           end
 
-          def with_message(message)
-            @message = message
-          end
-
-          def comparison_description
-            "#{expectation} #{@value}"
-          end
-
           def failure_message
             last_failing_submatcher.failure_message
           end
@@ -50,6 +57,10 @@ module Shoulda
             last_failing_submatcher.failure_message_when_negated
           end
 
+          def comparison_description
+            "#{comparison_expectation} #{@value}"
+          end
+
           private
 
           def all_bounds_correct?
@@ -116,7 +127,7 @@ module Shoulda
             [-diff, 0, diff]
           end
 
-          def expectation
+          def comparison_expectation
             case @operator
               when :> then "greater than"
               when :>= then "greater than or equal to"
diff --git a/spec/unit/shoulda/matchers/active_model/numericality_matchers/comparison_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/numericality_matchers/comparison_matcher_spec.rb
index addb3c1c..5c9b0340 100644
--- a/spec/unit/shoulda/matchers/active_model/numericality_matchers/comparison_matcher_spec.rb
+++ b/spec/unit/shoulda/matchers/active_model/numericality_matchers/comparison_matcher_spec.rb
@@ -1,22 +1,19 @@
 require 'unit_spec_helper'
 
 describe Shoulda::Matchers::ActiveModel::NumericalityMatchers::ComparisonMatcher do
-  subject { described_class.new(matcher, 0, :>) }
-
-  it_behaves_like 'a numerical submatcher'
+  it_behaves_like 'a numerical submatcher' do
+    subject { build_matcher }
+  end
 
   shared_examples_for 'strict qualifier' do
-    def validation_qualifier
-      matcher_qualifier.to_s.gsub(/^is_/, '').to_sym
-    end
-
     context 'asserting strict validation when validating strictly' do
       it 'accepts' do
         record = instance_with_validations(
           validation_qualifier => 1,
           strict: true
         )
-        expect(record).to matcher.__send__(matcher_qualifier, 1).strict
+        matcher = build_matcher(operator: operator, value: 1).strict
+        expect(record).to matcher
       end
     end
 
@@ -27,209 +24,231 @@ describe Shoulda::Matchers::ActiveModel::NumericalityMatchers::ComparisonMatcher
           validation_qualifier => 1,
           strict: true
         )
-        expect(record).not_to matcher.__send__(matcher_qualifier, 1)
+        matcher = build_matcher(operator: operator, value: 1)
+        expect(record).not_to matcher
       end
     end
 
     context 'asserting strict validation when not validating strictly' do
       it 'rejects' do
         record = instance_with_validations(validation_qualifier => 1)
-        expect(record).not_to matcher.__send__(matcher_qualifier, 1).strict
+        matcher = build_matcher(operator: operator, value: 1).strict
+        expect(record).not_to matcher
       end
     end
   end
 
   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
+    it 'raises an ArgumentError' do
+      numericality_matcher = double
+      expect { described_class.new(numericality_matcher, 0, :>) }.
+        to raise_error(ArgumentError)
     end
   end
 
-  context 'is_greater_than' do
-    include_examples 'strict qualifier' do
-      def matcher_qualifier
-        :is_greater_than
-      end
+  describe 'is_greater_than' do
+    include_examples 'strict qualifier'
+
+    it do
+      record = instance_with_validations(greater_than: 1.5)
+      matcher = build_matcher(operator: :>, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than: 2))
-        .to matcher.is_greater_than(2)
+      record = instance_with_validations(greater_than: 2)
+      matcher = build_matcher(operator: :>, value: 2)
+      expect(record).to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than: 1.5))
-        .not_to matcher.is_greater_than(2)
+      record = instance_with_validations(greater_than: 2.5)
+      matcher = build_matcher(operator: :>, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than: 2.5))
-        .not_to matcher.is_greater_than(2)
+      record = instance_without_validations
+      matcher = build_matcher(operator: :>, value: 2)
+      expect(record).not_to matcher
     end
 
-    it do
-      expect(instance_without_validations).not_to matcher.is_greater_than(2)
+    def operator
+      :>
+    end
+
+    def validation_qualifier
+      :greater_than
     end
   end
 
-  context 'is_greater_than_or_equal_to' do
-    include_examples 'strict qualifier' do
-      def matcher_qualifier
-        :is_greater_than_or_equal_to
-      end
+  describe 'is_greater_than_or_equal_to' do
+    include_examples 'strict qualifier'
+
+    it do
+      record = instance_with_validations(greater_than_or_equal_to: 1.5)
+      matcher = build_matcher(operator: :>=, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than_or_equal_to: 2))
-        .to matcher.is_greater_than_or_equal_to(2)
+      record = instance_with_validations(greater_than_or_equal_to: 2)
+      matcher = build_matcher(operator: :>=, value: 2)
+      expect(record).to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than_or_equal_to: 1.5))
-        .not_to matcher.is_greater_than_or_equal_to(2)
+      record = instance_with_validations(greater_than_or_equal_to: 2.5)
+      matcher = build_matcher(operator: :>=, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(greater_than_or_equal_to: 2.5))
-        .not_to matcher.is_greater_than_or_equal_to(2)
+      record = instance_without_validations
+      matcher = build_matcher(operator: :>=, value: 2)
+      expect(record).not_to matcher
     end
 
-    it do
-      expect(instance_without_validations)
-        .not_to matcher.is_greater_than_or_equal_to(2)
+    def operator
+      :>=
+    end
+
+    def validation_qualifier
+      :greater_than_or_equal_to
     end
   end
 
-  context 'is_less_than' do
-    include_examples 'strict qualifier' do
-      def matcher_qualifier
-        :is_less_than
-      end
+  describe 'is_less_than' do
+    include_examples 'strict qualifier'
+
+    it do
+      record = instance_with_validations(less_than: 1.5)
+      matcher = build_matcher(operator: :<, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than: 2))
-        .to matcher.is_less_than(2)
+      record = instance_with_validations(less_than: 2)
+      matcher = build_matcher(operator: :<, value: 2)
+      expect(record).to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than: 1.5))
-        .not_to matcher.is_less_than(2)
+      record = instance_with_validations(less_than: 2.5)
+      matcher = build_matcher(operator: :<, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than: 2.5))
-        .not_to matcher.is_less_than(2)
+      record = instance_without_validations
+      matcher = build_matcher(operator: :<, value: 2)
+      expect(record).not_to matcher
     end
 
-    it do
-      expect(instance_without_validations)
-        .not_to matcher.is_less_than(2)
+    def operator
+      :<
+    end
+
+    def validation_qualifier
+      :less_than
     end
   end
 
-  context 'is_less_than_or_equal_to' do
-    include_examples 'strict qualifier' do
-      def matcher_qualifier
-        :is_less_than_or_equal_to
-      end
+  describe 'is_less_than_or_equal_to' do
+    include_examples 'strict qualifier'
+
+    it do
+      record = instance_with_validations(less_than_or_equal_to: 1.5)
+      matcher = build_matcher(operator: :<=, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than_or_equal_to: 2))
-        .to matcher.is_less_than_or_equal_to(2)
+      record = instance_with_validations(less_than_or_equal_to: 2)
+      matcher = build_matcher(operator: :<=, value: 2)
+      expect(record).to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than_or_equal_to: 1.5))
-        .not_to matcher.is_less_than_or_equal_to(2)
+      record = instance_with_validations(less_than_or_equal_to: 2.5)
+      matcher = build_matcher(operator: :<=, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(less_than_or_equal_to: 2.5))
-        .not_to matcher.is_less_than_or_equal_to(2)
+      record = instance_without_validations
+      matcher = build_matcher(operator: :<=, value: 2)
+      expect(record).not_to matcher
     end
 
-    it do
-      expect(instance_without_validations)
-        .not_to matcher.is_less_than_or_equal_to(2)
+    def operator
+      :<=
+    end
+
+    def validation_qualifier
+      :less_than_or_equal_to
     end
   end
 
-  context 'is_equal_to' do
-    include_examples 'strict qualifier' do
-      def matcher_qualifier
-        :is_equal_to
-      end
+  describe 'is_equal_to' do
+    include_examples 'strict qualifier'
+
+    it do
+      record = instance_with_validations(equal_to: 1.5)
+      matcher = build_matcher(operator: :==, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(equal_to: 0))
-        .to matcher.is_equal_to(0)
+      record = instance_with_validations(equal_to: 2)
+      matcher = build_matcher(operator: :==, value: 2)
+      expect(record).to matcher
     end
 
     it do
-      expect(instance_with_validations(equal_to: -0.5))
-        .not_to matcher.is_equal_to(0)
+      record = instance_with_validations(equal_to: 2.5)
+      matcher = build_matcher(operator: :==, value: 2)
+      expect(record).not_to matcher
     end
 
     it do
-      expect(instance_with_validations(equal_to: 0.5))
-        .not_to matcher.is_equal_to(0)
+      record = instance_without_validations
+      matcher = build_matcher(operator: :==, value: 2)
+      expect(record).not_to matcher
     end
 
-    it do
-      expect(instance_without_validations)
-        .not_to matcher.is_equal_to(0)
+    def operator
+      :==
+    end
+
+    def validation_qualifier
+      :equal_to
     end
   end
 
-  context 'with_message' do
+  describe 'with_message' do
     it 'verifies the message for the validation' do
       instance = instance_with_validations(equal_to: 0, message: 'Must be zero')
-      expect(instance).to matcher.is_equal_to(0).with_message('Must be zero')
-    end
-  end
-
-  context 'qualified with on and validating with on' do
-    it 'accepts' do
-      expect(instance_with_validations(on: :customizable)).
-        to matcher.on(:customizable)
-    end
-  end
-
-  context 'qualified with on but not validating with on' do
-    it 'accepts since the validation never considers a context' do
-      expect(instance_with_validations).to matcher.on(:customizable)
-    end
-  end
-
-  context 'not qualified with on but validating with on' do
-    it 'rejects since the validation never runs' do
-      expect(instance_with_validations(on: :customizable)).
-        not_to matcher
+      matcher = build_matcher.with_message('Must be zero')
+      expect(instance).to matcher
     end
   end
 
   describe '#comparison_description' do
-    [{ operator: :>, value: 0, expectation: 'greater than 0' },
-     { operator: :>=, value: -1.0, expectation: 'greater than or equal to -1.0' },
-     { operator: :==, value: 2.2, expectation: 'equal to 2.2' },
-     { operator: :<, value: -3, expectation: 'less than -3' },
-     { 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 do
-          described_class.new(matcher, h[:value], h[:operator])
-            .comparison_description
+    tests = [
+      { operator: :>, value: 0, expectation: 'greater than 0' },
+      { operator: :>=, value: -1.0, expectation: 'greater than or equal to -1.0' },
+      { operator: :==, value: 2.2, expectation: 'equal to 2.2' },
+      { operator: :<, value: -3, expectation: 'less than -3' },
+      { operator: :<=, value: 4, expectation: 'less than or equal to 4' },
+    ]
+
+    tests.each do |test|
+      context "with :#{test[:operator]} as operator and #{test[:value]} as value" do
+        it do
+          matcher = build_matcher(operator: test[:operator], value: test[:value])
+          expect(matcher.comparison_description).to eq test[:expectation]
         end
-        it { should eq h[:expectation] }
       end
     end
   end
@@ -245,17 +264,25 @@ describe Shoulda::Matchers::ActiveModel::NumericalityMatchers::ComparisonMatcher
     model_with_validations(options).new(attribute_name => '1')
   end
 
-  def instance_without_validations
+  def model_without_validations
     define_model :example, attribute_name => :string do |model|
       model.attr_accessible(attribute_name)
-    end.new
+    end
+  end
+
+  def instance_without_validations
+    model_without_validations.new
   end
 
   def attribute_name
     :attr
   end
 
-  def matcher
-    validate_numericality_of(:attr)
+  def build_matcher(operator: :==, value: 0)
+    described_class.new(numericality_matcher, value, operator).for(attribute_name)
+  end
+
+  def numericality_matcher
+    double(diff_to_compare: 1)
   end
 end