From 5b31b56d6544339a6dd25311864e5385c1423abe Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Fri, 28 Sep 2018 16:36:07 -0400 Subject: [PATCH] Remove warning about setter methods This warning made sense when we had static attributes, since somebody might try to write something like: ```rb factory :composer do self.name = "Daniel" end ``` That would create a static declaration when the factory was defined, then raise the error about avoiding writers when the factory was run. Now this code will raise a NoMethodError right away when the factory is being defined. --- lib/factory_bot/attribute.rb | 34 ----------- lib/factory_bot/errors.rb | 5 +- spec/factory_bot/attribute_spec.rb | 7 --- spec/factory_bot/definition_proxy_spec.rb | 69 ++++++++++++++++------- 4 files changed, 49 insertions(+), 66 deletions(-) diff --git a/lib/factory_bot/attribute.rb b/lib/factory_bot/attribute.rb index 6cf0754..b193eea 100644 --- a/lib/factory_bot/attribute.rb +++ b/lib/factory_bot/attribute.rb @@ -10,7 +10,6 @@ module FactoryBot def initialize(name, ignored) @name = name.to_sym @ignored = ignored - ensure_non_attribute_writer! end def to_proc @@ -24,38 +23,5 @@ module FactoryBot def alias_for?(attr) FactoryBot.aliases_for(attr).include?(name) end - - private - - def ensure_non_attribute_writer! - NonAttributeWriterValidator.new(@name).validate! - end - - class NonAttributeWriterValidator - def initialize(method_name) - @method_name = method_name.to_s - @method_name_setter_match = @method_name.match(/(.*)=$/) - end - - def validate! - if method_is_writer? - raise AttributeDefinitionError, error_message - end - end - - private - - def method_is_writer? - !!@method_name_setter_match - end - - def attribute_name - @method_name_setter_match[1] - end - - def error_message - "factory_bot uses '#{attribute_name} value' syntax rather than '#{attribute_name} = value'" - end - end end end diff --git a/lib/factory_bot/errors.rb b/lib/factory_bot/errors.rb index 07692d3..9452bce 100644 --- a/lib/factory_bot/errors.rb +++ b/lib/factory_bot/errors.rb @@ -11,10 +11,7 @@ module FactoryBot # Raised when attempting to register a sequence from a dynamic attribute block class SequenceAbuseError < RuntimeError; end - # Raised when defining an invalid attribute: - # * Defining an attribute which has a name ending in "=" - # * Defining an attribute with both a static and lazy value - # * Defining an attribute twice in the same factory + # Raised when defining an attribute twice in the same factory class AttributeDefinitionError < RuntimeError; end # Raised when a method is defined in a factory or trait with arguments diff --git a/spec/factory_bot/attribute_spec.rb b/spec/factory_bot/attribute_spec.rb index 2330e6b..77de7c4 100644 --- a/spec/factory_bot/attribute_spec.rb +++ b/spec/factory_bot/attribute_spec.rb @@ -4,11 +4,4 @@ describe FactoryBot::Attribute do its(:name) { should eq name.to_sym } it { should_not be_association } - - it "raises an error when defining an attribute writer" do - error_message = %{factory_bot uses 'test value' syntax rather than 'test = value'} - expect { - FactoryBot::Attribute.new('test=', false) - }.to raise_error(FactoryBot::AttributeDefinitionError, error_message) - end end diff --git a/spec/factory_bot/definition_proxy_spec.rb b/spec/factory_bot/definition_proxy_spec.rb index ee6170e..fad7f7c 100644 --- a/spec/factory_bot/definition_proxy_spec.rb +++ b/spec/factory_bot/definition_proxy_spec.rb @@ -5,7 +5,8 @@ describe FactoryBot::DefinitionProxy, "#add_attribute" do it "declares a dynamic attribute on the factory" do attribute_value = -> { "dynamic attribute" } proxy.add_attribute(:attribute_name, &attribute_value) - expect(subject).to have_dynamic_declaration(:attribute_name).with_value(attribute_value) + expect(subject).to have_dynamic_declaration(:attribute_name). + with_value(attribute_value) end end @@ -16,7 +17,9 @@ describe FactoryBot::DefinitionProxy, "#add_attribute when the proxy ignores att it "declares a dynamic attribute on the factory" do attribute_value = -> { "dynamic attribute" } proxy.add_attribute(:attribute_name, &attribute_value) - expect(subject).to have_dynamic_declaration(:attribute_name).ignored.with_value(attribute_value) + expect(subject).to have_dynamic_declaration(:attribute_name). + ignored. + with_value(attribute_value) end end @@ -40,31 +43,54 @@ describe FactoryBot::DefinitionProxy, "#method_missing" do subject { FactoryBot::Definition.new(:name) } let(:proxy) { FactoryBot::DefinitionProxy.new(subject) } - it "declares an implicit declaration without args or a block" do - proxy.bogus - expect(subject).to have_implicit_declaration(:bogus).with_factory(subject) + context "when called without args or a block" do + it "declares an implicit declaration" do + proxy.bogus + expect(subject).to have_implicit_declaration(:bogus).with_factory(subject) + end end - it "declares an association when :factory is passed" do - proxy.author factory: :user - expect(subject).to have_association_declaration(:author).with_options(factory: :user) + context "when called with a ':factory' key" do + it "declares an association" do + proxy.author factory: :user + expect(subject).to have_association_declaration(:author). + with_options(factory: :user) + end end - it "declares a dynamic attribute" do - attribute_value = -> { "dynamic attribute" } - proxy.attribute_name(&attribute_value) - expect(subject).to have_dynamic_declaration(:attribute_name).with_value(attribute_value) + context "when called with a block" do + it "declares a dynamic attribute" do + attribute_value = -> { "dynamic attribute" } + proxy.attribute_name(&attribute_value) + expect(subject).to have_dynamic_declaration(:attribute_name). + with_value(attribute_value) + end end - it "raises a NoMethodError" do - definition = FactoryBot::Definition.new(:broken) - proxy = FactoryBot::DefinitionProxy.new(definition) + context "when called with a static-attribute-like argument" do + it "raises a NoMethodError" do + definition = FactoryBot::Definition.new(:broken) + proxy = FactoryBot::DefinitionProxy.new(definition) - invalid_call = -> { proxy.static_attributes_are_gone true } - expect(invalid_call).to raise_error( - NoMethodError, - "undefined method 'static_attributes_are_gone' in 'broken' factory", - ) + invalid_call = -> { proxy.static_attributes_are_gone true } + expect(invalid_call).to raise_error( + NoMethodError, + "undefined method 'static_attributes_are_gone' in 'broken' factory", + ) + end + end + + context "when called with a setter method" do + it "raises a NoMethodError" do + definition = FactoryBot::Definition.new(:broken) + proxy = FactoryBot::DefinitionProxy.new(definition) + + invalid_call = -> { proxy.setter_method = true } + expect(invalid_call).to raise_error( + NoMethodError, + "undefined method 'setter_method=' in 'broken' factory", + ) + end end end @@ -115,7 +141,8 @@ describe FactoryBot::DefinitionProxy, "#association" do it "declares an association with options" do proxy.association(:association_name, { name: "Awesome" }) - expect(subject).to have_association_declaration(:association_name).with_options(name: "Awesome") + expect(subject).to have_association_declaration(:association_name). + with_options(name: "Awesome") end end