validate_numericality_of on failure may not provide the reason for failure

[mcmire]

Given this validation

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

and this test:

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

It will fail with:

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.

I believe this is because none of the submatchers for ValidateNumericalityOfMatcher set a default message.

Really, all of the submatchers should share the same message, and updating that message should propagate to all submatchers.

[jacobsimeon]

I did some digging around on this one and here's what I found: Using the
validates shorthand for an attribute can lead to a false negative when there
are multiple numericality options being passed but they aren't all being tested.

For example, this test passes:
(Note that the only_integer option is present in the validation, but it is not
being tested)

it "passes when `only_integer` option is also specified (no shorthand)" do
  the_model =  define_model :example, attr: :string do
    validates_numericality_of(
      :attr,
      only_integer: true,
      greater_than_or_equal_to: 1900
    )
  end.new

  match = matcher.is_greater_than_or_equal_to(1900)
  expect(the_model).to(match)
end

This test uses the same effective validation but declares it using the
generic validates method and passing in multiple options. It fails with this
error message:

Did not expect errors when attr is set to 1900.000001, got error:

it "passes when `only_integer` option is also specified using shorthand" do
  the_model =  define_model :example, attr: :string do
    validates :attr, numericality: {
      only_integer: true,
      greater_than_or_equal_to: 1900
    }
  end.new

  match = matcher.is_greater_than_or_equal_to(1900)
  expect(the_model).to(match)
end

So, I'm wondering if the correct behavior of the failing test should actually be
for it to pass, since the validation is correctly declared. This may not be
feasible, so an alternative solution would be to provide a more meaningful error
message.

[jacobsimeon]

Ok, looks like this is related to #481

[mcmire]

Yes this issue is just related to the failure message and and not the actual behavior of the submatcher.

[maurogeorge]

@mcmire I think we can close this issue. The specific matcher has the correct message today as you can see here.

In all this file we have assertions on the message, just search for fail_with_message_including.

And the submatchers has the test for his messages too 1, 2, 3 and 4.

[mcmire]

The code does look correct (I'd expected this to be present and indeed it is, and has been there for a while), but I'd like to verify this in a test, because I think we only test the message partially (we don't test the "got error..." part of the message).

[maurogeorge]

@mcmire I started this on maurogeorge@5710a64. But I think I got a issue when we test against two or more qualifiers and the fail qualifier is not the last in the chain of methods, the message is showed like:

Expected errors to include "must be an integer" when attr is set to 0.1,
got errors:

The cause of the error is not showed.

[maurogeorge]

@mcmire I think we can close this since the #699 was merged 😄

[mcmire]

Yes, you're right :)