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 7bf8de84a..49ef78d8c 100644 --- a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb @@ -192,10 +192,8 @@ def failure_message message, but "must exist" instead. With that said, did you know that the `belong_to` matcher can test this -validation for you? Instead of using `validate_presence_of`, try the following -instead: - - it { should #{representation_of_belongs_to} } +validation for you? Instead of using `validate_presence_of`, try +#{suggestions_for_belongs_to} MESSAGE end @@ -279,17 +277,42 @@ def reason_for_existing_presence_validation end end - def representation_of_belongs_to - 'belong_to(:parent)'.tap do |str| - if association_reflection.options.include?(:optional) - str << ".optional(#{association_reflection.options[:optional]})" + def suggestions_for_belongs_to + if belongs_to_association_configured_to_be_required? + <<~MESSAGE + one of the following instead, depending on your use case: + + #{example_of_belongs_to(with: [:optional, false])} + #{example_of_belongs_to(with: [:required, true])} + MESSAGE + else + <<~MESSAGE + the following instead: + + #{example_of_belongs_to} + MESSAGE + end + end + + def example_of_belongs_to(with: nil) + initial_call = "should belong_to(:#{association_name})" + inside = + if with + "#{initial_call}.#{with.first}(#{with.second})" + else + initial_call end + + if Shoulda::Matchers.integrations.test_frameworks.any?(&:n_unit?) + inside + else + "it { #{inside} }" end end def belongs_to_association_configured_to_be_required? - association_reflection.options[:optional] == false || - association_reflection.options[:required] == true + association_options[:optional] == false || + association_options[:required] == true end def belongs_to_association_being_validated? @@ -301,6 +324,14 @@ def association_being_validated? !!association_reflection end + def association_name + association_reflection.name + end + + def association_options + association_reflection&.options + end + def association_reflection model.respond_to?(:reflect_on_association) && model.reflect_on_association(@attribute) diff --git a/lib/shoulda/matchers/configuration.rb b/lib/shoulda/matchers/configuration.rb index 08fa30903..fd988d1f8 100644 --- a/lib/shoulda/matchers/configuration.rb +++ b/lib/shoulda/matchers/configuration.rb @@ -5,6 +5,11 @@ def self.configure yield configuration end + # @private + def self.integrations + configuration.integrations + end + # @private def self.configuration @_configuration ||= Configuration.new @@ -12,8 +17,14 @@ def self.configuration # @private class Configuration + attr_reader :integrations + + def initialize + @integrations = nil + end + def integrate(&block) - Integrations::Configuration.apply(self, &block) + @integrations = Integrations::Configuration.apply(&block) end end end diff --git a/lib/shoulda/matchers/integrations/configuration.rb b/lib/shoulda/matchers/integrations/configuration.rb index b7179bf38..78f7953e8 100644 --- a/lib/shoulda/matchers/integrations/configuration.rb +++ b/lib/shoulda/matchers/integrations/configuration.rb @@ -5,11 +5,13 @@ module Matchers module Integrations # @private class Configuration - def self.apply(configuration, &block) - new(configuration, &block).apply + def self.apply(&block) + new(&block).apply end - def initialize(configuration, &block) + attr_reader :test_frameworks + + def initialize(&block) @test_frameworks = Set.new @libraries = Set.new @@ -47,6 +49,8 @@ def apply test_framework.include(Shoulda::Matchers::Independent) @libraries.each { |library| library.integrate_with(test_framework) } end + + self end private diff --git a/spec/support/unit/matchers/match_against.rb b/spec/support/unit/matchers/match_against.rb index 9614c5b09..bc73a77c9 100644 --- a/spec/support/unit/matchers/match_against.rb +++ b/spec/support/unit/matchers/match_against.rb @@ -140,7 +140,7 @@ def matcher_fails_in_positive? end def diff(expected, actual) - differ.diff(expected, actual)[1..-1] + differ.diff(actual, expected).strip end def differ 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 b0e06cedc..57f80be33 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 @@ -309,7 +309,7 @@ def model_creator end context 'and an explicit presence validation is not on the association' do - it 'does not match, instructing the user to use belong_to instead' do + it 'does not match, instructing the user to use belongs_to instead' do record = record_belonging_to( :parent, optional: false, @@ -337,9 +337,96 @@ def model_creator With that said, did you know that the `belong_to` matcher can test this validation for you? Instead of using `validate_presence_of`, try - the following instead: + one of the following instead, depending on your use case: + + it { should belong_to(:parent).optional(false) } + it { should belong_to(:parent).required(true) } + MESSAGE + end + end + end + + context 'declared with required: true' do + context 'and an explicit presence validation is on the association' do + it 'matches' do + record = record_belonging_to( + :parent, + required: true, + validate_presence: true, + ) + + expect { validate_presence_of(:parent) }.to match_against(record) + end + end + + context 'and an explicit presence validation is not on the association' do + it 'does not match, instructing the user to use belongs_to instead' do + record = record_belonging_to( + :parent, + required: true, + validate_presence: false, + model_name: 'Child', + parent_model_name: 'Parent', + ) + + expect { validate_presence_of(:parent) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected Child to validate that :parent cannot be empty/falsy, but this +could not be proved. + After setting :parent to ‹nil›, the matcher expected the Child to be + invalid and to produce the validation error "can't be blank" on + :parent. The record was indeed invalid, but it produced these + validation errors instead: + + * parent: ["must exist"] + + You're getting this error because you've instructed your `belongs_to` + association to add a presence validation to the attribute. *This* + presence validation doesn't use "can't be blank", the usual validation + message, but "must exist" instead. + + With that said, did you know that the `belong_to` matcher can test + this validation for you? Instead of using `validate_presence_of`, try + one of the following instead, depending on your use case: it { should belong_to(:parent).optional(false) } + it { should belong_to(:parent).required(true) } + MESSAGE + end + end + end + + context 'declared with required: false' do + context 'and an explicit presence validation is on the association' do + it 'matches' do + record = record_belonging_to( + :parent, + required: false, + validate_presence: true, + ) + + expect { validate_presence_of(:parent) }.to match_against(record) + end + end + + context 'and an explicit presence validation is not on the association' do + it 'does not match' do + record = record_belonging_to( + :parent, + required: false, + validate_presence: false, + model_name: 'Child', + parent_model_name: 'Parent', + ) + + expect { validate_presence_of(:parent) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected Child to validate that :parent cannot be empty/falsy, but this +could not be proved. + After setting :parent to ‹nil›, the matcher expected the Child to be + invalid, but it was valid instead. MESSAGE end end