From 645afa9689e8812c52387bd124a1ba28677f54c0 Mon Sep 17 00:00:00 2001 From: Joshua Clayton Date: Sun, 6 May 2012 16:56:37 -0400 Subject: [PATCH] Fix to_create so it works when defined in a trait --- lib/factory_girl.rb | 1 + lib/factory_girl/definition.rb | 24 +++++----- lib/factory_girl/definition_list.rb | 27 +++++++++++ lib/factory_girl/evaluation.rb | 2 +- lib/factory_girl/factory.rb | 16 ++++--- lib/factory_girl/null_factory.rb | 2 +- lib/factory_girl/trait.rb | 2 +- spec/acceptance/traits_spec.rb | 68 ++++++++++++++++++++++++++++ spec/factory_girl/definition_spec.rb | 38 +++++++++------- spec/factory_girl/factory_spec.rb | 8 ++-- 10 files changed, 148 insertions(+), 40 deletions(-) create mode 100644 lib/factory_girl/definition_list.rb diff --git a/lib/factory_girl.rb b/lib/factory_girl.rb index b964683..c59e844 100644 --- a/lib/factory_girl.rb +++ b/lib/factory_girl.rb @@ -30,6 +30,7 @@ require 'factory_girl/attribute_list' require 'factory_girl/trait' require 'factory_girl/aliases' require 'factory_girl/definition' +require 'factory_girl/definition_list' require 'factory_girl/definition_proxy' require 'factory_girl/syntax' require 'factory_girl/syntax_runner' diff --git a/lib/factory_girl/definition.rb b/lib/factory_girl/definition.rb index 0df0428..100932c 100644 --- a/lib/factory_girl/definition.rb +++ b/lib/factory_girl/definition.rb @@ -7,7 +7,7 @@ module FactoryGirl @declarations = DeclarationList.new(name) @callbacks = [] @defined_traits = [] - @to_create = default_to_create + @to_create = nil @base_traits = base_traits @additional_traits = [] @constructor = default_constructor @@ -23,8 +23,10 @@ module FactoryGirl attributes end - def processing_order - base_traits + [self] + additional_traits + def definition_list + DefinitionList.new( + base_traits.map(&:definition) + [self] + additional_traits.map(&:definition) + ) end def overridable @@ -33,6 +35,10 @@ module FactoryGirl end def inherit_traits(new_traits) + @base_traits += new_traits + end + + def append_traits(new_traits) @additional_traits += new_traits end @@ -40,6 +46,10 @@ module FactoryGirl @callbacks << callback end + def compiled_to_create + definition_list.to_create + end + def to_create(&block) if block_given? @to_create = block @@ -60,20 +70,12 @@ module FactoryGirl @constructor != default_constructor end - def custom_to_create? - @to_create != default_to_create - end - private def default_constructor @default_constructor ||= -> { new } end - def default_to_create - @default_to_create ||= ->(instance) { instance.save! } - end - def base_traits @base_traits.map { |name| trait_by_name(name) } end diff --git a/lib/factory_girl/definition_list.rb b/lib/factory_girl/definition_list.rb new file mode 100644 index 0000000..bd6fb23 --- /dev/null +++ b/lib/factory_girl/definition_list.rb @@ -0,0 +1,27 @@ +module FactoryGirl + class DefinitionList + include Enumerable + + def initialize(definitions = []) + @definitions = definitions + end + + def each(&block) + @definitions.each &block + end + + def callbacks + map(&:callbacks).flatten + end + + def attributes + map {|definition| definition.attributes.to_a }.flatten + end + + def to_create + map(&:to_create).compact.last + end + + delegate :[], :==, to: :@definitions + end +end diff --git a/lib/factory_girl/evaluation.rb b/lib/factory_girl/evaluation.rb index 55adc7f..252817f 100644 --- a/lib/factory_girl/evaluation.rb +++ b/lib/factory_girl/evaluation.rb @@ -6,7 +6,7 @@ module FactoryGirl def initialize(attribute_assigner, to_create) @attribute_assigner = attribute_assigner - @to_create = to_create + @to_create = to_create || ->(instance) { instance.save! } end delegate :object, :hash, to: :@attribute_assigner diff --git a/lib/factory_girl/factory.rb b/lib/factory_girl/factory.rb index 0bcdbeb..50fdfdf 100644 --- a/lib/factory_girl/factory.rb +++ b/lib/factory_girl/factory.rb @@ -17,7 +17,7 @@ module FactoryGirl end delegate :add_callback, :declare_attribute, :to_create, :define_trait, - :defined_traits, :inherit_traits, :processing_order, to: :@definition + :defined_traits, :inherit_traits, :append_traits, :definition_list, to: :@definition def build_class @build_class ||= if class_name.is_a? Class @@ -36,7 +36,7 @@ module FactoryGirl evaluator = evaluator_class.new(build_class, strategy, overrides.symbolize_keys) attribute_assigner = AttributeAssigner.new(evaluator, build_class, &constructor) - evaluation = Evaluation.new(attribute_assigner, to_create) + evaluation = Evaluation.new(attribute_assigner, compiled_to_create) evaluation.add_observer(CallbacksObserver.new(callbacks, evaluator)) strategy.result(evaluation).tap(&block) @@ -90,12 +90,16 @@ module FactoryGirl def with_traits(traits) self.clone.tap do |factory_with_traits| - factory_with_traits.inherit_traits traits + factory_with_traits.append_traits traits end end protected + def compiled_to_create + @definition.compiled_to_create || parent.compiled_to_create + end + def class_name @class_name || parent.class_name || name end @@ -107,14 +111,12 @@ module FactoryGirl def attributes compile AttributeList.new(@name).tap do |list| - processing_order.each do |factory| - list.apply_attributes factory.attributes - end + list.apply_attributes definition_list.attributes end end def callbacks - parent.callbacks + processing_order.map {|factory| factory.callbacks }.flatten + parent.callbacks + definition_list.callbacks end def constructor diff --git a/lib/factory_girl/null_factory.rb b/lib/factory_girl/null_factory.rb index d202da8..39b1b77 100644 --- a/lib/factory_girl/null_factory.rb +++ b/lib/factory_girl/null_factory.rb @@ -7,7 +7,7 @@ module FactoryGirl @definition = Definition.new end - delegate :defined_traits, :callbacks, :attributes, :constructor, to: :definition + delegate :defined_traits, :callbacks, :attributes, :constructor, :compiled_to_create, to: :definition def compile; end def class_name; end diff --git a/lib/factory_girl/trait.rb b/lib/factory_girl/trait.rb index 29b36e5..844a208 100644 --- a/lib/factory_girl/trait.rb +++ b/lib/factory_girl/trait.rb @@ -1,7 +1,7 @@ module FactoryGirl # @api private class Trait - attr_reader :name + attr_reader :name, :definition def initialize(name, &block) @name = name diff --git a/spec/acceptance/traits_spec.rb b/spec/acceptance/traits_spec.rb index 666168c..9d4d2ce 100644 --- a/spec/acceptance/traits_spec.rb +++ b/spec/acceptance/traits_spec.rb @@ -419,3 +419,71 @@ describe "making sure the factory is properly compiled the first time we want to user.role.should == 'admin' end end + +describe "traits with other than attributes or callbacks defined" do + before do + define_model("User", name: :string) + + FactoryGirl.define do + factory :user do + trait :with_to_create do + to_create {|instance| instance.name = "to_create" } + end + + factory :sub_user do + to_create {|instance| instance.name = "sub" } + + factory :child_user + end + + factory :sub_user_with_trait do + with_to_create + + factory :child_user_with_trait + end + + factory :sub_user_with_trait_and_override do + with_to_create + to_create {|instance| instance.name = "sub with trait and override" } + + factory :child_user_with_trait_and_override + end + end + end + end + + it "can apply to_create from traits" do + FactoryGirl.create(:user, :with_to_create).name.should == "to_create" + end + + it "can apply to_create from the definition" do + FactoryGirl.create(:sub_user).name.should == "sub" + FactoryGirl.create(:child_user).name.should == "sub" + end + + it "gives additional traits higher priority than to_create from the definition" do + FactoryGirl.create(:sub_user, :with_to_create).name.should == "to_create" + FactoryGirl.create(:child_user, :with_to_create).name.should == "to_create" + end + + it "gives base traits normal priority" do + FactoryGirl.create(:sub_user_with_trait).name.should == "to_create" + FactoryGirl.create(:child_user_with_trait).name.should == "to_create" + end + + it "gives base traits lower priority than overrides" do + FactoryGirl.create(:sub_user_with_trait_and_override).name.should == "sub with trait and override" + FactoryGirl.create(:child_user_with_trait_and_override).name.should == "sub with trait and override" + end + + it "gives additional traits higher priority than base traits and factory definition" do + FactoryGirl.define do + trait :overridden do + to_create {|instance| instance.name = "completely overridden" } + end + end + + FactoryGirl.create(:sub_user_with_trait_and_override, :overridden).name.should == "completely overridden" + FactoryGirl.create(:child_user_with_trait_and_override, :overridden).name.should == "completely overridden" + end +end diff --git a/spec/factory_girl/definition_spec.rb b/spec/factory_girl/definition_spec.rb index 70a77d9..017a763 100644 --- a/spec/factory_girl/definition_spec.rb +++ b/spec/factory_girl/definition_spec.rb @@ -49,13 +49,7 @@ describe FactoryGirl::Definition, "adding callbacks" do end describe FactoryGirl::Definition, "#to_create" do - its(:to_create) { should be_a(Proc) } - - it "calls save! on the object when run" do - instance = stub("model instance", :save! => true) - subject.to_create[instance] - instance.should have_received(:save!).once - end + its(:to_create) { should be_nil } it "returns the assigned value when given a block" do block = proc { nil } @@ -64,9 +58,11 @@ describe FactoryGirl::Definition, "#to_create" do end end -describe FactoryGirl::Definition, "#processing_order" do +describe FactoryGirl::Definition, "#definition_list" do let(:female_trait) { FactoryGirl::Trait.new(:female) } let(:admin_trait) { FactoryGirl::Trait.new(:admin) } + let(:female_definition) { female_trait.definition } + let(:admin_definition) { admin_trait.definition } before do subject.define_trait(female_trait) @@ -75,17 +71,22 @@ describe FactoryGirl::Definition, "#processing_order" do context "without base traits" do it "returns the definition without any traits" do - subject.processing_order.should == [subject] + subject.definition_list.should == [subject] end - it "finds the correct traits after inheriting" do + it "finds the correct definitions after appending" do + subject.append_traits([:female]) + subject.definition_list.should == [subject, female_definition] + end + + it "finds the correct definitions after inheriting" do subject.inherit_traits([:female]) - subject.processing_order.should == [subject, female_trait] + subject.definition_list.should == [female_definition, subject] end it "looks for the trait on FactoryGirl" do - subject.inherit_traits([:female, :admin]) - subject.processing_order.should == [subject, female_trait, admin_trait] + subject.append_traits([:female, :admin]) + subject.definition_list.should == [subject, female_definition, admin_definition] end end @@ -93,12 +94,17 @@ describe FactoryGirl::Definition, "#processing_order" do subject { FactoryGirl::Definition.new("my definition", [:female]) } it "returns the base traits and definition" do - subject.processing_order.should == [female_trait, subject] + subject.definition_list.should == [female_definition, subject] end - it "finds the correct traits after inheriting" do + it "finds the correct definitions after appending" do + subject.append_traits([:admin]) + subject.definition_list.should == [female_definition, subject, admin_definition] + end + + it "finds the correct definitions after inheriting" do subject.inherit_traits([:admin]) - subject.processing_order.should == [female_trait, subject, admin_trait] + subject.definition_list.should == [female_definition, admin_definition, subject] end end end diff --git a/spec/factory_girl/factory_spec.rb b/spec/factory_girl/factory_spec.rb index 0c7571a..4ac6725 100644 --- a/spec/factory_girl/factory_spec.rb +++ b/spec/factory_girl/factory_spec.rb @@ -262,18 +262,20 @@ describe FactoryGirl::Factory, "#with_traits" do subject { FactoryGirl::Factory.new(:user) } let(:admin_trait) { FactoryGirl::Trait.new(:admin) } let(:female_trait) { FactoryGirl::Trait.new(:female) } + let(:admin_definition) { admin_trait.definition } + let(:female_definition) { female_trait.definition } before do FactoryGirl.register_trait(admin_trait) FactoryGirl.register_trait(female_trait) end - it "returns a factory with the correct traits" do - subject.with_traits([:admin, :female]).processing_order[1, 2].should == [admin_trait, female_trait] + it "returns a factory with the correct definitions" do + subject.with_traits([:admin, :female]).definition_list[1, 2].should == [admin_definition, female_definition] end it "doesn't modify the original factory's processing order" do subject.with_traits([:admin, :female]) - subject.processing_order.should == [subject.definition] + subject.definition_list.should == [subject.definition] end end