Skip to content

Commit

Permalink
Enable ignoring_interference_by_writer by default
Browse files Browse the repository at this point in the history
Forcing people to add ignoring_interference_by_writer for each and every
case in which an attribute changes incoming values is pretty obnoxious
on our part (for instance, when using the numericality matcher against
an integer column + `only_integer`). So now, it's enabled by default.
This effectively means that people should never get
AtttributeChangedValueErrors again.
  • Loading branch information
mcmire committed Jan 5, 2016
1 parent 1189934 commit 5532f43
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 225 deletions.
14 changes: 7 additions & 7 deletions NEWS.md
Expand Up @@ -12,11 +12,10 @@
(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.
`allow_value`. *This is enabled by default, which means that you should never
get a CouldNotSetAttributeError again.* (You may get some more information if
a test fails, however.)

### Improvements

Expand All @@ -25,9 +24,6 @@
(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.

Expand All @@ -36,6 +32,10 @@
now contain information as to what value the matcher set on the attribute when
it failed, this change guarantees that the correct value will be shown.

* Continue to detect if attributes change incoming values, but now instead of
immediately seeing a CouldNotSetAttributeError, you will only be informed
about it if the test you've written fails.

# 3.0.1

### Bug fixes
Expand Down
Expand Up @@ -6,7 +6,7 @@ module Qualifiers
class IgnoreInterferenceByWriter
attr_reader :setting, :condition

def initialize(argument = :never)
def initialize(argument = :always)
set(argument)
@changed = false
end
Expand Down Expand Up @@ -59,6 +59,10 @@ def considering?(value)
end
end

def always?
setting == :always
end

def never?
setting == :never
end
Expand Down
Expand Up @@ -147,14 +147,13 @@ class ValidatePresenceOfMatcher < ValidationMatcher
def initialize(attribute)
super
@expected_message = :blank
@ignore_interference_by_writer =
Qualifiers::IgnoreInterferenceByWriter.new(when: :blank?)
end

def matches?(subject)
super(subject)

if secure_password_being_validated?
ignore_interference_by_writer.default_to(when: :blank?)
disallows_and_double_checks_value_of!(blank_value, @expected_message)
else
disallows_original_or_typecast_value?(blank_value, @expected_message)
Expand Down
@@ -1,6 +1,5 @@
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
]
Expand All @@ -10,61 +9,39 @@
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]
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 'raises an AttributeChangedValueError' do
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)

assertion = lambda do
expect(scenario.record).to matcher
end

expect(&assertion).to raise_error(
Shoulda::Matchers::ActiveModel::AllowValueMatcher::AttributeChangedValueError
)
expect(scenario.record).to matcher
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]
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 '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)
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)

expect(scenario.record).to matcher.ignoring_interference_by_writer
assertion = lambda do
expect(scenario.record).to matcher
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
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
Expand Down
165 changes: 90 additions & 75 deletions spec/unit/shoulda/matchers/active_model/allow_value_matcher_spec.rb
Expand Up @@ -410,29 +410,55 @@
end

context 'when the attribute interferes with attempts to be set' do
context 'when the matcher has not been qualified with #ignoring_interference_by_writer' do
context 'when the attribute cannot be changed from nil to non-nil' do
it 'raises an AttributeChangedValueError' do
model = define_active_model_class 'Example' do
attr_reader :name
context 'when the attribute cannot be changed from nil to non-nil' do
context 'and the record remains valid' do
it 'accepts (and does not raise an AttributeChangedValueError)' do
model = define_active_model_class 'Example', accessors: [:name] do
def name=(_value)
nil
end
end

expect(model.new).to allow_value('anything').for(:name)
end
end

context 'and the record becomes invalid' do
it 'rejects with an appropriate failure message' do
model = define_active_model_class 'Example', accessors: [:name] do
validates_presence_of :name

def name=(_value)
nil
end
end

assertion = -> {
assertion = lambda do
expect(model.new).to allow_value('anything').for(:name)
}
end

expect(&assertion).to raise_error(
described_class::AttributeChangedValueError
)
message = <<-MESSAGE.strip
After setting :name to ‹"anything"› -- which was read back as ‹nil› --
the matcher expected the Example to be valid, but it was invalid
instead, producing these validation errors:
* name: ["can't be blank"]
As indicated in the message above, :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

