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