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 | 336 ++++++++++++--- .../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, 2216 insertions(+), 368 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 b605b5bb9..3a00a61ee 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 e455d6043..19f58a4b3 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 615e3bc25..8b74f779b 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 ActiveModel # #### 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 ActiveModel # 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 ActiveModel # 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 ActiveModel # # 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 ActiveModel # 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 ActiveModel # # ##### 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 ActiveModel # 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 @@ def allow_value(*values) # @private class AllowValueMatcher include Helpers + include Qualifiers::IgnoringInterferenceByWriter attr_reader( :after_setting_value_callback, @@ -325,10 +319,10 @@ class AllowValueMatcher 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 @@ def expects_strict? @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 @@ def failure_message 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 @@ def failure_message_when_negated 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 @@ def model 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 @@ def attribute_setters_and_validators_for_values_to_set ) 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 ea57752ae..417982eec 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 @@ def message #{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 e174ad0d8..9fde90331 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 @@ def self.set(args) 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 @@ def successfully_set? 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 @@ def attribute_exists? 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 6b6c752b2..8d89fbe3b 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 @@ class AttributeSetterAndValidator :context, :expected_message, :expects_strict?, - :ignoring_interference_by_writer?, + :ignore_interference_by_writer, :instance, ) @@ -33,7 +33,7 @@ def attribute_setter 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 ad6363e4b..cd0ffba90 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 ActiveModel 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 @@ def strict(strict = true) 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 5c23c9fe1..4a54b8869 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 @@ class NumericTypeMatcher :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 000000000..5a2772044 --- /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 000000000..04f1ab689 --- /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 000000000..31302f02b --- /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 52d013e2a..7bc450488 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 ActiveModel # 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 ActiveModel # # 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 1c64d2cbd..3286bdf5d 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 ActiveModel # 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 3a0281ce7..5c029da72 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 ActiveModel # 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 2e5f82624..a996d295e 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 ActiveModel # 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 ActiveModel # 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 ActiveModel # # 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 ActiveModel # 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 ActiveModel # # 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 ActiveModel # # 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 ActiveModel # 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 ActiveModel # # 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 97fa2f471..f8fd8d628 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 ActiveModel # 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 f44d54312..1a81f720c 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 ActiveModel # # 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 ActiveModel # 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 @@ class ValidateNumericalityOfMatcher 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 @@ def qualify_submatchers 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 770bf9c43..c217212aa 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 ActiveModel # 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 @@ class ValidatePresenceOfMatcher < ValidationMatcher def initialize(attribute) super @expected_message = :blank + @ignore_interference_by_writer = + Qualifiers::IgnoreInterferenceByWriter.new(when: :blank?) end def matches?(subject) @@ -146,8 +181,6 @@ def disallows_and_double_checks_value_of!(value, message) 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 a7d97dd3f..3f6d4ba5b 100644 --- a/lib/shoulda/matchers/active_model/validation_matcher.rb +++ b/lib/shoulda/matchers/active_model/validation_matcher.rb @@ -3,7 +3,10 @@ module Matchers module ActiveModel # @private class ValidationMatcher + include Qualifiers::IgnoringInterferenceByWriter + def initialize(attribute) + super @attribute = attribute @expects_strict = false @subject = nil @@ -12,6 +15,10 @@ def initialize(attribute) @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 @@ def expects_strict? @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 @@ def expects_custom_validation_message? @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 @@ def build_allow_or_disallow_value_matcher(args) 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 6dbdb5997..6c4950eff 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 @@ def initialize(attribute) 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 @@ def matches?(given_record) @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 @@ def build_allow_or_disallow_value_matcher(args) 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 @@ def inspected_actual_scopes 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 @@ def allows_nil? 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 @@ def existing_record_valid? 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 @@ def has_secure_password? 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 @@ def validate_scopes_present? 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 @@ def validate_scopes_present? 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 @@ def validate_after_scope_change? 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 @@ def available_enum_values_for(scope, previous_value) 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 @@ def column_limit_for(attribute) 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" - else - prefix << "Given an existing #{model.name} whose :#{attribute}" - prefix << " is " - prefix << Shoulda::Matchers::Util.inspect_value(existing_value) - prefix << ", after" - end + prefix << "After taking the given #{model.name}" - prefix << " making a new #{model.name} and setting its" - prefix << " :#{attribute} to " + 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 - if last_value_set_on_new_record == existing_value - prefix << Shoulda::Matchers::Util.inspect_value( - last_value_set_on_new_record - ) - prefix << " as well" + prefix << ", and saving it as the existing record, then" else - prefix << " a different value, " - prefix << Shoulda::Matchers::Util.inspect_value( - last_value_set_on_new_record - ) + 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 << 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 dfc3d1aaf..d03babaca 100644 --- a/spec/acceptance/multiple_libraries_integration_spec.rb +++ b/spec/acceptance/multiple_libraries_integration_spec.rb @@ -25,6 +25,7 @@ class User < ActiveRecord::Base 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 000000000..2edb20e06 --- /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 322a9dd93..3c252e266 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 @@ def name=(_value) 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 972ace3a6..c277bc2cd 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 @@ def self.available_column_types 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 @@ def self.available_column_types 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 @@ def self.available_column_types 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 @@ def self.available_column_types 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 @@ def self.available_column_types 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 @@ def self.available_column_types 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 @@ def having_and_belonging_to_many(plural_name, options = {}) 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 467ec719a..6fa150ca9 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 @@ 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 @@ def matcher 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 @@ def record_validating_nothing 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 23449faea..7859bfa24 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 @@ 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 @@ 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 24c6fdbf0..a39e0b85f 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 @@ 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 @@ 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 2b06c8634..835215d76 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 @@ def self.testing_values_of_option(option_name, &block) matches_or_not.reverse! to_or_not_to.reverse! end - end end @@ -49,6 +48,10 @@ def build_object(options = {}, &block) 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 @@ def build_object(options = {}, &block) 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 @@ def build_object(options = {}, &block) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def build_object(options = {}, &block) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def add_outside_value_to(values) 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 @@ def expect_to_match_on_values(builder, values, &block) 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 @@ def expect_not_to_match_on_values(builder, values, &block) 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 @@ def expect_not_to_match_on_values(builder, values, &block) 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 @@ def expect_not_to_match_on_values(builder, values, &block) 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 @@ def expect_to_match_on_values(builder, range, &block) 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 @@ def expect_not_to_match_on_values(builder, range, &block) 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 @@ def expect_not_to_match_on_values(builder, range, &block) 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 @@ def build_object(options = {}, &block) 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] - - model = define_model :example, attribute_name => column_options - customize_model_class( - model, - attribute_name, - validation_options, - custom_validation - ) - - object = model.new + def define_simple_model(attribute_name: :attr, column_options: {}, &block) + define_model('Example', attribute_name => column_options, &block) + end - 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 @@ def build_object(options = {}, &block) 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] - - 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) + def define_simple_model(attribute_name: :attr, column_options: {}, &block) + define_active_model_class('Example', accessors: [attribute_name], &block) + end - 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 @@ def object_builder_class @_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) + model.class_eval do + define_method :custom_validation do + custom_validation.call(self, attribute_name) + end + + validate :custom_validation end + end - validate :custom_validation + 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 @@ def expect_to_match_ensuring_range_and_messages(range, low_value, high_value) 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 @@ def expect_to_match_ensuring_range_and_messages(range, low_value, high_value) 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 f7330d35a..65430616d 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 @@ 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 @@ 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 @@ 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 @@ 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 ea506bff1..c7ba5beda 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def default_validation_values 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 @@ def set_attr!; self.attr = 5 end 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 @@ def validate_numericality 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 babe711bc..8c2fbdedb 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ def active_resource_model 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 46e6b313b..767186994 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 @@ 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 @@ 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 @@ 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 @@ ) 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ def normalize_attributes(attributes) 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 @@ def column_options_from(attributes) 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 @@ def create_record_from(model, extra_attributes = {}) 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 @@ def validate_uniqueness 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