WIP: Add ignoring_interference_by_writer to everything

NEWS.md

@@ -1,21 +1,36 @@
 # HEAD
 
+### Bug fixes
+
+* Update `validate_numericality_of` so that submatchers are applied lazily
+  instead of immediately. Previously, qualifiers were order-dependent, meaning
+  that if you used `strict` before you used, say, `odd`, then `strict` wouldn't
+  actually apply to `odd`. Now the order that you specify qualifiers doesn't
+  matter.
+
+* Fix `allow_value` so that it does not raise an AttributeChangedValueError
+  (formerly CouldNotSetAttributeError) when used against an attribute that is an
+  enum in an ActiveRecord model.
+
+### Features
+
+* Add `ignoring_interference_by_writer` to all matchers, not just `allow_value`.
+  This makes it possible to get around CouldNotSetAttributeErrors that you are
+  probably used to seeing.
+
 ### Improvements
 
 * Improve failure messages and descriptions of all matchers across the board so
   that it is easier to understand what the matcher was doing when it failed.
   (You'll see a huge difference in the output of the numericality and uniqueness
   matchers in particular.)
 
+* The CouldNotSetAttributeError that you have probably seen is now called
+  AttributeChangedValueError.
+
 * Matchers now raise an error if any attributes that the matcher is attempting
   to set do not exist on the model.
 
-* Update `validate_numericality_of` so that submatchers are applied lazily
-  instead of immediately. Previously, qualifiers were order-dependent, meaning
-  that if you used `strict` before you used, say, `odd`, then `strict` wouldn't
-  actually apply to `odd`. Now the order that you specify qualifiers doesn't
-  matter.
-
 * Update `validate_numericality_of` so that it doesn't always run all of the
   submatchers, but stops on the first one that fails. Since failure messages
   now contain information as to what value the matcher set on the attribute when

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'

lib/shoulda/matchers/active_model/allow_value_matcher.rb

@@ -58,7 +58,7 @@ module ActiveModel
       # #### Caveats
       #
       # When using `allow_value` or any matchers that depend on it, you may
-      # encounter a CouldNotSetAttributeError. This exception is raised if the
+      # encounter an AttributeChangedValueError. This exception is raised if the
       # matcher, in attempting to set a value on the attribute, detects that
       # the value set is different from the value that the attribute returns
       # upon reading it back.
@@ -86,7 +86,7 @@ module ActiveModel
       #           it do
       #             foo = Foo.new
       #             foo.bar = "baz"
-      #             # This will raise a CouldNotSetAttributeError since `foo.bar` is now "123"
+      #             # This will raise an AttributeChangedValueError since `foo.bar` is now "123"
       #             expect(foo).not_to allow_value(nil).for(:bar)
       #           end
       #         end
@@ -108,7 +108,7 @@ module ActiveModel
       #         describe Foo do
       #           it do
       #             foo = Foo.new
-      #             # This will raise a CouldNotSetAttributeError since `foo.bar` is now "123"
+      #             # This will raise an AttributeChangedValueError since `foo.bar` is now "123"
       #             expect(foo).not_to allow_value("abc123").for(:bar)
       #           end
       #         end
@@ -118,32 +118,25 @@ module ActiveModel
       #
       #         describe Foo do
       #           # Assume that `attr` is a string
-      #           # This will raise a CouldNotSetAttributeError since `attr` typecasts `[]` to `"[]"`
+      #           # This will raise an AttributeChangedValueError since `attr` typecasts `[]` to `"[]"`
       #           it { should_not allow_value([]).for(:attr) }
       #         end
       #
-      # So when you encounter this exception, you have a couple of options:
-      #
-      # * If you understand the problem and wish to override this behavior to
-      #   get around this exception, you can add the
-      #   `ignoring_interference_by_writer` qualifier like so:
+      # Fortunately, if you understand why this is happening, and wish to get
+      # around this exception, it is possible to do so. You can use the
+      # `ignoring_interference_by_writer` qualifier like so:
       #
       #         it do
       #           should_not allow_value([]).
       #             for(:attr).
       #             ignoring_interference_by_writer
       #         end
       #
-      # * Note, however, that the above option will not always cause the test to
-      #   pass. In this case, this is telling you that you don't need to use
-      #   `allow_value`, or quite possibly even the validation that you're
-      #   testing altogether. In any case, we would probably make the argument
-      #   that since it's clear that something is responsible for sanitizing
-      #   incoming data before it's stored in your model, there's no need to
-      #   ensure that sanitization places the model in a valid state, if such
-      #   sanitization creates valid data. In terms of testing, the sanitization
-      #   code should probably be tested, but not the effects of that
-      #   sanitization on the validness of the model.
+      # Please note, however, that this qualifier won't magically cause your
+      # test to pass. It may just so happen that the final value that ends up
+      # being set causes the model to fail validation. In that case, you'll have
+      # to figure out what to do. You may need to write your own test, or
+      # perhaps even remove your test altogether.
       #
       # #### Qualifiers
       #
