From 118993480604d39c73687d069f7af3726f3e3f3e Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 30 Dec 2015 20:47:46 -0500 Subject: [PATCH] Add ignoring_interference_by_writer to all matchers `allow_value` matcher is, of course, concerned with setting values on a particular attribute on a particular record, and then checking that the record is valid after doing so. That comes with a caveat: if the attribute is overridden in such a way so that the same value going into the attribute isn't the same value coming out of it, then `allow_value` will balk -- it'll say, "I can't do that because that changes how I work." That's all well and good, but what the attribute intentionally changes incoming values? ActiveRecord's typecasting behavior, for instance, would trigger such an exception. What if the developer needs a way to get around this? This is where `ignoring_interference_by_writer` comes into play. You can tack it on to the end of the matcher, and you're free to go on your way. So, prior to this commit you could already apply it to `allow_value`, but now in this commit it also works on any other matcher. But, one little thing: sometimes using this qualifier isn't going to work. Perhaps you or something else actually *is* overriding the attribute to change incoming values in a specific way, and perhaps the value that comes out makes the record fail validation, and there's nothing you can do about it. So in this case, even if you're using `ignoring_interference_by_writer`, we want to inform you about what the attribute is doing -- what the input and output was. And so we do. --- NEWS.md | 6 + lib/shoulda/matchers/active_model.rb | 1 + .../active_model/allow_value_matcher.rb | 86 ++-- .../attribute_changed_value_error.rb | 24 +- .../allow_value_matcher/attribute_setter.rb | 40 +- .../attribute_setter_and_validator.rb | 4 +- .../active_model/disallow_value_matcher.rb | 18 +- .../numeric_type_matcher.rb | 2 + .../matchers/active_model/qualifiers.rb | 12 + .../ignore_interference_by_writer.rb | 97 +++++ .../ignoring_interference_by_writer.rb | 21 + .../validate_absence_of_matcher.rb | 81 ++-- .../validate_acceptance_of_matcher.rb | 33 ++ .../validate_exclusion_of_matcher.rb | 35 ++ .../validate_inclusion_of_matcher.rb | 60 ++- .../validate_length_of_matcher.rb | 35 ++ .../validate_numericality_of_matcher.rb | 42 +- .../validate_presence_of_matcher.rb | 37 +- .../active_model/validation_matcher.rb | 20 +- .../validate_uniqueness_of_matcher.rb | 287 +++++++++---- .../multiple_libraries_integration_spec.rb | 1 + .../ignoring_interference_by_writer.rb | 102 +++++ .../active_model/allow_value_matcher_spec.rb | 2 +- .../validate_absence_of_matcher_spec.rb | 96 ++++- .../validate_acceptance_of_matcher_spec.rb | 49 ++- .../validate_confirmation_of_matcher_spec.rb | 35 ++ .../validate_exclusion_of_matcher_spec.rb | 102 ++++- .../validate_inclusion_of_matcher_spec.rb | 338 ++++++++++++--- .../validate_length_of_matcher_spec.rb | 134 +++++- .../validate_numericality_of_matcher_spec.rb | 399 ++++++++++++++++++ .../validate_presence_of_matcher_spec.rb | 164 ++++++- .../validate_uniqueness_of_matcher_spec.rb | 223 +++++++--- 32 files changed, 2217 insertions(+), 369 deletions(-) create mode 100644 lib/shoulda/matchers/active_model/qualifiers.rb create mode 100644 lib/shoulda/matchers/active_model/qualifiers/ignore_interference_by_writer.rb create mode 100644 lib/shoulda/matchers/active_model/qualifiers/ignoring_interference_by_writer.rb create mode 100644 spec/support/unit/shared_examples/ignoring_interference_by_writer.rb diff --git a/NEWS.md b/NEWS.md index b605b5bb..3a00a61e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,12 @@ (formerly CouldNotSetAttributeError) when used against an attribute that is an enum in an ActiveRecord model. +### Features + +* Add a `ignoring_interference_by_writer` qualifier to all matchers, not just + `allow_value`. This makes it possible to get around CouldNotSetAttributeErrors + (now AttributeChangedValueErrors) that you are probably well acquainted with. + ### Improvements * Improve failure messages and descriptions of all matchers across the board so diff --git a/lib/shoulda/matchers/active_model.rb b/lib/shoulda/matchers/active_model.rb index e455d604..19f58a4b 100644 --- a/lib/shoulda/matchers/active_model.rb +++ b/lib/shoulda/matchers/active_model.rb @@ -1,4 +1,5 @@ require 'shoulda/matchers/active_model/helpers' +require 'shoulda/matchers/active_model/qualifiers' require 'shoulda/matchers/active_model/validation_matcher' require 'shoulda/matchers/active_model/validation_matcher/build_description' require 'shoulda/matchers/active_model/validator' diff --git a/lib/shoulda/matchers/active_model/allow_value_matcher.rb b/lib/shoulda/matchers/active_model/allow_value_matcher.rb index 615e3bc2..8b74f779 100644 --- a/lib/shoulda/matchers/active_model/allow_value_matcher.rb +++ b/lib/shoulda/matchers/active_model/allow_value_matcher.rb @@ -58,7 +58,7 @@ module Shoulda # #### Caveats # # When using `allow_value` or any matchers that depend on it, you may - # encounter a CouldNotSetAttributeError. This exception is raised if the + # encounter an AttributeChangedValueError. This exception is raised if the # matcher, in attempting to set a value on the attribute, detects that # the value set is different from the value that the attribute returns # upon reading it back. @@ -86,7 +86,7 @@ module Shoulda # it do # foo = Foo.new # foo.bar = "baz" - # # This will raise a CouldNotSetAttributeError since `foo.bar` is now "123" + # # This will raise an AttributeChangedValueError since `foo.bar` is now "123" # expect(foo).not_to allow_value(nil).for(:bar) # end # end @@ -108,7 +108,7 @@ module Shoulda # describe Foo do # it do # foo = Foo.new - # # This will raise a CouldNotSetAttributeError since `foo.bar` is now "123" + # # This will raise an AttributeChangedValueError since `foo.bar` is now "123" # expect(foo).not_to allow_value("abc123").for(:bar) # end # end @@ -118,15 +118,13 @@ module Shoulda # # describe Foo do # # Assume that `attr` is a string - # # This will raise a CouldNotSetAttributeError since `attr` typecasts `[]` to `"[]"` + # # This will raise an AttributeChangedValueError since `attr` typecasts `[]` to `"[]"` # it { should_not allow_value([]).for(:attr) } # end # - # So when you encounter this exception, you have a couple of options: - # - # * If you understand the problem and wish to override this behavior to - # get around this exception, you can add the - # `ignoring_interference_by_writer` qualifier like so: + # Fortunately, if you understand why this is happening, and wish to get + # around this exception, it is possible to do so. You can use the + # `ignoring_interference_by_writer` qualifier like so: # # it do # should_not allow_value([]). @@ -134,16 +132,11 @@ module Shoulda # ignoring_interference_by_writer # end # - # * Note, however, that the above option will not always cause the test to - # pass. In this case, this is telling you that you don't need to use - # `allow_value`, or quite possibly even the validation that you're - # testing altogether. In any case, we would probably make the argument - # that since it's clear that something is responsible for sanitizing - # incoming data before it's stored in your model, there's no need to - # ensure that sanitization places the model in a valid state, if such - # sanitization creates valid data. In terms of testing, the sanitization - # code should probably be tested, but not the effects of that - # sanitization on the validness of the model. + # Please note, however, that this qualifier won't magically cause your + # test to pass. It may just so happen that the final value that ends up + # being set causes the model to fail validation. In that case, you'll have + # to figure out what to do. You may need to write your own test, or + # perhaps even remove your test altogether. # # #### Qualifiers # @@ -274,9 +267,9 @@ module Shoulda # # ##### ignoring_interference_by_writer # - # Use `ignoring_interference_by_writer` if you've encountered a - # CouldNotSetAttributeError and wish to ignore it. Please read the Caveats - # section above for more information. + # Use `ignoring_interference_by_writer` to bypass an + # AttributeChangedValueError that you have encountered. Please read the + # Caveats section above for more information. # # class Address < ActiveRecord::Base # # Address has a zip_code field which is a string @@ -286,16 +279,16 @@ module Shoulda # describe Address do # it do # should_not allow_value([]). - # for(:zip_code). - # ignoring_interference_by_writer + # for(:zip_code). + # ignoring_interference_by_writer # end # end # # # Minitest (Shoulda) # class AddressTest < ActiveSupport::TestCase # should_not allow_value([]). - # for(:zip_code). - # ignoring_interference_by_writer + # for(:zip_code). + # ignoring_interference_by_writer # end # # @return [AllowValueMatcher] @@ -313,6 +306,7 @@ module Shoulda # @private class AllowValueMatcher include Helpers + include Qualifiers::IgnoringInterferenceByWriter attr_reader( :after_setting_value_callback, @@ -325,10 +319,10 @@ module Shoulda attr_writer :failure_message_preface, :values_to_preset def initialize(*values) + super @values_to_set = values @options = {} @after_setting_value_callback = -> {} - @ignoring_interference_by_writer = false @expects_strict = false @expects_custom_validation_message = false @context = nil @@ -387,15 +381,6 @@ module Shoulda @expects_strict end - def ignoring_interference_by_writer(value = true) - @ignoring_interference_by_writer = value - self - end - - def ignoring_interference_by_writer? - @ignoring_interference_by_writer - end - def _after_setting_value(&callback) @after_setting_value_callback = callback end @@ -432,6 +417,10 @@ module Shoulda end end + if include_attribute_changed_value_message? + message << "\n\n" + attribute_changed_value_message + end + Shoulda::Matchers.word_wrap(message) end @@ -497,9 +486,23 @@ module Shoulda end end + if include_attribute_changed_value_message? + message << "\n\n" + attribute_changed_value_message + end + Shoulda::Matchers.word_wrap(message) end + def attribute_changed_value_message + <<-MESSAGE.strip +As indicated in the message above, :#{result.attribute_setter.attribute_name} +seems to be changing certain values as they are set, and this could have +something to do with why this test is failing. If you've overridden the writer +method for this attribute, then you may need to change it to make this test +pass, or do something else entirely. + MESSAGE + end + def description ValidationMatcher::BuildDescription.call(self, simple_description) end @@ -512,8 +515,12 @@ module Shoulda instance.class end + def last_attribute_setter_used + result.attribute_setter + end + def last_value_set - result.attribute_setter.value_written + last_attribute_setter_used.value_written end protected @@ -579,6 +586,11 @@ module Shoulda ) end + def include_attribute_changed_value_message? + !ignore_interference_by_writer.never? && + result.attribute_setter.attribute_changed_value? + end + def inspected_values_to_set Shoulda::Matchers::Util.inspect_values(values_to_set).to_sentence( two_words_connector: " or ", diff --git a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_changed_value_error.rb b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_changed_value_error.rb index ea57752a..417982ee 100644 --- a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_changed_value_error.rb +++ b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_changed_value_error.rb @@ -13,23 +13,25 @@ The #{matcher_name} matcher attempted to set :#{attribute_name} on #{model.name} to #{value_written.inspect}, but when the attribute was read back, it had stored #{value_read.inspect} instead. -This creates a problem because it means that the model is behaving in a way that -is interfering with the test -- there's a mismatch between the test that was -written and test that was actually run. +This creates a problem because it means that the model is behaving in a +way that is interfering with the test -- there's a mismatch between the +test that you wrote and test that we actually ran. There are a couple of reasons why this could be happening: -* The writer method for :#{attribute_name} has been overridden and contains -custom logic to prevent certain values from being set or change which values -are stored. * ActiveRecord is typecasting the incoming value. +* The writer method for :#{attribute_name} has been overridden so that + incoming values are changed in some way. -Regardless, the fact you're seeing this message usually indicates a larger -problem. Please file an issue on the GitHub repo for shoulda-matchers, -including details about your model and the test you've written, and we can point -you in the right direction: +If this exception makes sense to you and you wish to bypass it, try +adding the `ignoring_interference_by_writer` qualifier onto the end of +your matcher. If the test still does not pass after that, then you may +need to do something different. -https://github.com/thoughtbot/shoulda-matchers/issues +If you need help, feel free to ask a question on the shoulda-matchers +issues list: + +http://github.com/thoughtbot/shoulda-matchers/issues MESSAGE end diff --git a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter.rb b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter.rb index e174ad0d..9fde9033 100644 --- a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter.rb +++ b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter.rb @@ -8,16 +8,23 @@ module Shoulda new(args).set end - attr_reader :result_of_checking, :result_of_setting, - :value_written + attr_reader( + :attribute_name, + :result_of_checking, + :result_of_setting, + :value_written, + ) def initialize(args) + @args = args @matcher_name = args.fetch(:matcher_name) @object = args.fetch(:object) @attribute_name = args.fetch(:attribute_name) @value_written = args.fetch(:value) - @ignoring_interference_by_writer = - args.fetch(:ignoring_interference_by_writer, false) + @ignore_interference_by_writer = args.fetch( + :ignore_interference_by_writer, + Qualifiers::IgnoreInterferenceByWriter.new + ) @after_set_callback = args.fetch(:after_set_callback, -> { }) @result_of_checking = nil @@ -125,10 +132,17 @@ module Shoulda set? && result_of_setting.successful? end + def value_read + @_value_read ||= object.public_send(attribute_name) + end + + def attribute_changed_value? + value_written != value_read + end + protected - attr_reader :matcher_name, :object, :attribute_name, - :after_set_callback + attr_reader :args, :matcher_name, :object, :after_set_callback private @@ -144,22 +158,14 @@ module Shoulda end end - def attribute_changed_value? - value_written != value_read - end - - def value_read - @_value_read ||= object.public_send(attribute_name) - end - - def ignoring_interference_by_writer? - !!@ignoring_interference_by_writer + def ignore_interference_by_writer + @ignore_interference_by_writer end def raise_attribute_changed_value_error? attribute_changed_value? && !(attribute_is_an_enum? && value_read_is_expected_for_an_enum?) && - !ignoring_interference_by_writer? + !ignore_interference_by_writer.considering?(value_read) end def attribute_is_an_enum? diff --git a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter_and_validator.rb b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter_and_validator.rb index 6b6c752b..8d89fbe3 100644 --- a/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter_and_validator.rb +++ b/lib/shoulda/matchers/active_model/allow_value_matcher/attribute_setter_and_validator.rb @@ -15,7 +15,7 @@ module Shoulda :context, :expected_message, :expects_strict?, - :ignoring_interference_by_writer?, + :ignore_interference_by_writer, :instance, ) @@ -33,7 +33,7 @@ module Shoulda object: instance, attribute_name: attribute_name, value: value, - ignoring_interference_by_writer: ignoring_interference_by_writer?, + ignore_interference_by_writer: ignore_interference_by_writer, after_set_callback: after_setting_value_callback ) end diff --git a/lib/shoulda/matchers/active_model/disallow_value_matcher.rb b/lib/shoulda/matchers/active_model/disallow_value_matcher.rb index ad6363e4..cd0ffba9 100644 --- a/lib/shoulda/matchers/active_model/disallow_value_matcher.rb +++ b/lib/shoulda/matchers/active_model/disallow_value_matcher.rb @@ -7,17 +7,21 @@ module Shoulda class DisallowValueMatcher extend Forwardable - def_delegators :allow_matcher, + def_delegators( + :allow_matcher, :_after_setting_value, :attribute_to_set, :description, :expects_strict?, - :model, - :last_value_set, - :simple_description, :failure_message_preface, :failure_message_preface=, - :values_to_preset= + :ignore_interference_by_writer, + :last_attribute_setter_used, + :last_value_set, + :model, + :simple_description, + :values_to_preset=, + ) def initialize(value) @allow_matcher = AllowValueMatcher.new(value) @@ -51,8 +55,8 @@ module Shoulda self end - def ignoring_interference_by_writer - allow_matcher.ignoring_interference_by_writer + def ignoring_interference_by_writer(value = :always) + allow_matcher.ignoring_interference_by_writer(value) self end diff --git a/lib/shoulda/matchers/active_model/numericality_matchers/numeric_type_matcher.rb b/lib/shoulda/matchers/active_model/numericality_matchers/numeric_type_matcher.rb index 5c23c9fe..4a54b886 100644 --- a/lib/shoulda/matchers/active_model/numericality_matchers/numeric_type_matcher.rb +++ b/lib/shoulda/matchers/active_model/numericality_matchers/numeric_type_matcher.rb @@ -14,6 +14,8 @@ module Shoulda :expects_strict?, :failure_message, :failure_message_when_negated, + :ignore_interference_by_writer, + :ignoring_interference_by_writer, :matches?, :on, :strict, diff --git a/lib/shoulda/matchers/active_model/qualifiers.rb b/lib/shoulda/matchers/active_model/qualifiers.rb new file mode 100644 index 00000000..5a277204 --- /dev/null +++ b/lib/shoulda/matchers/active_model/qualifiers.rb @@ -0,0 +1,12 @@ +module Shoulda + module Matchers + module ActiveModel + # @private + module Qualifiers + end + end + end +end + +require_relative 'qualifiers/ignore_interference_by_writer' +require_relative 'qualifiers/ignoring_interference_by_writer' diff --git a/lib/shoulda/matchers/active_model/qualifiers/ignore_interference_by_writer.rb b/lib/shoulda/matchers/active_model/qualifiers/ignore_interference_by_writer.rb new file mode 100644 index 00000000..04f1ab68 --- /dev/null +++ b/lib/shoulda/matchers/active_model/qualifiers/ignore_interference_by_writer.rb @@ -0,0 +1,97 @@ +module Shoulda + module Matchers + module ActiveModel + module Qualifiers + # @private + class IgnoreInterferenceByWriter + attr_reader :setting, :condition + + def initialize(argument = :never) + set(argument) + @changed = false + end + + def set(argument) + if argument.is_a?(self.class) + @setting = argument.setting + @condition = argument.condition + else + case argument + when true, :always + @setting = :always + when false, :never + @setting = :never + else + @setting = :sometimes + + if argument.is_a?(Hash) + @condition = argument.fetch(:when) + else + raise invalid_argument_error(argument) + end + end + end + + @changed = true + + self + rescue KeyError + raise invalid_argument_error(argument) + end + + def default_to(argument) + temporary_ignore_interference_by_writer = + IgnoreInterferenceByWriter.new(argument) + + unless changed? + @setting = temporary_ignore_interference_by_writer.setting + @condition = temporary_ignore_interference_by_writer.condition + end + + self + end + + def considering?(value) + case setting + when :always then true + when :never then false + else condition_matches?(value) + end + end + + def never? + setting == :never + end + + def changed? + @changed + end + + private + + def invalid_argument_error(invalid_argument) + ArgumentError.new(<<-ERROR) +Unknown argument: #{invalid_argument.inspect}. + +ignoring_interference_by_writer takes one of three arguments: + +* A symbol, either :never or :always. +* A boolean, either true (which means always) or false (which means + never). +* A hash with a single key, :when, and a single value, which is either + the name of a method or a Proc. + ERROR + end + + def condition_matches?(value) + if condition.respond_to?(:call) + condition.call(value) + else + value.public_send(condition) + end + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_model/qualifiers/ignoring_interference_by_writer.rb b/lib/shoulda/matchers/active_model/qualifiers/ignoring_interference_by_writer.rb new file mode 100644 index 00000000..31302f02 --- /dev/null +++ b/lib/shoulda/matchers/active_model/qualifiers/ignoring_interference_by_writer.rb @@ -0,0 +1,21 @@ +module Shoulda + module Matchers + module ActiveModel + module Qualifiers + # @private + module IgnoringInterferenceByWriter + attr_reader :ignore_interference_by_writer + + def initialize(*args) + @ignore_interference_by_writer = IgnoreInterferenceByWriter.new + end + + def ignoring_interference_by_writer(value = :always) + @ignore_interference_by_writer.set(value) + self + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb index 52d013e2..7bc45048 100644 --- a/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb @@ -4,21 +4,21 @@ module Shoulda # The `validate_absence_of` matcher tests the usage of the # `validates_absence_of` validation. # - # class Artillery + # class PowerHungryCountry # include ActiveModel::Model - # attr_accessor :arms + # attr_accessor :nuclear_weapons # - # validates_absence_of :arms + # validates_absence_of :nuclear_weapons # end # # # RSpec - # describe Artillery do - # it { should validate_absence_of(:arms) } + # describe PowerHungryCountry do + # it { should validate_absence_of(:nuclear_weapons) } # end # # # Minitest (Shoulda) - # class ArtilleryTest < ActiveSupport::TestCase - # should validate_absence_of(:arms) + # class PowerHungryCountryTest < ActiveSupport::TestCase + # should validate_absence_of(:nuclear_weapons) # end # # #### Qualifiers @@ -27,47 +27,80 @@ module Shoulda # # Use `on` if your validation applies only under a certain context. # - # class Artillery + # class PowerHungryCountry # include ActiveModel::Model - # attr_accessor :arms + # attr_accessor :nuclear_weapons # - # validates_absence_of :arms, on: :create + # validates_absence_of :nuclear_weapons, on: :create # end # # # RSpec - # describe Artillery do - # it { should validate_absence_of(:arms).on(:create) } + # describe PowerHungryCountry do + # it { should validate_absence_of(:nuclear_weapons).on(:create) } # end # # # Minitest (Shoulda) - # class ArtilleryTest < ActiveSupport::TestCase - # should validate_absence_of(:arms).on(:create) + # class PowerHungryCountryTest < ActiveSupport::TestCase + # should validate_absence_of(:nuclear_weapons).on(:create) # end # # ##### with_message # # Use `with_message` if you are using a custom validation message. # - # class Artillery + # class PowerHungryCountry # include ActiveModel::Model - # attr_accessor :arms + # attr_accessor :nuclear_weapons # - # validates_absence_of :arms, - # message: "We're fresh outta arms here, soldier!" + # validates_absence_of :nuclear_weapons, + # message: "there shall be peace on Earth" # end # # # RSpec - # describe Artillery do + # describe PowerHungryCountry do # it do - # should validate_absence_of(:arms). - # with_message("We're fresh outta arms here, soldier!") + # should validate_absence_of(:nuclear_weapons). + # with_message("there shall be peace on Earth") # end # end # # # Minitest (Shoulda) - # class ArtilleryTest < ActiveSupport::TestCase - # should validate_absence_of(:arms). - # with_message("We're fresh outta arms here, soldier!") + # class PowerHungryCountryTest < ActiveSupport::TestCase + # should validate_absence_of(:nuclear_weapons). + # with_message("there shall be peace on Earth") + # end + # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and causes it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class PowerHungryCountry + # include ActiveModel::Model + # attr_accessor :nuclear_weapons + # + # validates_absence_of :nuclear_weapons + # + # def nuclear_weapons=(value) + # @nuclear_weapons = value + [:hidden_weapon] + # end + # end + # + # # RSpec + # describe PowerHungryCountry do + # it do + # should validate_absence_of(:nuclear_weapons). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class PowerHungryCountryTest < ActiveSupport::TestCase + # should validate_absence_of(:nuclear_weapons). + # ignoring_interference_by_writer # end # # @return [ValidateAbsenceOfMatcher} diff --git a/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb index 1c64d2cb..3286bdf5 100644 --- a/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb @@ -73,6 +73,39 @@ module Shoulda # with_message('You must accept the terms of service') # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class Registration + # include ActiveModel::Model + # attr_accessor :terms_of_service + # + # validates_acceptance_of :terms_of_service + # + # def terms_of_service=(value) + # @terms_of_service = value || 'something else' + # end + # end + # + # # RSpec + # describe Registration do + # it do + # should validate_acceptance_of(:terms_of_service). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class RegistrationTest < ActiveSupport::TestCase + # should validate_acceptance_of(:terms_of_service). + # ignoring_interference_by_writer + # end + # # @return [ValidateAcceptanceOfMatcher] # def validate_acceptance_of(attr) diff --git a/lib/shoulda/matchers/active_model/validate_exclusion_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_exclusion_of_matcher.rb index 3a0281ce..5c029da7 100644 --- a/lib/shoulda/matchers/active_model/validate_exclusion_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_exclusion_of_matcher.rb @@ -112,6 +112,41 @@ module Shoulda # with_message('You chose a puny weapon') # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class Game < ActiveRecord::Base + # include ActiveModel::Model + # attr_accessor :weapon + # + # validates_exclusion_of :weapon + # + # def weapon=(value) + # @weapon = value.gsub(' ', '') + # end + # end + # + # # RSpec + # describe Game do + # it do + # should validate_exclusion_of(:weapon). + # in_array(['pistol', 'paintball gun', 'stick']). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class GameTest < ActiveSupport::TestCase + # should validate_exclusion_of(:weapon). + # in_array(['pistol', 'paintball gun', 'stick']). + # ignoring_interference_by_writer + # end + # # @return [ValidateExclusionOfMatcher] # def validate_exclusion_of(attr) diff --git a/lib/shoulda/matchers/active_model/validate_inclusion_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_inclusion_of_matcher.rb index 2e5f8262..a996d295 100644 --- a/lib/shoulda/matchers/active_model/validate_inclusion_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_inclusion_of_matcher.rb @@ -13,21 +13,22 @@ module Shoulda # include ActiveModel::Model # attr_accessor :state # - # validates_inclusion_of :state, in: %w(open resolved unresolved) + # validates_inclusion_of :state, + # in: ['open', 'resolved', 'unresolved'] # end # # # RSpec # describe Issue do # it do # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)) + # in_array(['open', 'resolved', 'unresolved']) # end # end # # # Minitest (Shoulda) # class IssueTest < ActiveSupport::TestCase # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)) + # in_array(['open', 'resolved', 'unresolved']) # end # # If your whitelist is a range of values, use `in_range`: @@ -57,7 +58,10 @@ module Shoulda # one of these three values. That means there isn't any way we can refute # this logic in a test. Hence, this will produce a warning: # - # it { should validate_inclusion_of(:imported).in_array([true, false]) } + # it do + # should validate_inclusion_of(:imported). + # in_array([true, false]) + # end # # The only case where `validate_inclusion_of` *could* be appropriate is # for ensuring that a boolean column accepts nil, but we recommend @@ -205,7 +209,7 @@ module Shoulda # # validates_presence_of :state # validates_inclusion_of :state, - # in: %w(open resolved unresolved), + # in: ['open', 'resolved', 'unresolved'], # allow_nil: true # end # @@ -213,7 +217,7 @@ module Shoulda # describe Issue do # it do # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)). + # in_array(['open', 'resolved', 'unresolved']). # allow_nil # end # end @@ -221,7 +225,7 @@ module Shoulda # # Minitest (Shoulda) # class IssueTest < ActiveSupport::TestCase # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)). + # in_array(['open', 'resolved', 'unresolved']). # allow_nil # end # @@ -235,7 +239,7 @@ module Shoulda # # validates_presence_of :state # validates_inclusion_of :state, - # in: %w(open resolved unresolved), + # in: ['open', 'resolved', 'unresolved'], # allow_blank: true # end # @@ -243,7 +247,7 @@ module Shoulda # describe Issue do # it do # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)). + # in_array(['open', 'resolved', 'unresolved']). # allow_blank # end # end @@ -251,10 +255,46 @@ module Shoulda # # Minitest (Shoulda) # class IssueTest < ActiveSupport::TestCase # should validate_inclusion_of(:state). - # in_array(%w(open resolved unresolved)). + # in_array(['open', 'resolved', 'unresolved']). # allow_blank # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class Issue + # include ActiveModel::Model + # attr_accessor :state + # + # validates_inclusion_of :state, + # in: ['open', 'resolved', 'unresolved'] + # + # def state=(value) + # @state = 'open' + # end + # end + # + # # RSpec + # describe Issue do + # it do + # should validate_inclusion_of(:state). + # in_array(['open', 'resolved', 'unresolved']). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class IssueTest < ActiveSupport::TestCase + # should validate_inclusion_of(:state). + # in_array(['open', 'resolved', 'unresolved']). + # ignoring_interference_by_writer + # end + # # @return [ValidateInclusionOfMatcher] # def validate_inclusion_of(attr) diff --git a/lib/shoulda/matchers/active_model/validate_length_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_length_of_matcher.rb index 97fa2f47..f8fd8d62 100644 --- a/lib/shoulda/matchers/active_model/validate_length_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_length_of_matcher.rb @@ -216,6 +216,41 @@ module Shoulda # with_long_message('Secret key must be less than 100 characters') # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class User + # include ActiveModel::Model + # attr_accessor :password + # + # validates_length_of :password, minimum: 10 + # + # def password=(value) + # @password = value.upcase + # end + # end + # + # # RSpec + # describe User do + # it do + # should validate_length_of(:password). + # is_at_least(10). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class UserTest < ActiveSupport::TestCase + # should validate_length_of(:password). + # is_at_least(10). + # ignoring_interference_by_writer + # end + # # @return [ValidateLengthOfMatcher] # def validate_length_of(attr) 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 f44d5431..1a81f720 100644 --- a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb @@ -279,7 +279,7 @@ module Shoulda # # Use `allow_nil` to assert that the attribute allows nil. # - # class Age + # class Post # include ActiveModel::Model # attr_accessor :age # @@ -296,6 +296,39 @@ module Shoulda # should validate_numericality_of(:age).allow_nil # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # Here, `gpa` is an integer column, so it will typecast all values to + # integers. We need to use `ignoring_interference_by_writer` because the + # `only_integer` qualifier will attempt to set `gpa` to a float and assert + # that it makes the record invalid. + # + # class Person < ActiveRecord::Base + # validates_numericality_of :gpa, only_integer: true + # end + # + # # RSpec + # describe Person do + # it do + # should validate_numericality_of(:gpa). + # only_integer. + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class PersonTest < ActiveSupport::TestCase + # should validate_numericality_of(:gpa). + # only_integer. + # ignoring_interference_by_writer + # end + # # @return [ValidateNumericalityOfMatcher] # def validate_numericality_of(attr) @@ -308,9 +341,12 @@ module Shoulda NON_NUMERIC_VALUE = 'abcd' DEFAULT_DIFF_TO_COMPARE = 1 + include Qualifiers::IgnoringInterferenceByWriter + attr_reader :diff_to_compare def initialize(attribute) + super @attribute = attribute @submatchers = [] @diff_to_compare = DEFAULT_DIFF_TO_COMPARE @@ -541,6 +577,10 @@ module Shoulda if @context submatcher.on(@context) end + + submatcher.ignoring_interference_by_writer( + ignore_interference_by_writer + ) end end diff --git a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb index 770bf9c4..c217212a 100644 --- a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb @@ -103,6 +103,39 @@ module Shoulda # with_message('Robot has no legs') # end # + # ##### ignoring_interference_by_writer + # + # Use `ignoring_interference_by_writer` when the attribute you're testing + # changes incoming values. This qualifier will instruct the matcher to + # suppress raising an AttributeValueChangedError, as long as changing the + # doesn't also change the outcome of the test and cause it to fail. See + # the documentation for `allow_value` for more information on this. + # + # class Robot + # include ActiveModel::Model + # attr_accessor :name + # + # validates_presence_of :name + # + # def name=(name) + # @name = name.to_s + # end + # end + # + # # RSpec + # describe Robot do + # it do + # should validate_presence_of(:name). + # ignoring_interference_by_writer + # end + # end + # + # # Minitest (Shoulda) + # class RobotTest < ActiveSupport::TestCase + # should validate_presence_of(:name). + # ignoring_interference_by_writer + # end + # # @return [ValidatePresenceOfMatcher] # def validate_presence_of(attr) @@ -114,6 +147,8 @@ module Shoulda def initialize(attribute) super @expected_message = :blank + @ignore_interference_by_writer = + Qualifiers::IgnoreInterferenceByWriter.new(when: :blank?) end def matches?(subject) @@ -146,8 +181,6 @@ module Shoulda def disallows_original_or_typecast_value?(value, message) disallows_value_of(blank_value, @expected_message) - rescue ActiveModel::AllowValueMatcher::AttributeChangedValueError => error - error.value_read.blank? end def blank_value diff --git a/lib/shoulda/matchers/active_model/validation_matcher.rb b/lib/shoulda/matchers/active_model/validation_matcher.rb index a7d97dd3..3f6d4ba5 100644 --- a/lib/shoulda/matchers/active_model/validation_matcher.rb +++ b/lib/shoulda/matchers/active_model/validation_matcher.rb @@ -3,7 +3,10 @@ module Shoulda module ActiveModel # @private class ValidationMatcher + include Qualifiers::IgnoringInterferenceByWriter + def initialize(attribute) + super @attribute = attribute @expects_strict = false @subject = nil @@ -12,6 +15,10 @@ module Shoulda @expects_custom_validation_message = false end + def description + ValidationMatcher::BuildDescription.call(self, simple_description) + end + def on(context) @context = context self @@ -26,11 +33,6 @@ module Shoulda @expects_strict end - def matches?(subject) - @subject = subject - false - end - def with_message(expected_message) if expected_message @expects_custom_validation_message = true @@ -44,8 +46,9 @@ module Shoulda @expects_custom_validation_message end - def description - ValidationMatcher::BuildDescription.call(self, simple_description) + def matches?(subject) + @subject = subject + false end def failure_message @@ -139,7 +142,8 @@ module Shoulda for(attribute). with_message(message). on(context). - strict(expects_strict?) + strict(expects_strict?). + ignoring_interference_by_writer(ignore_interference_by_writer) yield matcher if block_given? diff --git a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb index 6dbdb599..6c4950ef 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -219,11 +219,13 @@ module Shoulda super(attribute) @expected_message = :taken @options = {} - @existing_record = nil @existing_record_created = false - @original_existing_value = nil @failure_reason = nil @failure_reason_when_negated = nil + @attribute_setters = { + existing_record: [], + new_record: [] + } end def scoped_to(*scopes) @@ -277,9 +279,10 @@ module Shoulda @all_records = model.all existing_record_valid? && + validate_attribute_present? && validate_scopes_present? && scopes_match? && - validate_everything_except_duplicate_nils_or_blanks? && + validate_two_records_with_same_non_blank_value_cannot_coexist? && validate_case_sensitivity? && validate_after_scope_change? && allows_nil? && @@ -307,7 +310,11 @@ module Shoulda private def new_record - @_new_record ||= build_new_record + unless defined?(@new_record) + build_new_record + end + + @new_record end alias_method :subject, :new_record @@ -362,7 +369,7 @@ module Shoulda def allows_nil? if expects_to_allow_nil? - update_existing_record(nil) + update_existing_record!(nil) allows_value_of(nil, @expected_message) else true @@ -371,7 +378,7 @@ module Shoulda def allows_blank? if expects_to_allow_blank? - update_existing_record('') + update_existing_record!('') allows_value_of('', @expected_message) else true @@ -383,64 +390,57 @@ module Shoulda true else @failure_reason = - "Given record could not be set to #{value.inspect}: " + - existing_record.errors.full_messages + "The record you provided could not be created, " + + "as it failed with the following validation errors:\n\n" + + format_validation_errors(existing_record.errors) false end end def existing_record - @existing_record ||= find_or_create_existing_record + unless defined?(@existing_record) + find_or_create_existing_record + end + + @existing_record end def find_or_create_existing_record - if find_existing_record - find_existing_record - else - create_existing_record.tap do |existing_record| - @existing_record_created = true - end + @existing_record = find_existing_record + + unless @existing_record + @existing_record = create_existing_record + @existing_record_created = true end end def find_existing_record record = model.first - if valid_existing_record?(record) - record.tap do |existing_record| - @original_existing_value = existing_record.public_send(@attribute) - end + if record.present? + record else nil end end - def valid_existing_record?(record) - record.present? && - record_has_nil_when_required?(record) && - record_has_blank_when_required?(record) - end - - def record_has_nil_when_required?(record) - !expects_to_allow_nil? || record.public_send(@attribute).nil? - end - - def record_has_blank_when_required?(record) - !expects_to_allow_blank? || - record.public_send(@attribute).to_s.strip.empty? - end - def create_existing_record @given_record.tap do |existing_record| - @original_existing_value = value = arbitrary_non_blank_value - existing_record.public_send("#{@attribute}=", value) ensure_secure_password_set(existing_record) existing_record.save end end - def update_existing_record(value) - existing_record.update_column(attribute, value) + def update_existing_record!(value) + if existing_value_read != value + set_attribute_on!( + :existing_record, + existing_record, + @attribute, + value + ) + existing_record.save! + end end def ensure_secure_password_set(instance) @@ -468,15 +468,25 @@ module Shoulda end def build_new_record - existing_record.dup.tap do |new_record| - new_record.public_send("#{@attribute}=", existing_value) + @new_record = existing_record.dup - expected_scopes.each do |scope| - new_record.public_send( - "#{scope}=", - existing_record.public_send(scope) - ) - end + attribute_names_under_test.each do |attribute_name| + set_attribute_on_new_record!( + attribute_name, + existing_record.public_send(attribute_name) + ) + end + + @new_record + end + + def validate_attribute_present? + if model.method_defined?("#{attribute}=") + true + else + @failure_reason = + ":#{attribute} does not seem to be an attribute on #{model.name}." + false end end @@ -486,9 +496,9 @@ module Shoulda else reason = '' - reason << inspected_missing_scopes.to_sentence + reason << inspected_scopes_missing_on_model.to_sentence - if inspected_missing_scopes.many? + if inspected_scopes_missing_on_model.many? reason << " do not seem to be attributes" else reason << " does not seem to be an attribute" @@ -503,29 +513,29 @@ module Shoulda end def all_scopes_present_on_model? - missing_scopes.none? + scopes_missing_on_model.none? end - def missing_scopes + def scopes_missing_on_model @_missing_scopes ||= expected_scopes.select do |scope| - !@given_record.respond_to?("#{scope}=") + !model.method_defined?("#{scope}=") end end - def inspected_missing_scopes - missing_scopes.map(&:inspect) + def inspected_scopes_missing_on_model + scopes_missing_on_model.map(&:inspect) end - def validate_everything_except_duplicate_nils_or_blanks? - if existing_value.nil? || (expects_to_allow_blank? && existing_value.blank?) - update_existing_record(arbitrary_non_blank_value) + def validate_two_records_with_same_non_blank_value_cannot_coexist? + if existing_value_read.blank? + update_existing_record!(arbitrary_non_blank_value) end - disallows_value_of(existing_value, @expected_message) + disallows_value_of(existing_value_read, @expected_message) end def validate_case_sensitivity? - value = existing_value + value = existing_value_read if value.respond_to?(:swapcase) && !value.empty? swapcased_value = value.swapcase @@ -568,10 +578,10 @@ module Shoulda next_value_for(scope, previous_value) end - new_record.public_send("#{scope}=", next_value) + set_attribute_on_new_record!(scope, next_value) - if allows_value_of(existing_value, @expected_message) - new_record.public_send("#{scope}=", previous_value) + if allows_value_of(existing_value_read, @expected_message) + set_attribute_on_new_record!(scope, previous_value) true else false @@ -648,12 +658,63 @@ module Shoulda end end - def existing_value + def set_attribute_on!(record_type, record, attribute_name, value) + attribute_setter = build_attribute_setter( + record, + attribute_name, + value + ) + attribute_setter.set! + + @attribute_setters[record_type] << attribute_setter + end + + def set_attribute_on_existing_record!(attribute_name, value) + set_attribute_on!( + :existing_record, + existing_record, + attribute_name, + value + ) + end + + def set_attribute_on_new_record!(attribute_name, value) + set_attribute_on!( + :new_record, + new_record, + attribute_name, + value + ) + end + + def attribute_setter_for_existing_record + @attribute_setters[:existing_record].last + end + + def attribute_names_under_test + [@attribute] + expected_scopes + end + + def build_attribute_setter(record, attribute_name, value) + Shoulda::Matchers::ActiveModel::AllowValueMatcher::AttributeSetter.new( + matcher_name: :validate_uniqueness_of, + object: record, + attribute_name: attribute_name, + value: value, + ignore_interference_by_writer: ignore_interference_by_writer + ) + end + + def existing_value_read existing_record.public_send(@attribute) end - def model - @given_record.class + def existing_value_written + if attribute_setter_for_existing_record + attribute_setter_for_existing_record.value_written + else + existing_value_read + end end def column_for(scope) @@ -664,47 +725,91 @@ module Shoulda column_for(attribute).try(:limit) end + def model + @given_record.class + end + def failure_message_preface prefix = '' if @existing_record_created - prefix << "After taking the given #{model.name}," - prefix << " setting its :#{attribute} to " - prefix << Shoulda::Matchers::Util.inspect_value(existing_value) - prefix << ", and saving it as the existing record," - prefix << " then" - elsif @original_existing_value != existing_value - prefix << "Given an existing #{model.name}," - prefix << " after setting its :#{attribute} to " - prefix << Shoulda::Matchers::Util.inspect_value(existing_value) - prefix << ", then" + prefix << "After taking the given #{model.name}" + + if attribute_setter_for_existing_record + prefix << ', setting its ' + prefix << description_for_attribute_setter( + attribute_setter_for_existing_record + ) + else + prefix << ", whose :#{attribute} is " + prefix << "‹#{existing_value_read.inspect}›" + end + + prefix << ", and saving it as the existing record, then" else - prefix << "Given an existing #{model.name} whose :#{attribute}" - prefix << " is " - prefix << Shoulda::Matchers::Util.inspect_value(existing_value) - prefix << ", after" + if attribute_setter_for_existing_record + prefix << "Given an existing #{model.name}," + prefix << ' after setting its ' + prefix << description_for_attribute_setter( + attribute_setter_for_existing_record + ) + prefix << ', then' + else + prefix << "Given an existing #{model.name} whose :#{attribute}" + prefix << ' is ' + prefix << Shoulda::Matchers::Util.inspect_value( + existing_value_read + ) + prefix << ', after' + end end - prefix << " making a new #{model.name} and setting its" - prefix << " :#{attribute} to " + prefix << " making a new #{model.name} and setting its " - if last_value_set_on_new_record == existing_value - prefix << Shoulda::Matchers::Util.inspect_value( - last_value_set_on_new_record - ) - prefix << " as well" - else - prefix << " a different value, " - prefix << Shoulda::Matchers::Util.inspect_value( - last_value_set_on_new_record - ) - end + prefix << description_for_attribute_setter( + last_attribute_setter_used_on_new_record, + same_as_existing: existing_and_new_values_are_same? + ) prefix << ", the matcher expected the new #{model.name} to be" prefix end + def description_for_attribute_setter(attribute_setter, same_as_existing: nil) + description = ":#{attribute_setter.attribute_name} to " + + if same_as_existing == false + description << 'a different value, ' + end + + description << Shoulda::Matchers::Util.inspect_value( + attribute_setter.value_written + ) + + if attribute_setter.attribute_changed_value? + description << ' (read back as ' + description << Shoulda::Matchers::Util.inspect_value( + attribute_setter.value_read + ) + description << ')' + end + + if same_as_existing == true + description << ' as well' + end + + description + end + + def existing_and_new_values_are_same? + last_value_set_on_new_record == existing_value_written + end + + def last_attribute_setter_used_on_new_record + last_submatcher_run.last_attribute_setter_used + end + def last_value_set_on_new_record last_submatcher_run.last_value_set end diff --git a/spec/acceptance/multiple_libraries_integration_spec.rb b/spec/acceptance/multiple_libraries_integration_spec.rb index dfc3d1aa..d03babac 100644 --- a/spec/acceptance/multiple_libraries_integration_spec.rb +++ b/spec/acceptance/multiple_libraries_integration_spec.rb @@ -25,6 +25,7 @@ describe 'shoulda-matchers integrates with multiple libraries' do add_rspec_file 'spec/models/user_spec.rb', <<-FILE describe User do + subject { User.new(name: "John Smith") } it { should validate_presence_of(:name) } it { should validate_uniqueness_of(:name) } end diff --git a/spec/support/unit/shared_examples/ignoring_interference_by_writer.rb b/spec/support/unit/shared_examples/ignoring_interference_by_writer.rb new file mode 100644 index 00000000..2edb20e0 --- /dev/null +++ b/spec/support/unit/shared_examples/ignoring_interference_by_writer.rb @@ -0,0 +1,102 @@ +shared_examples_for 'ignoring_interference_by_writer' do |common_config| + valid_tests = [ + :raise_if_not_qualified, + :accept_if_qualified_but_changing_value_does_not_interfere, + :reject_if_qualified_but_changing_value_interferes + ] + tests = common_config.fetch(:tests) + tests.assert_valid_keys(valid_tests) + + define_method(:common_config) { common_config } + + context 'when the writer method for the attribute changes incoming values' do + context 'and the matcher has not been qualified with ignoring_interference_by_writer' do + config_for_test = tests[:raise_if_not_qualified] + + if config_for_test + it 'raises an AttributeChangedValueError' do + args = build_args(config_for_test) + scenario = build_scenario_for_validation_matcher(args) + matcher = matcher_from(scenario) + + assertion = lambda do + expect(scenario.record).to matcher + end + + expect(&assertion).to raise_error( + Shoulda::Matchers::ActiveModel::AllowValueMatcher::AttributeChangedValueError + ) + end + end + end + + context 'and the matcher has been qualified with ignoring_interference_by_writer' do + context 'and the value change does not cause a test failure' do + config_for_test = tests[:accept_if_qualified_but_changing_value_does_not_interfere] + + if config_for_test + it 'accepts (and does not raise an error)' do + args = build_args(config_for_test) + scenario = build_scenario_for_validation_matcher(args) + matcher = matcher_from(scenario) + + expect(scenario.record).to matcher.ignoring_interference_by_writer + end + end + end + + context 'and the value change causes a test failure' do + config_for_test = tests[:reject_if_qualified_but_changing_value_interferes] + + if config_for_test + it 'lists how the value got changed in the failure message' do + args = build_args(config_for_test) + scenario = build_scenario_for_validation_matcher(args) + matcher = matcher_from(scenario) + + assertion = lambda do + expect(scenario.record).to matcher.ignoring_interference_by_writer + end + + if config_for_test.key?(:expected_message_includes) + message = config_for_test[:expected_message_includes] + expect(&assertion).to fail_with_message_including(message) + else + message = config_for_test.fetch(:expected_message) + expect(&assertion).to fail_with_message(message) + end + end + end + end + end + end + + def build_args(config_for_test) + args_from_common_config.merge(args_from_config_for_test(config_for_test)) + end + + def args_from_common_config + common_config.slice( + :column_type, + :model_creator, + ) + end + + def args_from_config_for_test(config) + config.slice( + :attribute_name, + :attribute_overrides, + :changing_values_with, + :default_value, + :model_name, + ) + end + + def matcher_from(scenario) + scenario.matcher.tap do |matcher| + if respond_to?(:configure_validation_matcher) + configure_validation_matcher(matcher) + end + end + end +end diff --git a/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb index 322a9dd9..3c252e26 100644 --- a/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb @@ -435,7 +435,7 @@ invalid and to raise a validation exception with message matching it 'raises an AttributeChangedValueError' do model = define_active_model_class 'Example', accessors: [:name] do def name=(value) - unless value.nil? + if value super(value) end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_absence_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_absence_of_matcher_spec.rb index 972ace3a..c277bc2c 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_absence_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_absence_of_matcher_spec.rb @@ -32,8 +32,28 @@ describe Shoulda::Matchers::ActiveModel::ValidateAbsenceOfMatcher, type: :model expect(validating_absence_of(:attr, {}, type: type)). to validate_absence_of(:attr) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value + }, + } + ) + + define_method(:validation_matcher_scenario_args) do |*args| + super(*args).deep_merge(column_type: type) + end end end + + def validation_matcher_scenario_args + super.deep_merge(model_creator: :active_record) + end end context 'a model without an absence validation' do @@ -62,6 +82,22 @@ Example did not properly validate that :attr is empty/falsy. it 'does not override the default message with a blank' do expect(active_model_validating_absence_of(:attr)).to validate_absence_of(:attr).with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :upcase + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :upcase + }, + } + ) + + def validation_matcher_scenario_args + super.deep_merge(model_creator: :active_model) + end end context 'an ActiveModel class without an absence validation' do @@ -73,7 +109,7 @@ Example did not properly validate that :attr is empty/falsy. MESSAGE assertion = lambda do - expect(active_model_with(:attr)).to validate_absence_of(:attr) + expect(active_model_with(:attr)).to validate_absence_of(:attr) end expect(&assertion).to fail_with_message(message) @@ -84,6 +120,22 @@ Example did not properly validate that :attr is empty/falsy. it 'requires the attribute to not be set' do expect(having_many(:children, absence: true)).to validate_absence_of(:children) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value + }, + } + ) + + def validation_matcher_scenario_args + super.deep_merge(model_creator: :"active_record/has_many") + end end context 'a has_many association without an absence validation' do @@ -98,6 +150,22 @@ Example did not properly validate that :attr is empty/falsy. model = having_and_belonging_to_many(:children, absence: true) expect(model).to validate_absence_of(:children) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value + }, + } + ) + + def validation_matcher_scenario_args + super.deep_merge(model_creator: :"active_record/habtm") + end end context 'a non-absent has_and_belongs_to_many association' do @@ -145,15 +213,29 @@ Parent did not properly validate that :children is empty/falsy. end end - def validating_absence_of(attr, validation_options = {}, given_column_options = {}) + def define_model_validating_absence_of(attr, validation_options = {}, given_column_options = {}) default_column_options = { type: :string, options: {} } column_options = default_column_options.merge(given_column_options) - define_model :example, attr => column_options do - validates_absence_of attr, validation_options - end.new + define_model :example, attr => column_options do |model| + model.validates_absence_of(attr, validation_options) + + if block_given? + yield model + end + end end + def validating_absence_of(attr, validation_options = {}, given_column_options = {}) + model = define_model_validating_absence_of( + attr, + validation_options, + given_column_options + ) + model.new + end + alias_method :build_record_validating_absence_of, :validating_absence_of + def active_model_with(attr, &block) define_active_model_class('Example', accessors: [attr], &block).new end @@ -188,5 +270,9 @@ Parent did not properly validate that :children is empty/falsy. end end.new end + + def validation_matcher_scenario_args + super.deep_merge(matcher_name: :validate_absence_of) + end end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_acceptance_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_acceptance_of_matcher_spec.rb index 467ec719..6fa150ca 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_acceptance_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_acceptance_of_matcher_spec.rb @@ -9,6 +9,35 @@ describe Shoulda::Matchers::ActiveModel::ValidateAcceptanceOfMatcher, type: :mod it 'does not overwrite the default message with nil' do expect(record_validating_acceptance).to matcher.with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :never_falsy, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :never_falsy, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :always_nil, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr has been set to "1". + After setting :attr to ‹false› -- which was read back as ‹nil› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + }, + }, + model_creator: :active_model + ) end context 'a model without an acceptance validation' do @@ -33,8 +62,9 @@ describe Shoulda::Matchers::ActiveModel::ValidateAcceptanceOfMatcher, type: :mod validate_acceptance_of(:attr) end - def model_validating_nothing(&block) - define_active_model_class(:example, accessors: [:attr], &block) + def model_validating_nothing(options = {}, &block) + attribute_name = options.fetch(:attribute_name, :attr) + define_active_model_class(:example, accessors: [attribute_name], &block) end def record_validating_nothing @@ -42,12 +72,23 @@ describe Shoulda::Matchers::ActiveModel::ValidateAcceptanceOfMatcher, type: :mod end def model_validating_acceptance(options = {}) - model_validating_nothing do - validates_acceptance_of :attr, options + attribute_name = options.fetch(:attribute_name, :attr) + + model_validating_nothing(attribute_name: attribute_name) do + validates_acceptance_of attribute_name, options end end + alias_method :define_model_validating_acceptance, :model_validating_acceptance + def record_validating_acceptance(options = {}) model_validating_acceptance(options).new end + + alias_method :build_record_validating_acceptance, + :record_validating_acceptance + + def validation_matcher_scenario_args + { matcher_name: :validate_acceptance_of } + end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_confirmation_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_confirmation_of_matcher_spec.rb index 23449fae..7859bfa2 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_confirmation_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_confirmation_of_matcher_spec.rb @@ -27,6 +27,37 @@ describe Shoulda::Matchers::ActiveModel::ValidateConfirmationOfMatcher, type: :m with_message(nil) end end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :password, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :password_confirmation matches +:password. + After setting :password_confirmation to ‹"same value"›, then setting + :password to ‹"same value"› -- which was read back as ‹"same valuf"› + -- the matcher expected the Example to be valid, but it was invalid + instead, producing these validation errors: + + * password_confirmation: ["doesn't match Password"] + + As indicated in the message above, :password seems to be changing + certain values as they are set, and this could have something to do + with why this test is failing. If you've overridden the writer method + for this attribute, then you may need to change it to make this test + pass, or do something else entirely. + MESSAGE + }, + }, + model_creator: :active_model + ) end context 'when the model does not have a confirmation attribute' do @@ -114,4 +145,8 @@ Example did not properly validate that to validate_confirmation_of(builder.attribute_to_confirm) end end + + def validation_matcher_scenario_args + super.deep_merge(matcher_name: :validate_confirmation_of) + end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_exclusion_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_exclusion_of_matcher_spec.rb index 24c6fdbf..a39e0b85 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_exclusion_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_exclusion_of_matcher_spec.rb @@ -16,6 +16,44 @@ describe Shoulda::Matchers::ActiveModel::ValidateExclusionOfMatcher, type: :mode expect(validating_exclusion(in: 2..5)). to validate_exclusion_of(:attr).in_range(2..5).with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr lies outside the range ‹2› +to ‹5›. + After setting :attr to ‹1› -- which was read back as ‹2› -- the + matcher expected the Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["is reserved"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + }, + }, + model_creator: :active_model + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { in: 2..5 }) + end + + def configure_validation_matcher(matcher) + matcher.in_range(2..5) + end + end end context 'an attribute which must be excluded from a range with excluded end' do @@ -110,16 +148,66 @@ describe Shoulda::Matchers::ActiveModel::ValidateExclusionOfMatcher, type: :mode end end - def validating_exclusion(options) - define_model(:example, attr: :string) do - validates_exclusion_of :attr, options - end.new + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr is neither ‹"one"› nor +‹"two"›. + After setting :attr to ‹"one"› -- which was read back as ‹"onf"› -- + the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + }, + }, + model_creator: :active_model + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { in: ['one', 'two'] }) + end + + def configure_validation_matcher(matcher) + matcher.in_array(['one', 'two']) + end + end + + def define_model_validating_exclusion(options) + options = options.dup + column_type = options.delete(:column_type) { :string } + super options.merge(column_type: column_type) + end + end + + def define_model_validating_exclusion(options) + options = options.dup + attribute_name = options.delete(:attribute_name) { :attr } + column_type = options.delete(:column_type) { :integer } + + define_model(:example, attribute_name => column_type) do |model| + model.validates_exclusion_of(attribute_name, options) end end def validating_exclusion(options) - define_model(:example, attr: :integer) do - validates_exclusion_of :attr, options - end.new + define_model_validating_exclusion(options).new + end + + alias_method :build_record_validating_exclusion, :validating_exclusion + + def validation_matcher_scenario_args + super.deep_merge(matcher_name: :validate_exclusion_of) end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb index 2b06c863..835215d7 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_inclusion_of_matcher_spec.rb @@ -18,7 +18,6 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode matches_or_not.reverse! to_or_not_to.reverse! end - end end @@ -49,6 +48,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :integer, default_value: 1) + end end context 'against an attribute with a specific column limit' do @@ -95,6 +98,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :float, default_value: 1.0) + end end context 'against a decimal attribute' do @@ -119,11 +126,20 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge( + column_type: :decimal, + default_value: BigDecimal.new('1.0') + ) + end end context 'against a date attribute' do today = Date.today + define_method(:today) { today } + it_behaves_like 'it supports in_array', possible_values: (1..5).map { |n| today + n }, reserved_outside_value: described_class::ARBITRARY_OUTSIDE_DATE @@ -141,11 +157,17 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :date, default_value: today) + end end context 'against a datetime attribute' do now = DateTime.now + define_method(:now) { now } + it_behaves_like 'it supports in_array', possible_values: (1..5).map { |n| now + n }, reserved_outside_value: described_class::ARBITRARY_OUTSIDE_DATETIME @@ -163,11 +185,17 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :datetime, default_value: now) + end end context 'against a time attribute' do now = Time.now + define_method(:now) { now } + it_behaves_like 'it supports in_array', possible_values: (1..5).map { |n| now + n }, reserved_outside_value: described_class::ARBITRARY_OUTSIDE_TIME @@ -185,6 +213,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :time, default_value: now) + end end context 'against a string attribute' do @@ -202,6 +234,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + %w(qux) end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :string) + end end end @@ -210,7 +246,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode testing_values_of_option 'allow_nil' do |option_args, matches_or_not, to_or_not_to| it "#{matches_or_not[0]} when the validation specifies allow_nil" do - builder = build_object_allowing(valid_values, allow_nil: true) + builder = build_object_allowing( + valid_values, + validation_options: { allow_nil: true } + ) __send__("expect_#{to_or_not_to[0]}_match_on_values", builder, valid_values) do |matcher| matcher.allow_nil(*option_args) @@ -225,6 +264,39 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end end + +=begin + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: -> (value) { value || valid_values.first } + }, + reject_if_qualified_but_changing_value_interferes: { + attribute_name: :attr, + changing_values_with: :never_falsy, + expected_message_includes: <<-MESSAGE.strip + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) +=end + + def validation_matcher_scenario_args + super.deep_merge(validation_options: { allow_nil: true }) + end + + def configure_validation_matcher(matcher) + super(matcher).allow_nil + end end shared_examples_for 'it supports allow_blank' do |args| @@ -232,7 +304,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode testing_values_of_option 'allow_blank' do |option_args, matches_or_not, to_or_not_to| it "#{matches_or_not[0]} when the validation specifies allow_blank" do - builder = build_object_allowing(valid_values, allow_blank: true) + builder = build_object_allowing( + valid_values, + validation_options: { allow_blank: true } + ) __send__("expect_#{to_or_not_to[0]}_match_on_values", builder, valid_values) do |matcher| matcher.allow_blank(*option_args) @@ -247,6 +322,41 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end end + +=begin + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: -> (value) { + value.presence || valid_values.first + } + }, + reject_if_qualified_but_changing_value_interferes: { + attribute_name: :attr, + changing_values_with: :never_blank, + expected_message_includes: <<-MESSAGE.strip + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) +=end + + def validation_matcher_scenario_args + super.deep_merge(validation_options: { allow_blank: true }) + end + + def configure_validation_matcher(matcher) + super(matcher).allow_blank + end end shared_examples_for 'it supports with_message' do |args| @@ -254,7 +364,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode context 'given a string' do it 'matches when validation uses given message' do - builder = build_object_allowing(valid_values, message: 'a message') + builder = build_object_allowing( + valid_values, + validation_options: { message: 'a message' } + ) expect_to_match_on_values(builder, valid_values) do |matcher| matcher.with_message('a message') @@ -270,7 +383,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end it 'does not match when validation uses a message but it is not same as given' do - builder = build_object_allowing(valid_values, message: 'a different message') + builder = build_object_allowing( + valid_values, + validation_options: { message: 'a different message' } + ) expect_not_to_match_on_values(builder, valid_values) do |matcher| matcher.with_message('a message') @@ -280,7 +396,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode context 'given a regex' do it 'matches when validation uses a message that matches the regex' do - builder = build_object_allowing(valid_values, message: 'this is a message') + builder = build_object_allowing( + valid_values, + validation_options: { message: 'this is a message' } + ) expect_to_match_on_values(builder, valid_values) do |matcher| matcher.with_message(/a message/) @@ -296,7 +415,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end it 'does not match when validation uses a message but it does not match regex' do - builder = build_object_allowing(valid_values, message: 'a different message') + builder = build_object_allowing( + valid_values, + validation_options: { message: 'a different message' } + ) expect_not_to_match_on_values(builder, valid_values) do |matcher| matcher.with_message(/a message/) @@ -320,6 +442,8 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode zero = args[:zero] reserved_outside_value = args.fetch(:reserved_outside_value) + define_method(:valid_values) { args.fetch(:possible_values) } + it 'does not match a record with no validations' do builder = build_object expect_not_to_match_on_values(builder, possible_values) @@ -364,7 +488,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode context '+ strict' do context 'when the validation specifies strict' do it 'matches when the given values match the valid values' do - builder = build_object_allowing(possible_values, strict: true) + builder = build_object_allowing( + possible_values, + validation_options: { strict: true } + ) expect_to_match_on_values(builder, possible_values) do |matcher| matcher.strict @@ -372,7 +499,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end it 'does not match when the given values do not match the valid values' do - builder = build_object_allowing(possible_values, strict: true) + builder = build_object_allowing( + possible_values, + validation_options: { strict: true } + ) values = add_outside_value_to(possible_values) expect_not_to_match_on_values(builder, values) do |matcher| @@ -393,6 +523,26 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + attribute_name: :attr, + changing_values_with: :next_value, + expected_message_includes: <<-MESSAGE.strip + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + def expect_to_match_on_values(builder, values, &block) expect_to_match_in_array(builder, values, &block) end @@ -400,11 +550,21 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def expect_not_to_match_on_values(builder, values, &block) expect_not_to_match_in_array(builder, values, &block) end + + def validation_matcher_scenario_args + super.deep_merge(validation_options: { in: valid_values }) + end + + def configure_validation_matcher(matcher) + super(matcher).in_array(valid_values) + end end shared_examples_for 'it supports in_range' do |args| possible_values = args[:possible_values] + define_method(:valid_values) { args.fetch(:possible_values) } + it 'does not match a record with no validations' do builder = build_object expect_not_to_match_on_values(builder, possible_values) @@ -451,7 +611,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode context '+ strict' do context 'when the validation specifies strict' do it 'matches when the given range matches the range in the validation' do - builder = build_object_allowing(possible_values, strict: true) + builder = build_object_allowing( + possible_values, + validation_options: { strict: true } + ) expect_to_match_on_values(builder, possible_values) do |matcher| matcher.strict @@ -459,7 +622,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end it 'matches when the given range does not match the range in the validation' do - builder = build_object_allowing(possible_values, strict: true) + builder = build_object_allowing( + possible_values, + validation_options: { strict: true } + ) range = Range.new(possible_values.first, possible_values.last + 1) expect_not_to_match_on_values(builder, range) do |matcher| @@ -480,6 +646,26 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + attribute_name: :attr, + changing_values_with: :next_value, + expected_message_includes: <<-MESSAGE.strip + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + def expect_to_match_on_values(builder, range, &block) expect_to_match_in_range(builder, range, &block) end @@ -487,6 +673,14 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def expect_not_to_match_on_values(builder, range, &block) expect_not_to_match_in_range(builder, range, &block) end + + def validation_matcher_scenario_args + super.deep_merge(validation_options: { in: valid_values }) + end + + def configure_validation_matcher(matcher) + super(matcher).in_range(valid_values) + end end shared_context 'against a boolean attribute for true and false' do @@ -537,6 +731,8 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode context 'against a timestamp column' do now = DateTime.now + define_method(:now) { now } + it_behaves_like 'it supports in_array', possible_values: (1..5).map { |n| now + n }, reserved_outside_value: described_class::ARBITRARY_OUTSIDE_DATETIME @@ -554,6 +750,10 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode def add_outside_value_to(values) values + [values.last + 1] end + + def validation_matcher_scenario_args + super.deep_merge(column_type: :timestamp, default_value: now) + end end context 'against a boolean attribute' do @@ -615,27 +815,12 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end - def build_object_with_generic_attribute(options = {}, &block) - attribute_name = :attr - column_type = options.fetch(:column_type) - column_options = { - type: column_type, - options: options.fetch(:column_options, {}) - } - validation_options = options[:validation_options] - custom_validation = options[:custom_validation] + def define_simple_model(attribute_name: :attr, column_options: {}, &block) + define_model('Example', attribute_name => column_options, &block) + end - model = define_model :example, attribute_name => column_options - customize_model_class( - model, - attribute_name, - validation_options, - custom_validation - ) - - object = model.new - - object_builder_class.new(attribute_name, object, validation_options) + def validation_matcher_scenario_args + super.deep_merge(model_creator: :active_record) end end @@ -658,24 +843,12 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode end end - def build_object_with_generic_attribute(options = {}, &block) - attribute_name = :attr - validation_options = options[:validation_options] - custom_validation = options[:custom_validation] - value = options[:value] + def define_simple_model(attribute_name: :attr, column_options: {}, &block) + define_active_model_class('Example', accessors: [attribute_name], &block) + end - model = define_active_model_class :example, accessors: [attribute_name] - customize_model_class( - model, - attribute_name, - validation_options, - custom_validation - ) - - object = model.new - object.__send__("#{attribute_name}=", value) - - object_builder_class.new(attribute_name, object, validation_options) + def validation_matcher_scenario_args + super.deep_merge(model_creator: :active_model) end end @@ -727,24 +900,63 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode @_object_builder_class ||= Struct.new(:attribute, :object, :validation_options) end - def customize_model_class(klass, attribute_name, validation_options, custom_validation) - klass.class_eval do + def build_object_with_generic_attribute( + attribute_name: :attr, + validation_options: nil, + value: nil, + **other_options + ) + model = define_model_validating_inclusion( + attribute_name: attribute_name, + validation_options: validation_options, + **other_options + ) + + object = model.new + object.__send__("#{attribute_name}=", value) + + object_builder_class.new(attribute_name, object, validation_options) + end + + def define_model_validating_inclusion( + attribute_name: :attr, + column_type: :string, + column_options: {}, + validation_options: nil, + custom_validation: nil, + customize_model_class: -> (object) { } + ) + column_options = { type: column_type, options: column_options } + + define_simple_model( + attribute_name: attribute_name, + column_options: column_options + ) do |model| if validation_options - validates_inclusion_of attribute_name, validation_options + model.validates_inclusion_of(attribute_name, validation_options) end if custom_validation - define_method :custom_validation do - instance_exec(attribute_name, &custom_validation) - end + model.class_eval do + define_method :custom_validation do + custom_validation.call(self, attribute_name) + end - validate :custom_validation + validate :custom_validation + end + end + + if customize_model_class + model.instance_eval(&customize_model_class) end end end - def build_object_allowing(values, options = {}) - build_object(validation_options: options.merge(in: values)) + def build_object_allowing(values, validation_options: {}, **other_options) + build_object( + validation_options: validation_options.merge(in: values), + **other_options + ) end def expect_to_match(builder) @@ -791,13 +1003,13 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode low_message = 'too low' high_message = 'too high' - builder = build_object custom_validation: ->(attribute) { - value = __send__(attribute) + builder = build_object custom_validation: -> (object, attribute) { + value = object.public_send(attribute) if value < low_value - errors.add(attribute, low_message) + object.errors.add(attribute, low_message) elsif value > high_value - errors.add(attribute, high_message) + object.errors.add(attribute, high_message) end } @@ -808,4 +1020,8 @@ describe Shoulda::Matchers::ActiveModel::ValidateInclusionOfMatcher, type: :mode with_high_message(high_message) end end + + def validation_matcher_scenario_args + super.deep_merge(matcher_name: :validate_inclusion_of) + end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_length_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_length_of_matcher_spec.rb index f7330d35..65430616 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_length_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_length_of_matcher_spec.rb @@ -21,6 +21,44 @@ describe Shoulda::Matchers::ActiveModel::ValidateLengthOfMatcher, type: :model d expect(validating_length(minimum: 4)). to validate_length_of(:attr).is_at_least(4).with_short_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :upcase, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :upcase, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :add_character, + expected_message: <<-MESSAGE.strip +Example did not properly validate that the length of :attr is at least +4. + After setting :attr to ‹"xxx"› -- which was read back as ‹"xxxa"› -- + the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { minimum: 4 }) + end + + def configure_validation_matcher(matcher) + matcher.is_at_least(4) + end + end end context 'an attribute with a minimum length validation of 0' do @@ -50,6 +88,43 @@ describe Shoulda::Matchers::ActiveModel::ValidateLengthOfMatcher, type: :model d expect(validating_length(maximum: 4)). to validate_length_of(:attr).is_at_most(4).with_long_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :upcase, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :upcase, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :remove_character, + expected_message: <<-MESSAGE.strip +Example did not properly validate that the length of :attr is at most 4. + After setting :attr to ‹"xxxxx"› -- which was read back as ‹"xxxx"› -- + the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { maximum: 4 }) + end + + def configure_validation_matcher(matcher) + matcher.is_at_most(4) + end + end end context 'an attribute with a required exact length' do @@ -72,6 +147,43 @@ describe Shoulda::Matchers::ActiveModel::ValidateLengthOfMatcher, type: :model d expect(validating_length(is: 4)). to validate_length_of(:attr).is_equal_to(4).with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :upcase, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :upcase, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :add_character, + expected_message: <<-MESSAGE.strip +Example did not properly validate that the length of :attr is 4. + After setting :attr to ‹"xxx"› -- which was read back as ‹"xxxa"› -- + the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { is: 4 }) + end + + def configure_validation_matcher(matcher) + matcher.is_equal_to(4) + end + end end context 'an attribute with a required exact length and another validation' do @@ -161,9 +273,25 @@ describe Shoulda::Matchers::ActiveModel::ValidateLengthOfMatcher, type: :model d end end + def define_model_validating_length(options = {}) + options = options.dup + attribute_name = options.delete(:attribute_name) { :attr } + + define_model(:example, attribute_name => :string) do |model| + model.validates_length_of(attribute_name, options) + end + end + def validating_length(options = {}) - define_model(:example, attr: :string) do - validates_length_of :attr, options - end.new + define_model_validating_length(options).new + end + + alias_method :build_record_validating_length, :validating_length + + def validation_matcher_scenario_args + super.deep_merge( + matcher_name: :validate_length_of, + model_creator: :active_model + ) end end 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 ea506bff..c7ba5bed 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 @@ -127,6 +127,34 @@ describe Shoulda::Matchers::ActiveModel::ValidateNumericalityOfMatcher, type: :m expect(record).to validate_numericality end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :numeric_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number. + After setting :attr to ‹"abcd"› -- which was read back as ‹"1"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + context 'when the column is an integer column' do it 'raises an IneffectiveTestError' do record = build_record_validating_numericality( @@ -187,6 +215,47 @@ Example did not properly validate that :attr looks like a number. record = build_record_validating_numericality(allow_nil: true) expect(record).to validate_numericality.allow_nil end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value_or_numeric_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value_or_numeric_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value_or_non_numeric_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number, but +only if it is not nil. + In checking that Example allows :attr to be ‹nil›, after setting :attr + to ‹nil› -- which was read back as ‹"a"› -- the matcher expected the + Example to be valid, but it was invalid instead, producing these + validation errors: + + * attr: ["is not a number"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { allow_nil: true }) + end + + def configure_validation_matcher(matcher) + matcher.allow_nil + end + end end context 'and not validating with allow_nil' do @@ -218,6 +287,42 @@ only if it is not nil. record = build_record_validating_numericality(only_integer: true) expect(record).to validate_numericality.only_integer end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :numeric_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like an integer. + After setting :attr to ‹"0.1"› -- which was read back as ‹"1"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { only_integer: true }) + end + + def configure_validation_matcher(matcher) + matcher.only_integer + end + end end context 'and not validating with only_integer' do @@ -246,6 +351,42 @@ Example did not properly validate that :attr looks like an integer. expect(record).to validate_numericality.odd end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like an odd number. + After setting :attr to ‹"2"› -- which was read back as ‹"3"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { odd: true }) + end + + def configure_validation_matcher(matcher) + matcher.odd + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -306,6 +447,42 @@ Example did not properly validate that :attr looks like an odd number. expect(record).to validate_numericality.even end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like an even number. + After setting :attr to ‹"1"› -- which was read back as ‹"2"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { even: true }) + end + + def configure_validation_matcher(matcher) + matcher.even + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -368,6 +545,45 @@ Example did not properly validate that :attr looks like an even number. expect(record).to validate_numericality.is_less_than_or_equal_to(18) end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number less +than or equal to 18. + After setting :attr to ‹"18"› -- which was read back as ‹"19"› -- the + matcher expected the Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["must be less than or equal to 18"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge( + validation_options: { less_than_or_equal_to: 18 } + ) + end + + def configure_validation_matcher(matcher) + matcher.is_less_than_or_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -431,6 +647,43 @@ than or equal to 18. is_less_than(18) end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number less +than 18. + After setting :attr to ‹"17"› -- which was read back as ‹"18"› -- the + matcher expected the Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["must be less than 18"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { less_than: 18 }) + end + + def configure_validation_matcher(matcher) + matcher.is_less_than(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -492,6 +745,43 @@ than 18. expect(record).to validate_numericality.is_equal_to(18) end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number equal +to 18. + After setting :attr to ‹"18"› -- which was read back as ‹"19"› -- the + matcher expected the Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["must be equal to 18"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { equal_to: 18 }) + end + + def configure_validation_matcher(matcher) + matcher.is_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -557,6 +847,42 @@ to 18. is_greater_than_or_equal_to(18) end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number greater +than or equal to 18. + After setting :attr to ‹"17"› -- which was read back as ‹"18"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge( + validation_options: { greater_than_or_equal_to: 18 } + ) + end + + def configure_validation_matcher(matcher) + matcher.is_greater_than_or_equal_to(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -627,6 +953,40 @@ than or equal to 18. is_greater_than(18) end + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number greater +than 18. + After setting :attr to ‹"18"› -- which was read back as ‹"19"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) do + def validation_matcher_scenario_args + super.deep_merge(validation_options: { greater_than: 18 }) + end + + def configure_validation_matcher(matcher) + matcher.is_greater_than(18) + end + end + context 'when the column is an integer column' do it 'accepts (and does not raise an error)' do record = build_record_validating_numericality( @@ -1252,6 +1612,38 @@ greater than 18 and less than 99. expect(model.new).to validate_numericality_of(:attr) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :next_value, + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :numeric_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr looks like a number. + After setting :attr to ‹"abcd"› -- which was read back as ‹"1"› -- the + matcher expected the Example to be invalid, but it was valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + + def validation_matcher_scenario_args + super.deep_merge(model_creator: :active_model) + end end describe '#description' do @@ -1390,4 +1782,11 @@ greater than 18 and less than 99. def attribute_name :attr end + + def validation_matcher_scenario_args + super.deep_merge( + matcher_name: :validate_numericality_of, + model_creator: :active_record + ) + end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb index babe711b..8c2fbded 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb @@ -9,6 +9,35 @@ describe Shoulda::Matchers::ActiveModel::ValidatePresenceOfMatcher, type: :model it 'does not override the default message with a blank' do expect(validating_presence).to matcher.with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :never_falsy + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :nil_to_blank + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :never_falsy, + expected_message: <<-MESSAGE +Example did not properly validate that :attr cannot be empty/falsy. + After setting :attr to ‹nil› -- which was read back as ‹"dummy value"› + -- the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) end context 'a model without a presence validation' do @@ -37,6 +66,39 @@ Example did not properly validate that :attr cannot be empty/falsy. it 'does not override the default message with a blank' do expect(active_model_validating_presence).to matcher.with_message(nil) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :never_falsy + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :nil_to_blank + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :never_falsy, + expected_message: <<-MESSAGE +Example did not properly validate that :attr cannot be empty/falsy. + After setting :attr to ‹nil› -- which was read back as ‹"dummy value"› + -- the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + + def model_creator + :active_model + end end context 'an ActiveModel class without a presence validation' do @@ -59,6 +121,39 @@ Example did not properly validate that :attr cannot be empty/falsy. it 'requires the attribute to be set' do expect(has_many_children(presence: true)).to validate_presence_of(:children) end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :never_falsy + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :nil_to_blank + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :never_falsy, + expected_message: <<-MESSAGE +Example did not properly validate that :attr cannot be empty/falsy. + After setting :attr to ‹nil› -- which was read back as ‹"dummy value"› + -- the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + + def model_creator + :"active_record/has_many" + end end context 'a has_many association without a presence validation' do @@ -69,20 +164,56 @@ Example did not properly validate that :attr cannot be empty/falsy. end context 'a required has_and_belongs_to_many association' do - before do - define_model :child - @model = define_model :parent do - has_and_belongs_to_many :children - validates_presence_of :children - end.new + it 'accepts' do + expect(build_record_having_and_belonging_to_many). + to validate_presence_of(:children) + end + + def build_record_having_and_belonging_to_many create_table 'children_parents', id: false do |t| t.integer :child_id t.integer :parent_id end + + define_model :child + + define_model :parent do + has_and_belongs_to_many :children + validates_presence_of :children + end.new end - it 'accepts' do - expect(@model).to validate_presence_of(:children) + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + changing_values_with: :never_falsy + }, + accept_if_qualified_but_changing_value_does_not_interfere: { + changing_values_with: :nil_to_blank + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :never_falsy, + expected_message: <<-MESSAGE +Example did not properly validate that :attr cannot be empty/falsy. + After setting :attr to ‹nil› -- which was read back as ‹"dummy value"› + -- the matcher expected the Example to be invalid, but it was valid + instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + + def model_creator + :"active_record/has_and_belongs_to_many" end end @@ -204,13 +335,13 @@ raising a validation exception on failure. end end - context 'when the attribute typecasts nil to an empty array' do - it 'accepts' do - model = define_active_model_class :example do - attr_reader :foo + context 'when the attribute typecasts nil to another blank value, such as an empty array' do + it 'accepts (and does not raise an AttributeChangedValueError)' do + model = define_active_model_class :example, accessors: [:foo] do + validates_presence_of :foo def foo=(value) - @foo = Array.wrap(value) + super(Array.wrap(value)) end end @@ -251,4 +382,11 @@ raising a validation exception on failure. validates_presence_of :attr end.new end + + def validation_matcher_scenario_args + super.deep_merge( + matcher_name: :validate_presence_of, + model_creator: :active_record + ) + end end diff --git a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb index 46e6b313..76718699 100644 --- a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb @@ -182,7 +182,7 @@ Example did not properly validate that :attr is case-sensitively unique. end context 'when a non-existent attribute is specified as a scope' do - context 'when there is one scope' do + context 'when there is more than one scope' do it 'rejects with an appropriate failure message (and does not raise an error)' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope) ] @@ -192,7 +192,7 @@ Example did not properly validate that :attr is case-sensitively unique. expect(record).to validate_uniqueness.scoped_to(:non_existent) end - message = <<-MESSAGE + message = <<-MESSAGE.strip Example did not properly validate that :attr is case-sensitively unique within the scope of :non_existent. :non_existent does not seem to be an attribute on Example. @@ -202,7 +202,7 @@ within the scope of :non_existent. end end - context 'when there is more than scope' do + context 'when there is more than one scope' do it 'rejects with an appropriate failure message (and does not raise an error)' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope) ] @@ -215,7 +215,7 @@ within the scope of :non_existent. ) end - message = <<-MESSAGE + message = <<-MESSAGE.strip Example did not properly validate that :attr is case-sensitively unique within the scope of :non_existent1 and :non_existent2. :non_existent1 and :non_existent2 do not seem to be attributes on @@ -228,10 +228,10 @@ within the scope of :non_existent1 and :non_existent2. end define_method(:build_attribute) do |attribute_options| - attribute_options.merge( + attribute_options.deep_merge( column_type: column_type, value_type: value_type, - array: array + options: { array: array } ) end end @@ -338,6 +338,7 @@ within the scope of :other. context 'when no message is specified' do it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( + attribute_value: 'some value', validation_options: { message: 'bad value' } ) @@ -347,12 +348,12 @@ within the scope of :other. message = <<-MESSAGE Example did not properly validate that :attr is case-sensitively unique. - After taking the given Example, setting its :attr to ‹"an arbitrary - value"›, and saving it as the existing record, then making a new - Example and setting its :attr to ‹"an arbitrary value"› as well, the - matcher expected the new Example to be invalid and to produce the - validation error "has already been taken" on :attr. The record was - indeed invalid, but it produced these validation errors instead: + After taking the given Example, whose :attr is ‹"some value"›, and + saving it as the existing record, then making a new Example and + setting its :attr to ‹"some value"› as well, the matcher expected the + new Example to be invalid and to produce the validation error "has + already been taken" on :attr. The record was indeed invalid, but it + produced these validation errors instead: * attr: ["bad value"] MESSAGE @@ -365,6 +366,7 @@ Example did not properly validate that :attr is case-sensitively unique. context 'when the given and actual messages do not match' do it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( + attribute_value: 'some value', validation_options: { message: 'something else entirely' } ) @@ -377,12 +379,12 @@ Example did not properly validate that :attr is case-sensitively unique. message = <<-MESSAGE Example did not properly validate that :attr is case-sensitively unique, producing a custom validation error on failure. - After taking the given Example, setting its :attr to ‹"an arbitrary - value"›, and saving it as the existing record, then making a new - Example and setting its :attr to ‹"an arbitrary value"› as well, the - matcher expected the new Example to be invalid and to produce the - validation error "some message" on :attr. The record was indeed - invalid, but it produced these validation errors instead: + After taking the given Example, whose :attr is ‹"some value"›, and + saving it as the existing record, then making a new Example and + setting its :attr to ‹"some value"› as well, the matcher expected the + new Example to be invalid and to produce the validation error "some + message" on :attr. The record was indeed invalid, but it produced + these validation errors instead: * attr: ["something else entirely"] MESSAGE @@ -407,6 +409,7 @@ producing a custom validation error on failure. context 'when the given and actual messages do not match' do it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( + attribute_value: 'some value', validation_options: { message: 'something else entirely' } ) @@ -419,12 +422,12 @@ producing a custom validation error on failure. message = <<-MESSAGE Example did not properly validate that :attr is case-sensitively unique, producing a custom validation error on failure. - After taking the given Example, setting its :attr to ‹"an arbitrary - value"›, and saving it as the existing record, then making a new - Example and setting its :attr to ‹"an arbitrary value"› as well, the - matcher expected the new Example to be invalid and to produce a - validation error matching ‹/some message/› on :attr. The record was - indeed invalid, but it produced these validation errors instead: + After taking the given Example, whose :attr is ‹"some value"›, and + saving it as the existing record, then making a new Example and + setting its :attr to ‹"some value"› as well, the matcher expected the + new Example to be invalid and to produce a validation error matching + ‹/some message/› on :attr. The record was indeed invalid, but it + produced these validation errors instead: * attr: ["something else entirely"] MESSAGE @@ -445,6 +448,36 @@ producing a custom validation error on failure. end end end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + attribute_name: :attr, + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + default_value: 'some value', + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr is case-sensitively unique. + After taking the given Example, whose :attr is ‹"some valuf"›, and + saving it as the existing record, then making a new Example and + setting its :attr to ‹"some valuf"› (read back as ‹"some valug"›) as + well, the matcher expected the new Example to be invalid, but it was + valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) end context 'when the model has a scoped uniqueness validation' do @@ -662,6 +695,7 @@ within the scope of :scope1. it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( attribute_type: :string, + attribute_value: 'some value', validation_options: { case_sensitive: true } ) @@ -672,11 +706,10 @@ within the scope of :scope1. message = <<-MESSAGE Example did not properly validate that :attr is case-insensitively unique. - After taking the given Example, setting its :attr to ‹"an arbitrary - value"›, and saving it as the existing record, then making a new - Example and setting its :attr to a different value, ‹"AN ARBITRARY - VALUE"›, the matcher expected the new Example to be invalid, but it - was valid instead. + After taking the given Example, whose :attr is ‹"some value"›, and + saving it as the existing record, then making a new Example and + setting its :attr to a different value, ‹"SOME VALUE"›, the matcher + expected the new Example to be invalid, but it was valid instead. MESSAGE expect(&assertion).to fail_with_message(message) @@ -720,6 +753,45 @@ Example did not properly validate that :attr is case-sensitively unique. expect(record).to validate_uniqueness.case_insensitive end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + raise_if_not_qualified: { + attribute_name: :attr, + changing_values_with: :next_value, + }, + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + default_value: 'some value', + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip +Example did not properly validate that :attr is case-insensitively +unique. + After taking the given Example, whose :attr is ‹"some valuf"›, and + saving it as the existing record, then making a new Example and + setting its :attr to ‹"some valuf"› (read back as ‹"some valug"›) as + well, the matcher expected the new Example to be invalid, but it was + valid instead. + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + } + } + ) + + def validation_matcher_scenario_args + super.deep_merge(validation_options: { case_sensitive: false }) + end + + def configure_validation_matcher(matcher) + super(matcher).case_insensitive + end end end @@ -799,10 +871,10 @@ but only if it is not nil. message = <<-MESSAGE Example did not properly validate that :attr is case-sensitively unique, but only if it is not nil. - Given an existing Example whose :attr is ‹nil›, after making a new - Example and setting its :attr to ‹nil› as well, the matcher expected - the new Example to be valid, but it was invalid instead, producing - these validation errors: + Given an existing Example, after setting its :attr to ‹nil›, then + making a new Example and setting its :attr to ‹nil› as well, the + matcher expected the new Example to be valid, but it was invalid + instead, producing these validation errors: * attr: ["has already been taken"] MESSAGE @@ -980,10 +1052,10 @@ but only if it is not blank. message = <<-MESSAGE Example did not properly validate that :attr is case-sensitively unique, but only if it is not blank. - Given an existing Example whose :attr is ‹""›, after making a new - Example and setting its :attr to ‹""› as well, the matcher expected - the new Example to be valid, but it was invalid instead, producing - these validation errors: + Given an existing Example, after setting its :attr to ‹""›, then + making a new Example and setting its :attr to ‹""› as well, the + matcher expected the new Example to be valid, but it was invalid + instead, producing these validation errors: * attr: ["has already been taken"] MESSAGE @@ -1043,26 +1115,45 @@ but only if it is not blank. end end + context 'when the model does not have the attribute being tested' do + it 'fails with an appropriate failure message' do + model = define_model(:example) + + assertion = lambda do + expect(model.new).to validate_uniqueness_of(:attr) + end + + message = <<-MESSAGE.strip +Example did not properly validate that :attr is case-sensitively unique. + :attr does not seem to be an attribute on Example. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end + end + let(:model_attributes) { {} } def default_attribute { value_type: :string, column_type: :string, - array: false + options: { array: false, null: true } } end def normalize_attribute(attribute) if attribute.is_a?(Hash) - if attribute.key?(:type) - attribute[:value_type] = attribute[:type] - attribute[:column_type] = attribute[:type] + attribute_copy = attribute.dup + + if attribute_copy.key?(:type) + attribute_copy[:value_type] = attribute_copy[:type] + attribute_copy[:column_type] = attribute_copy[:type] end - default_attribute.merge(attribute) + default_attribute.deep_merge(attribute_copy) else - default_attribute.merge(name: attribute) + default_attribute.deep_merge(name: attribute) end end @@ -1074,15 +1165,10 @@ but only if it is not blank. def column_options_from(attributes) attributes.inject({}) do |options, attribute| - options_for_attribute = options[attribute[:name]] = { + options[attribute[:name]] = { type: attribute[:column_type], options: attribute.fetch(:options, {}) } - - if attribute[:array] - options_for_attribute[:options][:array] = attribute[:array] - end - options end end @@ -1090,10 +1176,14 @@ but only if it is not blank. def attributes_with_values_for(model) model_attributes[model].each_with_object({}) do |attribute, attrs| attrs[attribute[:name]] = attribute.fetch(:value) do - dummy_value_for( - attribute[:value_type], - array: attribute[:array] - ) + if attribute[:options][:null] + nil + else + dummy_value_for( + attribute[:value_type], + array: attribute[:options][:array] + ) + end end end end @@ -1152,25 +1242,21 @@ but only if it is not blank. end end - def determine_scope_attribute_names_from(scope_attributes) - scope_attributes.map do |attribute| - if attribute.is_a?(Hash) - attribute[:name] - else - attribute - end - end - end - def define_model_validating_uniqueness(options = {}, &block) + attribute_name = options.fetch(:attribute_name) { self.attribute_name } attribute_type = options.fetch(:attribute_type, :string) attribute_options = options.fetch(:attribute_options, {}) - attribute = { + attribute = normalize_attribute( name: attribute_name, value_type: attribute_type, column_type: attribute_type, options: attribute_options - } + ) + + if options.key?(:attribute_value) + attribute[:value] = options[:attribute_value] + end + scope_attributes = normalize_attributes(options.fetch(:scopes, [])) scope_attribute_names = scope_attributes.map { |attr| attr[:name] } additional_attributes = normalize_attributes( @@ -1239,4 +1325,11 @@ but only if it is not blank. def attribute_name :attr end + + def validation_matcher_scenario_args + super.deep_merge( + matcher_name: :validate_uniqueness_of, + model_creator: :"active_record/uniqueness_matcher" + ) + end end