Commit Graph

21 Commits

Author SHA1 Message Date
Elliot Winkler 507dc57004 Fix negative versions of validation matchers
When using a validation matcher in the negative, i.e.:

    should_not validate_*(...)

as opposed to:

    should validate_*(...)

...it's common to receive the following error:

    undefined method `attribute_setter' for nil:NilClass

This happens particularly when using a matcher that makes use of
AllowValueMatcher or DisallowValueMatcher internally (which all of the
validation matchers do).

Whenever you make an assertion by using a matcher in the negative as
opposed to the positive, RSpec still calls the `matches?` method for
that matcher; however, the assertion will pass if that returns *false*
as opposed to true. In other words, it just inverts the result.

However, whenever we are using AllowValueMatcher or
DisallowValueMatcher, it doesn't really work to invert the result. like
this. This is because AllowValueMatcher and DisallowValueMatcher,
despite their name, aren't truly opposites of each other.

AllowValueMatcher performs these steps:

1. Set the attribute on the record to some value
2. Run validations on the record
3. Ask whether validations pass or fail
4. If validations fail, store the value that caused the failure along
   with the validation errors and return false
5. Otherwise, return true

However, DisallowValueMatcher performs these steps:

1. Set the attribute on the record to some value
2. Run validations on the record
3. Ask whether validations pass or fail
4. If validations *pass*, store the value that caused the failure along
   with some metadata and return false
5. Otherwise, return true

This difference in logic is achieved by having AllowValueMatcher
implement `does_not_match?` and then having DisallowValueMatcher use
this for its positive case and use `matches?` for its negative case.
It's easy to see because of this that `does_not_match?` is not the same
as `!matches?` and vice versa.

So a matcher that makes use of these submatchers internally needs to use
their opposite versions whenever that matcher is used in the negative
case. In other words, all of the matchers need a `does_not_match?` which
is like `matches?`, except that all of the logic is inverted, and in all
the cases in which AllowValueMatcher is used, DisallowValueMatcher needs
to be used.

Doing this ensures that when `failure_message` is called on
AllowValueMatcher or DisallowValueMatcher, step 4 in the list of steps
above stores a proper value that can then be referenced in the failure
message for the validation matcher itself.
2018-09-15 13:43:30 -03:00
Elliot Winkler a559713f96 Add a test for numericality + money columns
18b2859 and a0f1221 fixed the numericality matcher so that it no longer
raises CouldNotSetAttributeError when used against numeric columns.
Unfortunately, the issue still existed for money columns.

This is fixed now, but we never explicitly tested for money columns, and
this commit adds that test.
2016-01-10 00:10:24 -07:00
Brian McManus 12e788f1d0 Numericality validation of virtual attributes
Commit #18b2859d2522a4866c398b9a32ebc3de4ec79389 broke numericality
validation of virtual attributes in ActiveRecord models. This commit
added a `column_type` method that assumes if `columns_hash` is a thing
then our attribute would be present within it. This is not true for
virtual attributes and leads to:
```
     NoMethodError:
           undefined method `type' for nil:NilClass
         # /Users/bdmac/.gem/ruby/2.2.2/gems/shoulda-matchers-3.0.1/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb:442:in
         # `column_type'
