Fix issues with inline traits not taking precedence

Closes #242

lib/factory_girl/definition.rb

@@ -2,12 +2,13 @@ module FactoryGirl
   class Definition
     attr_reader :callbacks, :defined_traits, :declarations
 
-    def initialize(name = nil)
-      @declarations   = DeclarationList.new(name)
-      @callbacks      = []
-      @defined_traits = []
-      @to_create      = lambda {|instance| instance.save! }
-      @traits         = []
+    def initialize(name = nil, base_traits = [])
+      @declarations      = DeclarationList.new(name)
+      @callbacks         = []
+      @defined_traits    = []
+      @to_create         = lambda {|instance| instance.save! }
+      @base_traits       = base_traits
+      @additional_traits = []
     end
 
     delegate :declare_attribute, :to => :declarations
@@ -20,17 +21,17 @@ module FactoryGirl
       attributes
     end
 
+    def processing_order
+      base_traits + [self] + additional_traits
+    end
+
     def overridable
       declarations.overridable
       self
     end
 
-    def traits
-      @traits.reverse.map { |name| trait_by_name(name) }
-    end
-
     def inherit_traits(new_traits)
-      @traits += new_traits
+      @additional_traits += new_traits
     end
 
     def add_callback(callback)
@@ -51,6 +52,14 @@ module FactoryGirl
 
     private
 
+    def base_traits
+      @base_traits.map { |name| trait_by_name(name) }
+    end
+
+    def additional_traits
+      @additional_traits.map { |name| trait_by_name(name) }
+    end
+
     def trait_by_name(name)
       trait_for(name) || FactoryGirl.trait_by_name(name)
     end

lib/factory_girl/factory.rb

@@ -12,14 +12,12 @@ module FactoryGirl
       @aliases          = options[:aliases] || []
       @class_name       = options[:class]
       @default_strategy = options[:default_strategy]
-      @definition       = Definition.new(@name)
+      @definition       = Definition.new(@name, options[:traits] || [])
       @compiled         = false
-
-      inherit_traits(options[:traits] || [])
     end
 
     delegate :add_callback, :declare_attribute, :to_create, :define_trait,
-             :defined_traits, :traits, :inherit_traits, :to => :@definition
+             :defined_traits, :inherit_traits, :processing_order, :to => :@definition
 
     def factory_name
       $stderr.puts "DEPRECATION WARNING: factory.factory_name is deprecated; use factory.name instead."
@@ -127,10 +125,6 @@ module FactoryGirl
 
     private
 
-    def processing_order
-      [traits.reverse, @definition].flatten
-    end
-
     def assert_valid_options(options)
       options.assert_valid_keys(:class, :parent, :default_strategy, :aliases, :traits)
 

spec/acceptance/traits_spec.rb

@@ -312,3 +312,51 @@ describe "applying inline traits" do
     FactoryGirl.create(:user, :with_post).posts.should_not be_empty
   end
 end
+
+describe "inline traits overriding existing attributes" do
+  before do
+    define_model("User", :status => :string)
+
+    FactoryGirl.define do
+      factory :user do
+        status "pending"
+
+        trait(:accepted) { status "accepted" }
+        trait(:declined) { status "declined" }
+
+        factory :declined_user, :traits => [:declined]
+        factory :extended_declined_user, :traits => [:declined] do
+          status "extended_declined"
+        end
+      end
+    end
+  end
+
+  it "returns the default status" do
+    FactoryGirl.build(:user).status.should == "pending"
+  end
+
+  it "prefers inline trait attributes over default attributes" do
+    FactoryGirl.build(:user, :accepted).status.should == "accepted"
+  end
+
+  it "prefers traits on a factory over default attributes" do
+    FactoryGirl.build(:declined_user).status.should == "declined"
+  end
+
+  it "prefers inline trait attributes over traits on a factory" do
+    FactoryGirl.build(:declined_user, :accepted).status.should == "accepted"
+  end
+
+  it "prefers attributes on factories over attributes from non-inline traits" do
+    FactoryGirl.build(:extended_declined_user).status.should == "extended_declined"
+  end
+
+  it "prefers inline traits over attributes on factories" do
+    FactoryGirl.build(:extended_declined_user, :accepted).status.should == "accepted"
+  end
+
+  it "prefers overridden attributes over attributes from traits, inline traits, or attributes on factories" do
+    FactoryGirl.build(:extended_declined_user, :accepted, :status => "completely overridden").status.should == "completely overridden"
+  end
+end

spec/factory_girl/definition_spec.rb

@@ -64,24 +64,41 @@ describe FactoryGirl::Definition, "#to_create" do
   end
 end
 
-describe FactoryGirl::Definition, "#traits" do
-  let(:female_trait) { stub("female trait", :name => :female) }
-  let(:admin_trait)  { stub("admin trait", :name => :admin) }
+describe FactoryGirl::Definition, "#processing_order" do
+  let(:female_trait) { FactoryGirl::Trait.new(:female) }
+  let(:admin_trait)  { FactoryGirl::Trait.new(:admin) }
 
   before do
     subject.define_trait(female_trait)
     FactoryGirl.stubs(:trait_by_name => admin_trait)
   end
 
-  its(:traits) { should be_empty }
+  context "without base traits" do
+    it "returns the definition without any traits" do
+      subject.processing_order.should == [subject]
+    end
 
-  it "finds the correct traits after inheriting" do
-    subject.inherit_traits([:female])
-    subject.traits.should == [female_trait]
+    it "finds the correct traits after inheriting" do
+      subject.inherit_traits([:female])
+      subject.processing_order.should == [subject, female_trait]
+    end
+
+    it "looks for the trait on FactoryGirl" do
+      subject.inherit_traits([:female, :admin])
+      subject.processing_order.should == [subject, female_trait, admin_trait]
+    end
   end
 
-  it "looks for the trait on FactoryGirl" do
-    subject.inherit_traits([:female, :admin])
-    subject.traits.should == [admin_trait, female_trait]
+  context "with base traits" do
+    subject { FactoryGirl::Definition.new("my definition", [:female]) }
+
+    it "returns the base traits and definition" do
+      subject.processing_order.should == [female_trait, subject]
+    end
+
+    it "finds the correct traits after inheriting" do
+      subject.inherit_traits([:admin])
+      subject.processing_order.should == [female_trait, subject, admin_trait]
+    end
   end
 end

spec/factory_girl/factory_spec.rb

@@ -305,11 +305,11 @@ describe FactoryGirl::Factory, "#with_traits" do
   end
 
   it "returns a factory with the correct traits" do
-    subject.with_traits([:admin, :female]).traits.should =~ [admin_trait, female_trait]
+    subject.with_traits([:admin, :female]).processing_order[1, 2].should == [admin_trait, female_trait]
   end
 
-  it "doesn't modify the original factory's traits" do
+  it "doesn't modify the original factory's processing order" do
     subject.with_traits([:admin, :female])
-    subject.traits.should be_empty
+    subject.processing_order.should == [subject.definition]
   end
 end