From e906b16afa22fb2759cc3aff0492e65450774cda Mon Sep 17 00:00:00 2001 From: Frank Luithle Date: Sun, 5 Sep 2010 17:23:09 +0200 Subject: [PATCH 1/3] optimize AllowValueMatcher slightly --- .../active_record/matchers/allow_value_matcher.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/shoulda/active_record/matchers/allow_value_matcher.rb b/lib/shoulda/active_record/matchers/allow_value_matcher.rb index bfdc4f757..846cd3e73 100644 --- a/lib/shoulda/active_record/matchers/allow_value_matcher.rb +++ b/lib/shoulda/active_record/matchers/allow_value_matcher.rb @@ -58,10 +58,14 @@ def description private def errors_match? - @instance.valid? - @errors = errors_for_attribute(@instance, @attribute) - @errors = [@errors] unless @errors.is_a?(Array) - @expected_message ? (errors_match_regexp? || errors_match_string?) : (@errors.compact.any?) + if ! @instance.valid? + @errors = errors_for_attribute(@instance, @attribute) + @errors = [@errors] unless @errors.is_a?(Array) + @expected_message ? (errors_match_regexp? || errors_match_string?) : (@errors.compact.any?) + else + @errors = [] + false + end end def errors_for_attribute(instance, attribute) From f0fdeb1f1982a6c6b02b95f9764905e8caa81e7c Mon Sep 17 00:00:00 2001 From: Frank Luithle Date: Sun, 5 Sep 2010 17:24:55 +0200 Subject: [PATCH 2/3] remove a redundancy inside AllowValueMatcher --- lib/shoulda/active_record/matchers/allow_value_matcher.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/shoulda/active_record/matchers/allow_value_matcher.rb b/lib/shoulda/active_record/matchers/allow_value_matcher.rb index 846cd3e73..f8fb8b70e 100644 --- a/lib/shoulda/active_record/matchers/allow_value_matcher.rb +++ b/lib/shoulda/active_record/matchers/allow_value_matcher.rb @@ -79,7 +79,6 @@ def errors_for_attribute(instance, attribute) def errors_match_regexp? if Regexp === @expected_message @matched_error = @errors.detect { |e| e =~ @expected_message } - !@matched_error.nil? else false end From a08b3bf79acf30391830ab55b471f1c36c13180c Mon Sep 17 00:00:00 2001 From: Frank Luithle Date: Sun, 5 Sep 2010 17:33:32 +0200 Subject: [PATCH 3/3] allow multiple values for AllowValueMatcher With the deprecation of "should_allow_values_for" there is currently no clean way to specify multiple allowed (or forbidden) values for an attribute in a single assertion. This can make test cases unnecessarily verbose. This patch gives AllowValueMatcher the ability to take multiple values. The new matcher is fully downward compatible (i.e. given a single argument it behaves identically to the old matcher). This matcher will stop checking for bad values when the first bad value is encountered. --- .../matchers/allow_value_matcher.rb | 29 ++++++++++---- test/README | 1 + .../active_record/allow_value_matcher_test.rb | 38 +++++++++++++++++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/lib/shoulda/active_record/matchers/allow_value_matcher.rb b/lib/shoulda/active_record/matchers/allow_value_matcher.rb index f8fb8b70e..d483b04f5 100644 --- a/lib/shoulda/active_record/matchers/allow_value_matcher.rb +++ b/lib/shoulda/active_record/matchers/allow_value_matcher.rb @@ -2,7 +2,10 @@ module Shoulda # :nodoc: module ActiveRecord # :nodoc: module Matchers - # Ensures that the attribute can be set to the given value. + # Ensures that the attribute can be set to the given value or values. If + # multiple values are given the match succeeds only if all given values + # are allowed. Otherwise, the matcher fails at the first bad value in the + # argument list (the remaining arguments are not processed then). # # Options: # * with_message - value the test expects to find in @@ -13,15 +16,16 @@ module Matchers # it { should_not allow_value('bad').for(:isbn) } # it { should allow_value("isbn 1 2345 6789 0").for(:isbn) } # - def allow_value(value) - AllowValueMatcher.new(value) + def allow_value(*values) + raise ArgumentError.new("need at least one argument") if values.empty? + AllowValueMatcher.new(*values) end class AllowValueMatcher # :nodoc: include Helpers - def initialize(value) - @value = value + def initialize(*values) + @values_to_match = values end def for(attribute) @@ -39,8 +43,12 @@ def matches?(instance) if Symbol === @expected_message @expected_message = default_error_message(@expected_message) end - @instance.send("#{@attribute}=", @value) - !errors_match? + @values_to_match.each do |value| + @value = value + @instance.send("#{@attribute}=", @value) + return false if errors_match? + end + true end def failure_message @@ -52,7 +60,12 @@ def negative_failure_message end def description - "allow #{@attribute} to be set to #{@value.inspect}" + "allow #{@attribute} to be set to " << + if @values_to_match.length > 1 + "any of [#{@values_to_match.map {|v| v.inspect }.join(', ')}]" + else + @values_to_match.first.inspect + end end private diff --git a/test/README b/test/README index 12a996c7b..feca3ce14 100644 --- a/test/README +++ b/test/README @@ -30,6 +30,7 @@ Dependencies: * Rails gem installed in the host system * A working sqlite3 installation. +* Cucumber gem if you want to run the Rails acceptance tests If you have problems running these tests, please notify the mailing list: shoulda@googlegroups.com diff --git a/test/matchers/active_record/allow_value_matcher_test.rb b/test/matchers/active_record/allow_value_matcher_test.rb index beeb40194..7ad088c17 100644 --- a/test/matchers/active_record/allow_value_matcher_test.rb +++ b/test/matchers/active_record/allow_value_matcher_test.rb @@ -13,10 +13,18 @@ class AllowValueMatcherTest < ActiveSupport::TestCase # :nodoc: should "allow a good value" do assert_accepts allow_value("abcde").for(:attr), @model end - + + should "allow several good values" do + assert_accepts allow_value("abcde", "deabc").for(:attr), @model + end + should "not allow a bad value" do assert_rejects allow_value("xyz").for(:attr), @model end + + should "not allow several bad values" do + assert_rejects allow_value("xyz", "zyx", nil, []).for(:attr), @model + end end context "an attribute with a format validation and a custom message" do @@ -31,7 +39,7 @@ class AllowValueMatcherTest < ActiveSupport::TestCase # :nodoc: assert_accepts allow_value('abcde').for(:attr).with_message(/bad/), @model end - + should "not allow a bad value" do assert_rejects allow_value('xyz').for(:attr).with_message(/bad/), @model @@ -52,13 +60,35 @@ class AllowValueMatcherTest < ActiveSupport::TestCase # :nodoc: should "allow a good value" do assert_accepts allow_value("12345").for(:attr), @model end - - bad_values = [nil, "", "abc", "0", "50001", "123456"] + + bad_values = [nil, "", "abc", "0", "50001", "123456", []] + bad_values.each do |value| should "not allow a bad value (#{value.inspect})" do assert_rejects allow_value(value).for(:attr), @model end end + + should "not allow bad values (#{bad_values.map {|v| v.inspect}.join(', ')})" do + assert_rejects allow_value(*bad_values).for(:attr), @model + end + end + + context "an AllowValueMatcher with multiple values" do + setup { @matcher = allow_value("foo", "bar").for(:baz) } + + should "describe itself" do + expected = 'allow baz to be set to any of ["foo", "bar"]' + assert_equal expected, @matcher.description + end end + context "an AllowValueMatcher with a single value" do + setup { @matcher = allow_value("foo").for(:baz) } + + should "describe itself" do + expected = 'allow baz to be set to "foo"' + assert_equal expected, @matcher.description + end + end end