Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix issues with inline traits not taking precedence
Closes #242
  • Loading branch information
joshuaclayton committed Jan 18, 2012
1 parent 330f91b commit 065c6c1
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 32 deletions.
31 changes: 20 additions & 11 deletions lib/factory_girl/definition.rb
Expand Up @@ -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
Expand All @@ -20,17 +21,17 @@ def compile
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)
Expand All @@ -51,6 +52,14 @@ def define_trait(trait)

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
Expand Down
10 changes: 2 additions & 8 deletions lib/factory_girl/factory.rb
Expand Up @@ -12,14 +12,12 @@ def initialize(name, options = {}) #:nodoc:
@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."
Expand Down Expand Up @@ -127,10 +125,6 @@ def callbacks

private

def processing_order
[traits.reverse, @definition].flatten
end

def assert_valid_options(options)
options.assert_valid_keys(:class, :parent, :default_strategy, :aliases, :traits)

Expand Down
48 changes: 48 additions & 0 deletions spec/acceptance/traits_spec.rb
Expand Up @@ -312,3 +312,51 @@
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
37 changes: 27 additions & 10 deletions spec/factory_girl/definition_spec.rb
Expand Up @@ -64,24 +64,41 @@
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
6 changes: 3 additions & 3 deletions spec/factory_girl/factory_spec.rb
Expand Up @@ -305,11 +305,11 @@ def self.to_s
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

0 comments on commit 065c6c1

Please sign in to comment.