Fix errors method check in be_valid matcher

[kkuchta]

Bug report + bug fix PR.

If the .be_valid? matcher fails, it checks for the presence of a
.errors method and uses it, if present, in the failure message. This
works great, unless the subject happens to include an errors method
that takes an argument, in which case we get an error (because we call
errors with 0 arguments).

Here's a simple repro case which'll result in wrong number of arguments (given 0, expected 1) for the .errors method.

  it 'foo' do
    some_obj = Object.new
    def some_obj.valid?() false; end
    def some_obj.errors(foo) 'bar'; end

    expect(some_obj).to be_valid
  end

I'd propose to fix this with an arity check (this PR). I've included a spec that fails without this fix (piggybacking on an existing spec that already tested the general case of an object with a valid? method but no errors method).

[JonRowe]

I'd like an extra spec here, and for the majority the boolean can be collapsed. You will need to see if theres a solution for the 1.8.7 failure too unless you're happy to wait for RSpec Rails 4.

[JonRowe]

You don't need this duplicate methods check, if if actual.respond_to?(:errors) && actual.method(:errors).arity < 1 will suffice.

[kkuchta]

I added the methods.include? check because, given how many ways there are to define functions in ruby, I figured there was probably some way or another to have an object not work with the original check.

When I actually tried to construct such an object, I found that it is possible, but it's pretty edge-casey:

class A
  def respond_to_missing?(name, _)
    return true if name.to_sym == :foo
    super
  end
end
a = A.new
a.respond_to?(:foo) # true
a.method(:foo).arity # -1
a.foo # NoMethodError: undefined method `foo' for #<A:0x00007fd60c168430>
a.methods.include?(:foo) # false

So with a class defined like that, we could still run into errors in this code without the .include? check. Although if you think that's too much of an edge-case to bother accounting for, I'm happy to remove it.

[JonRowe]

We don't support classes that don't behave according to ruby semantics, if someone uses the trick you mention it won't work with the matcher and thats ok, it'll just return false. If you add the correct method_missing definition to go with that snippet the method should be returned.

[kkuchta]

Thanks for the feedback! I'll break this case out into a separate spec and see if I can get it working in 1.8.7.

[kkuchta]

@JonRowe - I think I've addressed all your comments. Thoughts?

[benoittgt]

I reran the one failing build. LGTM and it works on old Ruby.

Thanks for the PR. 🎉