@@ -274,9 +267,9 @@ module ActiveModel
       #
       # ##### ignoring_interference_by_writer
       #
-      # Use `ignoring_interference_by_writer` if you've encountered a
-      # CouldNotSetAttributeError and wish to ignore it. Please read the Caveats
-      # section above for more information.
+      # Use `ignoring_interference_by_writer` to bypass an
+      # AttributeChangedValueError that you have encountered. Please read the
+      # Caveats section above for more information.
       #
       #     class Address < ActiveRecord::Base
       #       # Address has a zip_code field which is a string
@@ -286,16 +279,16 @@ module ActiveModel
       #     describe Address do
       #       it do
       #         should_not allow_value([]).
-      #         for(:zip_code).
-      #         ignoring_interference_by_writer
+      #           for(:zip_code).
+      #           ignoring_interference_by_writer
       #       end
       #     end
       #
       #     # Minitest (Shoulda)
       #     class AddressTest < ActiveSupport::TestCase
       #       should_not allow_value([]).
-      #       for(:zip_code).
-      #       ignoring_interference_by_writer
+      #         for(:zip_code).
+      #         ignoring_interference_by_writer
       #     end
       #
       # @return [AllowValueMatcher]
@@ -313,6 +306,7 @@ def allow_value(*values)
       # @private
       class AllowValueMatcher
         include Helpers
+        include Qualifiers::IgnoringInterferenceByWriter
 
         attr_reader(
           :after_setting_value_callback,
@@ -325,10 +319,10 @@ class AllowValueMatcher
         attr_writer :failure_message_preface, :values_to_preset
 
         def initialize(*values)
+          super
           @values_to_set = values
           @options = {}
           @after_setting_value_callback = -> {}
-          @ignoring_interference_by_writer = false
           @expects_strict = false
           @expects_custom_validation_message = false
           @context = nil
@@ -387,15 +381,6 @@ def expects_strict?
           @expects_strict
         end
 
-        def ignoring_interference_by_writer(value = true)
-          @ignoring_interference_by_writer = value
-          self
-        end
-
-        def ignoring_interference_by_writer?
-          @ignoring_interference_by_writer
-        end
-
         def _after_setting_value(&callback)
           @after_setting_value_callback = callback
         end
@@ -432,6 +417,10 @@ def failure_message
             end
           end
 
+          if include_attribute_changed_value_message?
+            message << "\n\n" + attribute_changed_value_message
+          end
+
           Shoulda::Matchers.word_wrap(message)
         end
 
@@ -497,9 +486,23 @@ def failure_message_when_negated
             end
           end
 
+          if include_attribute_changed_value_message?
+            message << "\n\n" + attribute_changed_value_message
+          end
+
           Shoulda::Matchers.word_wrap(message)
         end
 
+        def attribute_changed_value_message
+          <<-MESSAGE.strip
+As indicated in the message above, :#{result.attribute_setter.attribute_name}
+seems to be changing certain values as they are set, and this could have
+something to do with why this test is failing. If you've overridden the writer
+method for this attribute, then you may need to change it to make this test
+pass, or do something else entirely.
+          MESSAGE
+        end
+
         def description
           ValidationMatcher::BuildDescription.call(self, simple_description)
         end
@@ -512,8 +515,12 @@ def model
           instance.class
         end
 
+        def last_attribute_setter_used
+          result.attribute_setter
+        end
+
         def last_value_set
-          result.attribute_setter.value_written
+          last_attribute_setter_used.value_written
         end
 
         protected
@@ -579,6 +586,11 @@ def attribute_setters_and_validators_for_values_to_set
             )
         end
 
+        def include_attribute_changed_value_message?
+          !ignore_interference_by_writer.never? &&
+            result.attribute_setter.attribute_changed_value?
+        end
+
         def inspected_values_to_set
           Shoulda::Matchers::Util.inspect_values(values_to_set).to_sentence(
             two_words_connector: " or ",

lib/shoulda/matchers/active_model/allow_value_matcher/attribute_changed_value_error.rb

@@ -13,23 +13,26 @@ def message
 #{model.name} to #{value_written.inspect}, but when the attribute was
 read back, it had stored #{value_read.inspect} instead.
 
-This creates a problem because it means that the model is behaving in a way that
-is interfering with the test -- there's a mismatch between the test that was
-written and test that was actually run.
+This creates a problem because it means that the model is behaving in a
+way that is interfering with the test -- there's a mismatch between the
+test that you wrote and test that we actually ran.
 
 There are a couple of reasons why this could be happening:
 
-* The writer method for :#{attribute_name} has been overridden and contains
-custom logic to prevent certain values from being set or change which values
-are stored.
+* 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.
 
-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 you understand this and you wish to bypass this exception, you can
+tack 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, then post an issue to the shoulda-matchers issues list
+and we'll get you squared away:
+
+http://github.com/thoughtbot/shoulda-matchers/issues
             MESSAGE
           end