From 5f33f8147fc9d84e69a33758b000486ebe42c8ae Mon Sep 17 00:00:00 2001 From: Sergey Novik Date: Sun, 19 Jul 2015 18:11:07 +0300 Subject: [PATCH 1/3] Add specs for attributes comparison (based on latest Equalizer gem working version) --- spec/unit/virtus/attribute/comparison_spec.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 spec/unit/virtus/attribute/comparison_spec.rb diff --git a/spec/unit/virtus/attribute/comparison_spec.rb b/spec/unit/virtus/attribute/comparison_spec.rb new file mode 100644 index 0000000..0b51a9c --- /dev/null +++ b/spec/unit/virtus/attribute/comparison_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Virtus::Attribute, '#== (defined by including Virtus::Equalizer)' do + let(:attribute) { described_class.build(String, :name => :name) } + + # Currently that's the way it works and it happens because default_value objects + # don't have equalizer, resulting in attributes object mismatch. + # This behavior (and a spec) will need a change in future. + it 'returns false when attributes have same type and options' do + equal_attribute = described_class.build(String, :name => :name) + expect(attribute == equal_attribute).to be_falsey + end + + it 'returns false when attributes have different type' do + different_attribute = described_class.build(Integer, :name => :name) + expect(attribute == different_attribute).to be_falsey + end + + it 'returns false when attributes have different options' do + different_attribute = described_class.build(Integer, :name => :name_two) + expect(attribute == different_attribute).to be_falsey + end +end From 3748f6eb2cf368d6a78fb1ef50c2724648d96a9b Mon Sep 17 00:00:00 2001 From: Sergey Novik Date: Sun, 19 Jul 2015 18:11:24 +0300 Subject: [PATCH 2/3] Use Virtus::Equalizer instead of Equalizer gem in Virtus#Attribute (behavior preserved) --- lib/virtus/attribute.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/virtus/attribute.rb b/lib/virtus/attribute.rb index 999a2ef..0654d4d 100644 --- a/lib/virtus/attribute.rb +++ b/lib/virtus/attribute.rb @@ -18,7 +18,7 @@ module Virtus class Attribute extend DescendantsTracker, Options, TypeLookup - include ::Equalizer.new(:type, :options) + include Equalizer.new(inspect) << :type << :options accept_options :primitive, :accessor, :default, :lazy, :strict, :required, :finalize, :nullify_blank From ef57af319334a1d4f3e0860acbde7c6d6f0eb8ef Mon Sep 17 00:00:00 2001 From: Sergey Novik Date: Sun, 19 Jul 2015 18:26:50 +0300 Subject: [PATCH 3/3] Change behavior of Attribute#== method By changing Coercer#== and DefaultValue#== methods, we allow Attribute#== to actually compare objects now (before, every comparison would return `false` because of different instances of DefaultValue model in options[:default_value] key. --- lib/virtus/attribute/default_value.rb | 2 ++ lib/virtus/coercer.rb | 1 + spec/unit/virtus/attribute/comparison_spec.rb | 7 ++----- spec/unit/virtus/attribute_set/append_spec.rb | 8 ++++---- spec/unit/virtus/attribute_set/element_set_spec.rb | 22 +++++++++++++++------- spec/unit/virtus/attribute_set/merge_spec.rb | 8 +++++--- 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/lib/virtus/attribute/default_value.rb b/lib/virtus/attribute/default_value.rb index a2fcd31..eca7350 100644 --- a/lib/virtus/attribute/default_value.rb +++ b/lib/virtus/attribute/default_value.rb @@ -7,6 +7,8 @@ class Attribute class DefaultValue extend DescendantsTracker + include Equalizer.new(inspect) << :value + # Builds a default value instance # # @return [Virtus::Attribute::DefaultValue] diff --git a/lib/virtus/coercer.rb b/lib/virtus/coercer.rb index a06e273..676d553 100644 --- a/lib/virtus/coercer.rb +++ b/lib/virtus/coercer.rb @@ -3,6 +3,7 @@ module Virtus # Abstract coercer class # class Coercer + include Equalizer.new(inspect) << :primitive << :type # @api private attr_reader :primitive, :type diff --git a/spec/unit/virtus/attribute/comparison_spec.rb b/spec/unit/virtus/attribute/comparison_spec.rb index 0b51a9c..796cd20 100644 --- a/spec/unit/virtus/attribute/comparison_spec.rb +++ b/spec/unit/virtus/attribute/comparison_spec.rb @@ -3,12 +3,9 @@ describe Virtus::Attribute, '#== (defined by including Virtus::Equalizer)' do let(:attribute) { described_class.build(String, :name => :name) } - # Currently that's the way it works and it happens because default_value objects - # don't have equalizer, resulting in attributes object mismatch. - # This behavior (and a spec) will need a change in future. - it 'returns false when attributes have same type and options' do + it 'returns true when attributes have same type and options' do equal_attribute = described_class.build(String, :name => :name) - expect(attribute == equal_attribute).to be_falsey + expect(attribute == equal_attribute).to be_truthy end it 'returns false when attributes have different type' do diff --git a/spec/unit/virtus/attribute_set/append_spec.rb b/spec/unit/virtus/attribute_set/append_spec.rb index 7fbb20a..577b07e 100644 --- a/spec/unit/virtus/attribute_set/append_spec.rb +++ b/spec/unit/virtus/attribute_set/append_spec.rb @@ -38,10 +38,10 @@ it { is_expected.to equal(object) } - it 'replaces the original attribute' do - expect { subject }.to change { object.to_a }. - from(attributes). - to([ attribute ]) + it "replaces the original attribute object" do + expect { subject }.to change { object.to_a.map(&:__id__) }. + from(attributes.map(&:__id__)). + to([attribute.__id__]) end end end diff --git a/spec/unit/virtus/attribute_set/element_set_spec.rb b/spec/unit/virtus/attribute_set/element_set_spec.rb index 5db7e41..8d9c638 100644 --- a/spec/unit/virtus/attribute_set/element_set_spec.rb +++ b/spec/unit/virtus/attribute_set/element_set_spec.rb @@ -37,20 +37,28 @@ it { is_expected.to equal(attribute) } - it 'replaces the original attribute' do - expect { subject }.to change { object.to_a }.from(attributes).to([ attribute ]) + it "replaces the original attribute object" do + expect { subject }.to change { object.to_a.map(&:__id__) }. + from(attributes.map(&:__id__)). + to([attribute.__id__]) end - it 'allows #[] to access the attribute with a symbol' do - expect { subject }.to change { object['name'] }.from(original).to(attribute) + it 'allows #[] to access the attribute with a string' do + expect { subject }.to change { object['name'].__id__ }. + from(original.__id__). + to(attribute.__id__) end - it 'allows #[] to access the attribute with a string' do - expect { subject }.to change { object[:name] }.from(original).to(attribute) + it 'allows #[] to access the attribute with a symbol' do + expect { subject }.to change { object[:name].__id__ }. + from(original.__id__). + to(attribute.__id__) end it 'allows #reset to track overridden attributes' do - expect { subject }.to change { object.reset.to_a }.from(attributes).to([ attribute ]) + expect { subject }.to change { object.reset.to_a.map(&:__id__) }. + from(attributes.map(&:__id__)). + to([attribute.__id__]) end end end diff --git a/spec/unit/virtus/attribute_set/merge_spec.rb b/spec/unit/virtus/attribute_set/merge_spec.rb index 72dc39c..9981ece 100644 --- a/spec/unit/virtus/attribute_set/merge_spec.rb +++ b/spec/unit/virtus/attribute_set/merge_spec.rb @@ -21,12 +21,14 @@ context 'with a duplicate attribute' do let(:attributes) { [Virtus::Attribute.build(String, :name => name)] } - let(:attribute) { Virtus::Attribute.build(String, :name => name) } + let(:attribute) { Virtus::Attribute.build(String, :name => name) } it { is_expected.to equal(object) } - it 'replaces the original attribute' do - expect { subject }.to change { object.to_a }.from(attributes).to([attribute]) + it "replaces the original attribute object" do + expect { subject }.to change { object.to_a.map(&:__id__) }. + from(attributes.map(&:__id__)). + to([attribute.__id__]) end end end