```
2016-01-07 12:12:16 -07:00
Elliot Winkler 5ed0362419 Remove IneffectiveTestError
Prior to this commit, the numericality matcher raised an
IneffectiveTestError if used against a numeric column (unless other
qualifiers were added). I had a misunderstanding of how the numericality
validation works -- it turns out it doesn't work against the output of
the attribute in question, it works against the input. So raising this
exception is pointless and doesn't actually protect against anything.
2016-01-05 21:50:27 -07:00
Elliot Winkler 5532f4359a Enable ignoring_interference_by_writer by default
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.
2016-01-05 00:58:17 -07:00
Elliot Winkler 1189934806 Add ignoring_interference_by_writer to all matchers
`allow_value` matcher is, of course, concerned with setting values on a
particular attribute on a particular record, and then checking that the
record is valid after doing so. That comes with a caveat: if the
attribute is overridden in such a way so that the same value going into
the attribute isn't the same value coming out of it, then `allow_value`
will balk -- it'll say, "I can't do that because that changes how I
work."

That's all well and good, but what the attribute intentionally changes
incoming values? ActiveRecord's typecasting behavior, for instance,
would trigger such an exception. What if the developer needs a way to
get around this? This is where `ignoring_interference_by_writer` comes
into play. You can tack it on to the end of the matcher, and you're free
to go on your way.

So, prior to this commit you could already apply it to `allow_value`,
but now in this commit it also works on any other matcher.

But, one little thing: sometimes using this qualifier isn't going to
work. Perhaps you or something else actually *is* overriding the
attribute to change incoming values in a specific way, and perhaps the
value that comes out makes the record fail validation, and there's
nothing you can do about it. So in this case, even if you're using
`ignoring_interference_by_writer`, we want to inform you about what the
attribute is doing -- what the input and output was. And so we do.
2016-01-05 00:58:16 -07:00
Elliot Winkler 6b3253147a allow_value: Inspect values more clearly
Modify descriptions and failure messages for all matchers by way of
allow_value and put small angle brackets around inspected values. This
is to visually distinguish an inspected value from the rest of the text,
and is especially noticeable for complex values such as an array that
contains an object, particularly if the inspected version of the value
wraps onto another line. It's a little easier to see:

    When attempting to set :attr on Example to ‹[#<Child id:
    nil>]›...

rather than:

    When attempting to set :attr on Example to [#<Child id:
    nil>]...
2015-12-30 21:51:54 -05:00
Elliot Winkler bffc9c9b73 Refactor numericality matcher
This is part of a collection of commits that aim to improve failure
messages across the board. The goal here is to make the matcher easier
to debug when something goes wrong.

* Have the failure message describe what the matcher was trying to do
  when it failed.
* Make the description of the matcher more readable.
* Change how the matcher works by stopping at the first failing
  submatcher instead of running all submatchers. Coincidentally, pending
  tests involving a strict validation but a non-strict expectation now
  pass.
* Fix or fill in tests involving failure messages and descriptions to
  match these changes and recent changes to ValidationMatcher and
  allow_value.
2015-12-13 20:22:22 -07:00
Raúl Acuña a0f12216fd Do not raise CouldNotSetAttributeError on decimal columns 2015-10-23 14:38:01 -06:00
Elliot Winkler 18b2859d25 Fix numericality matcher w/ numeric columns
Fix the matcher so it still raises a CouldNotSetAttributeError against a
numeric column, but only if the matcher has not been qualified at all.
When the matcher is qualified with anything else, then it's okay to use
a numeric column, as long as the matcher no longer asserts that the
record disallows a non-numeric value.
2015-10-08 23:37:08 -06:00
Elliot Winkler 9d9dc4e6b9 Tighten CouldNotSetAttributeError restriction
Why:

* Previously, `allow_value` would raise a CouldNotSetAttributeError
  if the value being set didn't match the value the attribute had after
  being set, but only if the attribute was being changed from nil to
  non-nil or non-nil to nil.
* It turns out it doesn't matter which value you're trying to set the
  attribute to -- if the attribute rejects that change it's confusing
  either way. (In fact, I was recently bit by a case in which I was
  trying to validate numericality of an attribute, where the writer
  method for that attribute was overridden to ensure that the attribute
  always stored a number and never contained non-number characters.
  This ended up making the numericality validation useless, of
  course -- but it caused confusion because the test acted in a way
  I didn't expect.)

To satisfy the above:

* `allow_value` now raises a CouldNotSetAttributeError if the attribute
  rejects the value being set in *any* way.
* However, add a `ignoring_interference_by_writer` qualifier so that
  it is possible to manually override this behavior.
* Fix tests that are failing now because of this new change:
  * Fix tests for allow_value matcher
  * Fix tests for numericality matcher
  * Remove tests for numericality matcher + integer column
    * An integer column will typecast any non-integer value to an
      integer.
    * Because of the typecasting, our tests for the numericality matcher
      against an integer column don't quite work, because we can't
      really test what happens when the attribute is set to a
      non-integer value. Now that `allow_value` is more strict, we're
      getting a CouldNotSetAttributeError when attempting to do so.
    * The tests mentioned were originally added to ensure that we are
      handling RangeErrors that ActiveRecord used to emit. This doesn't
      happen anymore, so the tests aren't necessary anymore either.
  * Fix tests for acceptance matcher
  * Fix tests for absence matcher
2015-09-27 14:56:59 -06:00
Elliot Winkler e708789714 Fix failure message for numericality matcher
Secondary author: Mauro George <maurogot@gmail.com>

Sometimes the failure message did not always provide the reason for
failure. For instance, given this validation:

    validates :ano, numericality: { only_integer: true, greater_than_or_equal_to: 1900 }

this test:

    it { should validate_numericality_of(:ano).is_greater_than_or_equal_to(1900) }

would fail with the following:

    Did not expect errors  when ano is set to 1900.000000000001, got error:

as you can see, the failure message doesn't contain the validation
error.

In the process of making this fix, we changed how the matcher fails so
that it will so when the first submatcher fails, not the last. This
changes what the failure message looks like, but not the basic
functionality of the matcher itself.
2015-06-01 01:38:35 -06:00
Elliot Winkler 9748869091 Add #on qualifier to numericality matcher
This is present in all other matchers through ValidationMatcher.
However, ValidateNumericalityOfMatcher does not inherit from
ValidationMatcher. (This happens to be okay, since
ValidateNumericalityOfMatcher contains submatchers and we have to make
sure to pass the context all the way through.)

Hat tip @sj26 for most of the content of this commit, as well as
@anujbiyani for another approach to this.
2015-04-03 23:28:29 -06:00
Adam89 4e4c546d0d Added strict option to Numericality matcher 2015-03-29 00:06:03 -06:00
Elliot Winkler 388af8bfa5 Refactor specs for numericality matcher
Mainly, use some metaprogramming to add tests that combine the
qualifiers in various ways. This paves the way to add a #strict
qualifier and yet test its interaction with other qualifiers completely.
2015-03-28 18:59:49 -06:00
Elliot Winkler 9ba21381d7 Handle RangeErrors emitted now in ActiveRecord 4.2
In Rails 4.2, ActiveRecord was changed such that if you attempt to set
an attribute to a value and that value is outside the range of the
column, then it will raise a RangeError. For instance, an integer column
with a limit of 2 (i.e. a smallint) only accepts values between -32768
and +32767.

This means that if you try to do any of these three things, a RangeError
could be raised:

* Use validate_numericality_of along with any of the comparison
  submatchers and a value that sits on either side of the boundary.
* Use allow_value with a value that sits outside the range.
* Use validates_inclusion_of against an integer column. (Here we attempt
  to set that column to a non-integer value to verify that the attribute
  does not allow said value. That value is really a string version of a
  large number, so if the column does not take large numbers then the
  matcher could blow up.)

Ancillary changes in this commit:

* Remove ValidationMessageFinder and ExceptionMessageFinder in favor of
  Validator, StrictValidator, and ValidatorWithCapturedRangeError.
* The allow_value matcher now uses an instance of Validator under the
  hood. StrictValidator and/or ValidatorWithCapturedRangeError may be
  mixed into the Validator object as needed.
2015-01-22 21:05:09 -07:00
Elliot Winkler 84a366fbc5 Fix test that was written poorly 2015-01-22 21:05:08 -07:00
Elliot Winkler d86fe2d1c0 Fix test suite to properly tag example groups
When running unit tests we need to ensure that we are simulating how
other people will use the matchers as best we can. This means, for
instance, tagging any example groups that test controller matchers as
controller example groups.
2014-12-25 00:44:53 -05:00
Elliot Winkler 3a76f9389f Convert from Mocha to RSpec Mocks 2014-11-19 16:43:32 -07:00
Elliot Winkler f922613386 Reorganize unit tests, part II
* Change 'spec' Rake task to 'spec:unit'
* Require unit_spec_helper.rb in unit tests, not spec_helper.rb
* Re-namespace files in spec/support/unit under UnitTests
* Files in spec/support/unit/helpers no longer automatically add
  themselves to RSpec - this happens in unit_spec_helper.rb
* Extract RecordWithDifferentErrorAttributeBuilder and
  RecordValidatingConfirmationBuilder to separate files
2014-11-05 09:53:20 -07:00
Elliot Winkler bbdf8a807e Reorganize unit tests, part I
* Move spec/shoulda to spec/unit_tests/shoulda
* Move spec/support/*.rb to spec/support/unit_tests/{helpers,matchers}
* Move spec_helper.rb to unit_spec_helper.rb
2014-11-04 14:43:59 -07:00