Specs are failing
jpmoral opened this issue · 7 comments
Several specs are failing with
undefined method 'first' for #<Virtus::Attribute>
This seems to be because of this change in the equalizer gem:
old:
def ==(other)
other = coerce(other) if respond_to?(:coerce, true)
other.is_a?(self.class) && cmp?(__method__, other)
end
new:
def ==(other)
other = coerce(other).first if respond_to?(:coerce, true) # added call to 'first'
other.is_a?(self.class) && cmp?(__method__, other)
end
The spec trips up on trying to compare Virtus::Attribute
s, which don't have a first
method defined.
I'd be happy to attempt a fix if I knew more about each project.
We need our own Attribute#==
method. Equalizer calls #coerce
internally which in case of virtus attributes doesn't do the same thing as, let's say, Float#coerce
. This should fix the specs and the behavior of equality method too.
Is it as simple as going back to not using the equalizer gem?
On a side note, changing how the specs are written from:
expect { subject }.to change { object.reset.to_a }.from(attributes).to([ attribute ])
to:
expect(object.reset.to_a).to eq attributes
subject
expect(object.reset.to_a).to eq [attribute]
gets them passing. It's much more difficult to understand though.
We're discussing how to solve this on the Equalizer side, see related issue.
So I tried to look into it and it seems like we can implement a simple own matcher for comparing Virtus::Attribute
models (for Attribute
model type
and option
fields matching), but I thought I would wait for some movement on Equalizer side. Are you working on it already? If not, I would love to discuss it and help.
Actually, I implemented Attribute#==
method already - probably I can open a PR and we can merge if you like it, at least specs will be passing now (and some related bugs will be fixed I guess, since there should be some unreported ones related to that, if specs are failing). After that we can either wait for Equalizer fix or start using Virtus's own matchers everywhere (this will also remove the dependency, but that's a pretty good amount of potentially unnecessary work). Tell me your thoughts please, and nice project there!
@novikserg you may want to check out this thread: dkubb/equalizer#18 I'm not sure how to proceed with this, currently leaning towards going back to the built-in equalizer that was deprecated in 1.0.0 but now for pragmatic reasons I feel like it could be brought back (esp. that it would remove the need for values .. end
block in value_object
extension.
@solnic Alright, thanks for the info, I will send a PR in the following week or two.
Specs are fixed, closing.