diff --git a/.hound/ruby.yml b/.hound/ruby.yml index c942222f..372cc677 100644 --- a/.hound/ruby.yml +++ b/.hound/ruby.yml @@ -8,6 +8,7 @@ AllCops: - "db/**/*" DisplayCopNames: false StyleGuideCopsOnly: false + TargetRubyVersion: 2.4 Naming/AccessorMethodName: Description: Check the naming of accessor methods for get_/set_. Enabled: false @@ -22,7 +23,6 @@ Naming/BinaryOperatorParameterName: Naming/ClassAndModuleCamelCase: Description: Use CamelCase for classes and modules. StyleGuide: https://github.com/bbatsov/ruby-style-guide#camelcase-classes - Enabled: true Naming/ConstantName: Description: Constants should use SCREAMING_SNAKE_CASE. StyleGuide: https://github.com/bbatsov/ruby-style-guide#screaming-snake-case diff --git a/lib/shoulda/matchers/active_model/allow_or_disallow_value_matcher.rb b/lib/shoulda/matchers/active_model/allow_or_disallow_value_matcher.rb index 631946d1..3c29ee82 100644 --- a/lib/shoulda/matchers/active_model/allow_or_disallow_value_matcher.rb +++ b/lib/shoulda/matchers/active_model/allow_or_disallow_value_matcher.rb @@ -20,7 +20,7 @@ module Shoulda :values_to_preset, ) - def initialize(*values) + def initialize(*values, part_of_larger_matcher: false) super @values_to_set = values @options = {} @@ -32,6 +32,7 @@ module Shoulda @failure_message_preface = nil @attribute_changed_value_message = nil @was_negated = nil + @part_of_larger_matcher = part_of_larger_matcher end def for(attribute_name) @@ -93,6 +94,19 @@ module Shoulda @was_negated end + def part_of_larger_matcher? + @part_of_larger_matcher + end + + def include_attribute_changed_value_message? + !ignore_interference_by_writer.never? && + result.attribute_setter.attribute_changed_value? + end + + def attribute_changed_value_message + stored_attribute_changed_value_message.call + end + def description ValidationMatcher::BuildDescription.call(self, simple_description) end @@ -163,8 +177,8 @@ module Shoulda message = attribute_setter.failure_message end - if include_attribute_changed_value_message? - message << "\n\n" + attribute_changed_value_message.call + if !part_of_larger_matcher? && include_attribute_changed_value_message? + message << "\n\n" + attribute_changed_value_message end Shoulda::Matchers.word_wrap(message) @@ -232,8 +246,8 @@ module Shoulda message = attribute_setter.failure_message end - if include_attribute_changed_value_message? - message << "\n\n" + attribute_changed_value_message.call + if !part_of_larger_matcher? && include_attribute_changed_value_message? + message << "\n\n" + attribute_changed_value_message end Shoulda::Matchers.word_wrap(message) @@ -271,23 +285,18 @@ module Shoulda end end - def include_attribute_changed_value_message? - !ignore_interference_by_writer.never? && - result.attribute_setter.attribute_changed_value? - end - - def attribute_changed_value_message + def stored_attribute_changed_value_message @attribute_changed_value_message || method(:default_attribute_changed_value_message) end def default_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. +As indicated 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. Otherwise, you +may need to do something else entirely. MESSAGE 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 4d871a53..c09e9c64 100644 --- a/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_absence_of_matcher.rb @@ -84,7 +84,8 @@ module Shoulda end def simple_description - "fail validation when :#{attribute} is empty/falsy" + # "fail validation when :#{attribute} is empty/falsy" + "validate absence of :#{attribute}" end protected @@ -118,9 +119,9 @@ module Shoulda end def column_type - subject.class.respond_to?(:columns_hash) && - subject.class.columns_hash[attribute.to_s].respond_to?(:type) && - subject.class.columns_hash[attribute.to_s].type + record.class.respond_to?(:columns_hash) && + record.class.columns_hash[attribute.to_s].respond_to?(:type) && + record.class.columns_hash[attribute.to_s].type end def collection? @@ -132,8 +133,8 @@ module Shoulda end def reflection - subject.class.respond_to?(:reflect_on_association) && - subject.class.reflect_on_association(attribute) + record.class.respond_to?(:reflect_on_association) && + record.class.reflect_on_association(attribute) end end end 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 69f0f6aa..3dd07f22 100644 --- a/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_acceptance_of_matcher.rb @@ -86,14 +86,19 @@ module Shoulda @expected_message = :accepted end - def matches?(subject) - super(subject) - disallows_value_of(false, @expected_message) + def matches?(record) + add_matcher_disallowing(false, expected_message) + super(record) end def simple_description - %(validate that :#{@attribute} has been set to "1") + # "fail validation when :#{attribute} is not set to a true-like value" + "validate acceptance of :#{attribute}" end + + private + + attr_reader :expected_message end end end diff --git a/lib/shoulda/matchers/active_model/validation_matcher.rb b/lib/shoulda/matchers/active_model/validation_matcher.rb index b30b5262..2cbfde0a 100644 --- a/lib/shoulda/matchers/active_model/validation_matcher.rb +++ b/lib/shoulda/matchers/active_model/validation_matcher.rb @@ -81,36 +81,34 @@ module Shoulda message = "\n\n" if was_negated? - message << 'At least one of these submatchers should have failed:' + message << 'At least one of these subtests should have failed:' else - message << 'All of these submatchers should have passed:' + message << 'All of these subtests should have passed:' end message << "\n\n" - list = submatcher_results.map do |submatcher, matched| - icon = if matched then '✔︎' else '✘' end - submessage = "#{icon} should #{submatcher.description}" + list = submatcher_results.map do |result| + item = '' - if !matched - sub_failure_message = - if submatcher.was_negated? - submatcher.failure_message_when_negated - else - submatcher.failure_message - end - - submessage << "\n\n" - submessage << Shoulda::Matchers.word_wrap( - "#{sub_failure_message}\n", - indent: 2, - ) + if result.expected? + item << "* #{result.submatcher_expectation} (✔︎)" + elsif result.matched? + item << "* #{result.submatcher_expectation} (PASSED ✘)" + else + item << "* #{result.submatcher_failure_message} (✘)" end - submessage + item end - message << Shoulda::Matchers.word_wrap(list.join("\n")) + message << list.join("\n") + + if submatcher_result_with_attribute_changed_value_message.present? + message << "\n\n#{submatcher_result_with_attribute_changed_value_message.submatcher_attribute_changed_value_message}" + end + + Shoulda::Matchers.word_wrap(message) end def was_negated? @@ -178,24 +176,24 @@ module Shoulda private def overall_failure_message - message = - "Expected #{model.name} to #{description}, " + - "but there were some issues." - - Shoulda::Matchers.word_wrap(message) + Shoulda::Matchers.word_wrap(<<~MESSAGE) + Your test expecting #{model.name} to #{simple_description} didn't + pass. + MESSAGE end def overall_failure_message_when_negated - Shoulda::Matchers.word_wrap( - "Expected #{model.name} not to #{description}, but it did." - ) + Shoulda::Matchers.word_wrap(<<~MESSAGE) + Your test expecting #{model.name} not to #{simple_description} + didn't pass. + MESSAGE end def indented_failure_message_for_first_failing_submatcher if failure_message_for_first_failing_submatcher.present? "\n" + Shoulda::Matchers.word_wrap( failure_message_for_first_failing_submatcher, - indent: 2 + indent: 2, ) end end @@ -205,25 +203,39 @@ module Shoulda end def all_submatchers_match? - submatcher_results.all? { |submatcher, matched| matched } + submatcher_results.all?(&:expected?) end def first_failing_submatcher - tuple = submatcher_results.detect { |submatcher, matched | !matched } + tuple = submatcher_results.detect(&:unexpected?) if tuple tuple.first end end + def submatcher_result_with_attribute_changed_value_message + unexpected_submatcher_results.detect do |result| + result.submatcher_includes_attribute_changed_value_message? + end + end + + def unexpected_submatcher_results + submatcher_results.select(&:unexpected?) + end + def submatcher_results @_submatcher_results ||= submatchers.map do |submatcher| - [submatcher, submatcher.matches?(subject)] + SubmatcherResult.new( + submatcher, + submatcher.matches?(subject), + was_negated?, + ) end end def build_allow_or_disallow_value_matcher(matcher_class:, values:, message:) - matcher = matcher_class.new(*values). + matcher = matcher_class.new(*values, part_of_larger_matcher: true). for(attribute). with_message(message). on(context). @@ -234,6 +246,63 @@ module Shoulda matcher end + + class SubmatcherResult + def initialize(submatcher, matched, parent_was_negated) + @submatcher = submatcher + @matched = matched + @parent_was_negated = parent_was_negated + end + + def matched? + @matched + end + + def expected? + (!parent_was_negated? && matched?) || + (parent_was_negated? && !matched?) + end + + def unexpected? + !expected? + end + + def submatcher_model + submatcher.model + end + + def submatcher_description + submatcher.description + end + + def submatcher_failure_message + submatcher.failure_message + end + + def submatcher_expectation + if submatcher.was_negated? + "Expected #{submatcher.model} not to #{submatcher.description}" + else + "Expected #{submatcher.model} to #{submatcher.description}" + end + end + + def submatcher_includes_attribute_changed_value_message? + submatcher.include_attribute_changed_value_message? + end + + def submatcher_attribute_changed_value_message + submatcher.attribute_changed_value_message + end + + private + + attr_reader :submatcher + + def parent_was_negated? + @parent_was_negated + end + end end end end diff --git a/lib/shoulda/matchers/active_model/validator.rb b/lib/shoulda/matchers/active_model/validator.rb index ef75f97c..62b01ed7 100644 --- a/lib/shoulda/matchers/active_model/validator.rb +++ b/lib/shoulda/matchers/active_model/validator.rb @@ -30,7 +30,7 @@ module Shoulda def fails? perform_validation - validation_messages.any? || validation_messages_match? + validation_messages_match? end def captured_validation_exception? 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 5a6e3fa3..5f1cd44e 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 @@ -57,16 +57,13 @@ describe Shoulda::Matchers::ActiveModel::ValidateAbsenceOfMatcher, type: :model record = define_model(:example, attr: :string).new message = <<-MESSAGE -Expected Example to fail validation when :attr is empty/falsy, but there -were some issues. +Expected Example to validate absence of :attr, but there were some +issues. -All of these submatchers should have passed: +All of these subtests should have passed: -✘ should fail validation when :attr is set to ‹"an arbitrary value"›, - producing a custom validation error - - After setting :attr to ‹"an arbitrary value"›, the matcher expected - the Example to be invalid, but it was valid instead. +* After setting :attr to ‹"an arbitrary value"›, the matcher expected + the Example to be invalid, but it was valid instead. (✘) MESSAGE assertion = lambda do @@ -103,16 +100,13 @@ All of these submatchers should have passed: context 'an ActiveModel class without an absence validation' do it 'rejects with the correct failure message' do message = <<-MESSAGE -Expected Example to fail validation when :attr is empty/falsy, but there -were some issues. +Expected Example to validate absence of :attr, but there were some +issues. -All of these submatchers should have passed: +All of these subtests should have passed: -✘ should fail validation when :attr is set to ‹"an arbitrary value"›, - producing a custom validation error - - After setting :attr to ‹"an arbitrary value"›, the matcher expected - the Example to be invalid, but it was valid instead. +* After setting :attr to ‹"an arbitrary value"›, the matcher expected + the Example to be invalid, but it was valid instead. (✘) MESSAGE assertion = lambda do @@ -174,16 +168,13 @@ All of these submatchers should have passed: model = having_and_belonging_to_many(:children, absence: false) message = <<-MESSAGE -Expected Parent to fail validation when :children is empty/falsy, but -there were some issues. +Expected Parent to validate absence of :children, but there were some +issues. -All of these submatchers should have passed: +All of these subtests should have passed: -✘ should fail validation when :children is set to ‹[#]›, - producing a custom validation error - - After setting :children to ‹[#]›, the matcher expected - the Parent to be invalid, but it was valid instead. +* After setting :children to ‹[#]›, the matcher expected + the Parent to be invalid, but it was valid instead. (✘) MESSAGE assertion = lambda do 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 09fd900d..1e86f33c 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 @@ -21,15 +21,19 @@ describe Shoulda::Matchers::ActiveModel::ValidateAcceptanceOfMatcher, type: :mod 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. +Your test expecting Example to validate acceptance of :attr didn't pass. - 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. +All of these subtests should have passed: + +* 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 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. Otherwise, you may +need to do something else entirely. MESSAGE }, },