expect(&assertion).to fail_with_message(message)
end
end
end

context 'when the attribute cannot be changed from non-nil to nil' do
it 'raises an AttributeChangedValueError' do
context 'when the attribute cannot be changed from non-nil to nil' do
context 'and the record remains valid' do
it 'accepts (and does not raise an AttributeChangedValueError)' do
model = define_active_model_class 'Example', accessors: [:name] do
def name=(value)
if value
Expand All @@ -443,104 +469,93 @@ def name=(value)

record = model.new(name: 'some name')

assertion = -> {
expect(record).to allow_value(nil).for(:name)
}

expect(&assertion).to raise_error(
described_class::AttributeChangedValueError
)
expect(record).to allow_value(nil).for(:name)
end
end

context 'when the attribute cannot be changed from a non-nil value to another non-nil value' do
it 'raises an AttributeChangedValueError' do
model = define_active_model_class 'Example' do
attr_reader :name
context 'and the record becomes invalid' do
it 'rejects with an appropriate failure message' do
model = define_active_model_class 'Example', accessors: [:name] do
validates_absence_of :name

def name=(_value)
@name = 'constant name'
def name=(value)
if value
super(value)
end
end
end

record = model.new(name: 'some name')

assertion = -> {
expect(record).to allow_value('another name').for(:name)
}
assertion = lambda do
expect(record).to allow_value(nil).for(:name)
end

expect(&assertion).to raise_error(
described_class::AttributeChangedValueError
)
message = <<-MESSAGE.strip
After setting :name to ‹nil› -- which was read back as ‹"some name"› --
the matcher expected the Example to be valid, but it was invalid
instead, producing these validation errors:
* name: ["must be blank"]
As indicated in the message above, :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

expect(&assertion).to fail_with_message(message)
end
end
end

context 'when the matcher has been qualified with #ignoring_interference_by_writer' do
context 'when the attribute cannot be changed from nil to non-nil' do
it 'does not raise an error at all' do
model = define_active_model_class 'Example' do
attr_reader :name

context 'when the attribute cannot be changed from a non-nil value to another non-nil value' do
context 'and the record remains valid' do
it 'accepts (and does not raise an AttributeChangedValueError)' do
model = define_active_model_class 'Example', accessors: [:name] do
def name=(_value)
nil
super('constant name')
end
end

assertion = lambda do
expect(model.new).
to allow_value('anything').
for(:name).
ignoring_interference_by_writer
end
record = model.new(name: 'some name')

expect(&assertion).not_to raise_error
expect(record).to allow_value('another name').for(:name)
end
end

context 'when the attribute cannot be changed from non-nil to nil' do
it 'does not raise an error at all' do
model = define_active_model_class 'Example' do
attr_reader :name
context 'and the record becomes invalid' do
it 'rejects with an appropriate failure message' do
model = define_active_model_class 'Example', accessors: [:name] do
validates_format_of :name, with: /another name/

def name=(value)
@name = value unless value.nil?
super('constant name')
end
end

record = model.new(name: 'some name')

assertion = lambda do
expect(record).
to allow_value(nil).
for(:name).
ignoring_interference_by_writer
expect(record).to allow_value('another name').for(:name)
end

expect(&assertion).not_to raise_error
end
end
message = <<-MESSAGE.strip
After setting :name to ‹"another name"› -- which was read back as
‹"constant name"› -- the matcher expected the Example to be valid, but
it was invalid instead, producing these validation errors:
context 'when the attribute cannot be changed from a non-nil value to another non-nil value' do
it 'does not raise an error at all' do
model = define_active_model_class 'Example' do
attr_reader :name
* name: ["is invalid"]
def name=(_value)
@name = 'constant name'
end
end

record = model.new(name: 'some name')

assertion = lambda do
expect(record).
to allow_value('another name').
for(:name).
ignoring_interference_by_writer
end
As indicated in the message above, :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

expect(&assertion).not_to raise_error
expect(&assertion).to fail_with_message(message)
end
end
end
Expand Down

0 comments on commit 5532f43

Please sign in to comment.