Open krainboltgreene opened 9 years ago
What output do you currently get?
Currently pairing, will get some output when I can.
The error message is:
Failure/Error: expect(errors).to include(name: "can't be blank")
expected #<ActiveModel::Errors:0x007ff541a11088 @base=#<Widget id: nil, name: nil>, @messages={:name => ["can't be blank"]}> to include {:name => "can't be blank"}
Diff:
@@ -1,2 +1,2 @@
-[{:name=>"can't be blank"}]
+[:name]
The issue is ActiveModel::Errors
has a custom implementation of include?
:
def include?(attribute)
messages[attribute].present?
end
Which only checks if the attribute is a key for the messages. Thus the diff says it has [:name]
.
Also, note that messages
is a hash of arrays. So this will work:
expect(errors.messages).to include(name: ["can't be blank"])
expect(errors.messages).to include(name: including("can't be blank"))
The root issue is that the RSpec::Matchers::BuiltIn::Include
matcher has special case logic if the actual object is_a?(Hash)
. ActiveModel::Errors
is not a hash:
errors.is_a?(Hash)
# => false
However, it acts as a hash. More specifically, it contains custom implementations for many methods (such as include?
) which behave similar to how a Hash
instance would behave. ActiveModel::Errors
does implement to_hash
; the Ruby implicit conversion method for when an object "can be a hash".
@myronmarston Perhaps we could consider improving the matcher to check both is_a?(Hash) || respond_to?(:to_hash)
and use the special case hash logic there? (I'm happy to submit a PR this weekend for this)
ActiveModel::Errors
does implementto_hash
; the Ruby implicit conversion method for when an object "can be a hash".
I thought that to_h
was the conversion protocol? When Ruby 2.0 came out, it was announced that was the official conversion method that was going in core:
http://blog.marc-andre.ca/2013/02/23/ruby-2-by-example/#to_h
I've implemented to_hash
on objects (before to_h
was a thing) but I thought of it just as a convenience and not as one of Ruby's built-in protocols.
Regardless, relying on to_h
is a bit problematic; while it works on some arrays, it fails on others:
irb(main):003:0> [[:a, 1], [:b, 2]].to_h
=> {:a=>1, :b=>2}
irb(main):004:0> [:a].to_h
TypeError: wrong element type Symbol at 0 (expected array)
from (irb):4:in `to_h'
from (irb):4
from /Users/myron/.rubies/ruby-2.2.0/bin/irb:11:in `<main>'
Thus we can't assume it'll always work. Given the complexities around coercing to a hash, I think going that direction is potentially problematic. Maybe we can improve the include
matcher so that under the following conditions:
actual
is not a hash:key => value
has been given as the expected inclusionactual.include?(:key => value)
returns falseactual.include?(:key)
returns true...we include a note in the failure message about the fact that the object is not a hash and they may need to convert it to one.
My understanding is there are two protocols:
explicit (to_h
)
Returns
self
. If called on a subclass of Hash, converts the receiver to a Hash object.
implicit (to_hash
)
Returns
self
.
In Ruby 1.8.7 only to_hash
exists.
Explicit is for when a thing, can potentially convert to the core type. Implicit is for when a thing, can be the actual core type. I think the main source I got this from is Avdi's "Confident Ruby".
Huh, I never new about that. Sounds like to_hash
is the way to go.
Yeah that's news to me.
On Sat, May 16, 2015 at 3:59 PM, Myron Marston notifications@github.com wrote:
Huh, I never new about that. Sounds like to_hash is the way to go.
— Reply to this email directly or view it on GitHub https://github.com/rspec/rspec-rails/issues/1367#issuecomment-102694190.
Kurtis Rainbolt-Greene, Hacker Software Developer 1631 8th St. New Orleans, LA 70115
Started looking into implementing this. It's a bit trickier than I initially anticipated due to the matcher being diffable.
I have it mostly working. On a failing example the output is:
Failure/Error: expect(implicit_hash_no_match).to include_key_matcher
expected #<ImplicitHash:0x007ff8da2fe718 @hash={:a => 15}> to include {:a => (a value within 3 of 10)}
Diff:
@@ -1,2 +1,2 @@
-[{:a=>(a value within 3 of 10)}]
+#<ImplicitHash:0x007ff8da2fe718 @hash={:a=>15}>
Though I'd prefer it to be:
Failure/Error: expect(implicit_hash_no_match).to include_key_matcher
expected #<ImplicitHash:0x007ff8da2fe718 @hash={:a => 15}> to include {:a => (a value within 3 of 10)}
Diff:
@@ -1,2 +1,2 @@
-[{:a=>(a value within 3 of 10)}]
+:a => 15,
Just debating implementation details I guess.
Shouldn't it be?
expect(model.errors.messages).to include(name: include("can't be blank"))
Its always several messages IIRC
You're right 👍 Fixed
The original issue is because the errors object is hash like, but not a hash, so this is one of our matchers that could (should?) be improved in rspec-rails
The error might not be quite that, but basically that.