From 7c9254a452fb8beb7a197fa52848b49758077c2c Mon Sep 17 00:00:00 2001
From: Joshua Clayton <jclayton@thoughtbot.com>
Date: Sat, 13 Aug 2011 01:03:12 -0400
Subject: [PATCH] Clean up attribute and core factory specs

---
 lib/factory_girl/aliases.rb                   |   4 +-
 lib/factory_girl/attribute.rb                 |  19 +-
 lib/factory_girl/attribute/association.rb     |   3 -
 lib/factory_girl/attribute/callback.rb        |   2 -
 lib/factory_girl/attribute/dynamic.rb         |   2 -
 lib/factory_girl/sequence.rb                  |   4 +-
 spec/factory_girl/aliases_spec.rb             |  40 ++--
 .../attribute/association_spec.rb             |  39 ++--
 spec/factory_girl/attribute/callback_spec.rb  |  29 ++-
 spec/factory_girl/attribute/dynamic_spec.rb   |  82 ++++----
 spec/factory_girl/attribute/implicit_spec.rb  |  46 ++---
 spec/factory_girl/attribute/sequence_spec.rb  |  23 ++-
 spec/factory_girl/attribute/static_spec.rb    |  33 ++--
 spec/factory_girl/attribute_spec.rb           |  44 ++---
 spec/factory_girl/deprecated_spec.rb          |  23 +--
 spec/factory_girl/factory_spec.rb             | 176 ++++++++----------
 spec/factory_girl/find_definitions_spec.rb    |   2 +-
 spec/factory_girl/proxy_spec.rb               |  97 +++++-----
 spec/factory_girl/registry_spec.rb            |  42 ++---
 spec/factory_girl/sequence_spec.rb            |  96 +++-------
 spec/factory_girl_spec.rb                     |  15 +-
 21 files changed, 342 insertions(+), 479 deletions(-)

diff --git a/lib/factory_girl/aliases.rb b/lib/factory_girl/aliases.rb
index 36abfa399..e6055e989 100644
--- a/lib/factory_girl/aliases.rb
+++ b/lib/factory_girl/aliases.rb
@@ -1,8 +1,8 @@
 module FactoryGirl
-
   class << self
     attr_accessor :aliases #:nodoc:
   end
+
   self.aliases = [
     [/(.+)_id/, '\1'],
     [/(.*)/, '\1_id']
@@ -13,8 +13,6 @@ def self.aliases_for(attribute) #:nodoc:
       pattern, replace = *params
       if pattern.match(attribute.to_s)
         attribute.to_s.sub(pattern, replace).to_sym
-      else
-        nil
       end
     end.compact << attribute
   end
diff --git a/lib/factory_girl/attribute.rb b/lib/factory_girl/attribute.rb
index a9df00047..b21748920 100644
--- a/lib/factory_girl/attribute.rb
+++ b/lib/factory_girl/attribute.rb
@@ -14,13 +14,7 @@ class Attribute #:nodoc:
 
     def initialize(name)
       @name = name.to_sym
-
-      if @name.to_s =~ /=$/
-        attribute_name = $`
-        raise AttributeDefinitionError,
-          "factory_girl uses 'f.#{attribute_name} value' syntax " +
-          "rather than 'f.#{attribute_name} = value'"
-      end
+      ensure_non_attribute_writer!
     end
 
     def add_to(proxy)
@@ -43,6 +37,15 @@ def <=>(another)
       self.priority <=> another.priority
     end
 
-  end
+    private
 
+    def ensure_non_attribute_writer!
+      if @name.to_s =~ /=$/
+        attribute_name = $`
+        raise AttributeDefinitionError,
+          "factory_girl uses 'f.#{attribute_name} value' syntax " +
+          "rather than 'f.#{attribute_name} = value'"
+      end
+    end
+  end
 end
diff --git a/lib/factory_girl/attribute/association.rb b/lib/factory_girl/attribute/association.rb
index edc92f71b..de9e909a8 100644
--- a/lib/factory_girl/attribute/association.rb
+++ b/lib/factory_girl/attribute/association.rb
@@ -1,8 +1,6 @@
 module FactoryGirl
   class Attribute #:nodoc:
-
     class Association < Attribute  #:nodoc:
-
       attr_reader :factory
 
       def initialize(name, factory, overrides)
@@ -19,6 +17,5 @@ def association?
         true
       end
     end
-
   end
 end
diff --git a/lib/factory_girl/attribute/callback.rb b/lib/factory_girl/attribute/callback.rb
index 0fc7fe988..9fad69559 100644
--- a/lib/factory_girl/attribute/callback.rb
+++ b/lib/factory_girl/attribute/callback.rb
@@ -1,6 +1,5 @@
 module FactoryGirl
   class Attribute #:nodoc:
-
     class Callback < Attribute  #:nodoc:
       def initialize(name, block)
         @name  = name.to_sym
@@ -11,6 +10,5 @@ def add_to(proxy)
         proxy.add_callback(name, @block)
       end
     end
-
   end
 end
diff --git a/lib/factory_girl/attribute/dynamic.rb b/lib/factory_girl/attribute/dynamic.rb
index 44031b2f3..9dc28ff30 100644
--- a/lib/factory_girl/attribute/dynamic.rb
+++ b/lib/factory_girl/attribute/dynamic.rb
@@ -1,6 +1,5 @@
 module FactoryGirl
   class Attribute #:nodoc:
-
     class Dynamic < Attribute  #:nodoc:
       def initialize(name, block)
         super(name)
@@ -15,6 +14,5 @@ def add_to(proxy)
         proxy.set(name, value)
       end
     end
-
   end
 end
diff --git a/lib/factory_girl/sequence.rb b/lib/factory_girl/sequence.rb
index 703893f20..8e14291f2 100644
--- a/lib/factory_girl/sequence.rb
+++ b/lib/factory_girl/sequence.rb
@@ -9,9 +9,9 @@ class Sequence
     attr_reader :name
 
     def initialize(name, value = 1, &proc) #:nodoc:
-      @name = name
+      @name  = name
       @proc  = proc
-      @value = value || 1
+      @value = value
     end
 
     def next
diff --git a/spec/factory_girl/aliases_spec.rb b/spec/factory_girl/aliases_spec.rb
index 4b4ae1b75..7ba1c275b 100644
--- a/spec/factory_girl/aliases_spec.rb
+++ b/spec/factory_girl/aliases_spec.rb
@@ -1,33 +1,31 @@
 require 'spec_helper'
 
-describe Factory, "aliases" do
-
-  it "should include an attribute as an alias for itself by default" do
-    FactoryGirl.aliases_for(:test).should include(:test)
+describe FactoryGirl, "aliases" do
+  context "aliases for an attribute" do
+    subject { FactoryGirl.aliases_for(:test) }
+    it      { should include(:test) }
+    it      { should include(:test_id) }
   end
 
-  it "should include the root of a foreign key as an alias by default" do
-    FactoryGirl.aliases_for(:test_id).should include(:test)
+  context "aliases for a foreign key" do
+    subject { FactoryGirl.aliases_for(:test_id) }
+    it      { should include(:test) }
+    it      { should include(:test_id) }
   end
 
-  it "should include an attribute's foreign key as an alias by default" do
-    FactoryGirl.aliases_for(:test).should include(:test_id)
+  context "aliases for an attribute starting with an underscore" do
+    subject { FactoryGirl.aliases_for(:_id) }
+    it      { should_not include(:id) }
   end
+end
 
-  it "should NOT include an attribute as an alias when it starts with underscore" do
-    FactoryGirl.aliases_for(:_id).should_not include(:id)
+describe Factory, "after defining an alias" do
+  before do
+    Factory.alias(/(.*)_suffix/, '\1')
   end
 
-  describe "after adding an alias" do
-
-    before do
-      Factory.alias(/(.*)_suffix/, '\1')
-    end
-
-    it "should return the alias in the aliases list" do
-      FactoryGirl.aliases_for(:test_suffix).should include(:test)
-    end
-
-  end
+  subject { FactoryGirl.aliases_for(:test_suffix) }
 
+  it { should include(:test) }
+  it { should include(:test_suffix_id) }
 end
diff --git a/spec/factory_girl/attribute/association_spec.rb b/spec/factory_girl/attribute/association_spec.rb
index f8aba2b4c..648ebfbc1 100644
--- a/spec/factory_girl/attribute/association_spec.rb
+++ b/spec/factory_girl/attribute/association_spec.rb
@@ -1,32 +1,25 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Association do
-  before do
-    @name      = :author
-    @factory   = :user
-    @overrides = { :first_name => 'John' }
-    @attr      = FactoryGirl::Attribute::Association.new(@name, @factory, @overrides)
-  end
+  let(:name)      { :author }
+  let(:factory)   { :user }
+  let(:overrides) { { :first_name => "John" } }
+  let(:proxy)     { stub("proxy") }
 
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  subject       { FactoryGirl::Attribute::Association.new(name, factory, overrides) }
 
-  it "is an association" do
-    @attr.should be_association
-  end
+  it            { should be_association }
+  its(:name)    { should == name }
+  its(:factory) { should == factory }
 
-  it "should have a factory" do
-    @attr.factory.should == @factory
-  end
-
-  it "should tell the proxy to associate when being added to a proxy" do
-    proxy = stub("proxy", :associate => nil)
-    @attr.add_to(proxy)
-    proxy.should have_received(:associate).with(@name, @factory, @overrides)
+  it "tells the proxy to create an association when being added" do
+    proxy.stubs(:associate)
+    subject.add_to(proxy)
+    proxy.should have_received(:associate).with(name, factory, overrides)
   end
+end
 
-  it "should convert names to symbols" do
-    FactoryGirl::Attribute::Association.new('name', :user, {}).name.should == :name
-  end
+describe FactoryGirl::Attribute::Association, "with a string name" do
+  subject    { FactoryGirl::Attribute::Association.new("name", :user, {}) }
+  its(:name) { should == :name }
 end
diff --git a/spec/factory_girl/attribute/callback_spec.rb b/spec/factory_girl/attribute/callback_spec.rb
index 600e8927f..8323c0d7b 100644
--- a/spec/factory_girl/attribute/callback_spec.rb
+++ b/spec/factory_girl/attribute/callback_spec.rb
@@ -1,23 +1,22 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Callback do
-  before do
-    @name  = :after_create
-    @block = proc{ 'block' }
-    @attr  = FactoryGirl::Attribute::Callback.new(@name, @block)
-  end
+  let(:name)  { :after_create }
+  let(:block) { proc { "block" } }
+  let(:proxy) { stub("proxy") }
 
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  subject     { FactoryGirl::Attribute::Callback.new(name, block) }
 
-  it "should set its callback on a proxy" do
-    proxy = stub("proxy", :add_callback => true)
-    @attr.add_to(proxy)
-    proxy.should have_received(:add_callback).with(@name, @block)
-  end
+  its(:name)  { should == name }
 
-  it "should convert names to symbols" do
-    FactoryGirl::Attribute::Callback.new('name', nil).name.should == :name
+  it "set its callback on a proxy" do
+    proxy.stubs(:add_callback)
+    subject.add_to(proxy)
+    proxy.should have_received(:add_callback).with(name, block)
   end
 end
+
+describe FactoryGirl::Attribute::Callback, "with a string name" do
+  subject    { FactoryGirl::Attribute::Callback.new("name", nil) }
+  its(:name) { should == :name }
+end
diff --git a/spec/factory_girl/attribute/dynamic_spec.rb b/spec/factory_girl/attribute/dynamic_spec.rb
index e01f7c044..a2091975f 100644
--- a/spec/factory_girl/attribute/dynamic_spec.rb
+++ b/spec/factory_girl/attribute/dynamic_spec.rb
@@ -1,56 +1,56 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Dynamic do
-  before do
-    @name  = :first_name
-    @block = lambda { 'value' }
-    @attr  = FactoryGirl::Attribute::Dynamic.new(@name, @block)
-  end
+  let(:name)  { :first_name }
+  let(:proxy) { stub("proxy", :set => nil) }
+  let(:block) { lambda { } }
 
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  subject { FactoryGirl::Attribute::Dynamic.new(name, block) }
 
-  it "should call the block to set a value" do
-    @proxy = stub("proxy", :set => nil)
-    @attr.add_to(@proxy)
-    @proxy.should have_received(:set).with(@name, 'value')
-  end
+  its(:name) { should == name }
 
-  it "should yield the proxy to the block when adding its value to a proxy" do
-    @block = lambda {|a| a }
-    @attr  = FactoryGirl::Attribute::Dynamic.new(:user, @block)
-    @proxy = stub("proxy", :set => nil)
-    @attr.add_to(@proxy)
-    @proxy.should have_received(:set).with(:user, @proxy)
-  end
+  context "with a block returning a static value" do
+    let(:block) { lambda { "value" } }
 
-  it "evaluates the block with in the context of the proxy without an argument" do
-    result = 'other attribute value'
-    @block = lambda { other_attribute }
-    @attr  = FactoryGirl::Attribute::Dynamic.new(:user, @block)
-    @proxy = stub("proxy", :set => nil, :other_attribute => result)
-    @attr.add_to(@proxy)
-    @proxy.should have_received(:set).with(:user, result)
+    it "calls the block to set a value" do
+      subject.add_to(proxy)
+      proxy.should have_received(:set).with(name, "value")
+    end
   end
 
-  it "should raise an error when defining an attribute writer" do
-    lambda {
-      FactoryGirl::Attribute::Dynamic.new('test=', nil)
-    }.should raise_error(FactoryGirl::AttributeDefinitionError)
+  context "with a block returning its block-level variable" do
+    let(:block) { lambda {|thing| thing } }
+
+    it "yields the proxy to the block" do
+      subject.add_to(proxy)
+      proxy.should have_received(:set).with(name, proxy)
+    end
   end
 
-  it "should raise an error when returning a sequence" do
-    Factory.stubs(:sequence => FactoryGirl::Sequence.new(:email))
-    block = lambda { Factory.sequence(:email) }
-    attr = FactoryGirl::Attribute::Dynamic.new(:email, block)
-    proxy = stub("proxy")
-    lambda {
-      attr.add_to(proxy)
-    }.should raise_error(FactoryGirl::SequenceAbuseError)
+  context "with a block referencing an attribute on the proxy" do
+    let(:block)  { lambda { attribute_defined_on_proxy } }
+    let(:result) { "other attribute value" }
+
+    before do
+      proxy.stubs(:attribute_defined_on_proxy => result)
+    end
+
+    it "evaluates the attribute from the proxy" do
+      subject.add_to(proxy)
+      proxy.should have_received(:set).with(name, result)
+    end
   end
 
-  it "should convert names to symbols" do
-    FactoryGirl::Attribute::Dynamic.new('name', nil).name.should == :name
+  context "with a block returning a sequence" do
+    let(:block) { lambda { Factory.sequence(:email) } }
+
+    it "raises a sequence abuse error" do
+      expect { subject.add_to(proxy) }.to raise_error(FactoryGirl::SequenceAbuseError)
+    end
   end
 end
+
+describe FactoryGirl::Attribute::Dynamic, "with a string name" do
+  subject    { FactoryGirl::Attribute::Dynamic.new("name", nil) }
+  its(:name) { should == :name }
+end
diff --git a/spec/factory_girl/attribute/implicit_spec.rb b/spec/factory_girl/attribute/implicit_spec.rb
index 33a49b386..80a9b94c5 100644
--- a/spec/factory_girl/attribute/implicit_spec.rb
+++ b/spec/factory_girl/attribute/implicit_spec.rb
@@ -1,50 +1,38 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Implicit do
-  before do
-    @name       = :author
-    @attr       = FactoryGirl::Attribute::Implicit.new(@name)
-  end
+  let(:name)  { :author }
+  let(:proxy) { stub("proxy") }
+  subject     { FactoryGirl::Attribute::Implicit.new(name) }
 
-  it "has a name" do
-    @attr.name.should == @name
-  end
+  its(:name) { should == name }
 
   context "with a known factory" do
     before do
       FactoryGirl.factories.stubs(:registered? => true)
-      # stub(FactoryGirl.factories).registered? { true }
     end
 
-    it "associates the factory" do
-      proxy = stub("proxy", :associate => nil)
-      # stub(proxy).associate
-      @attr.add_to(proxy)
-      proxy.should have_received(:associate).with(@name, @name, {})
-    end
+    it { should be_association }
 
-    it "is an association" do
-      @attr.should be_association
-    end
+    its(:factory) { should == name }
 
-    it "has a factory" do
-      @attr.factory.should == @name
+    it "associates the factory" do
+      proxy.stubs(:associate)
+      subject.add_to(proxy)
+      proxy.should have_received(:associate).with(name, name, {})
     end
   end
 
   context "with a known sequence" do
-    before do
-      FactoryGirl.register_sequence(FactoryGirl::Sequence.new(@name, 1) { "magic" })
-    end
+    let(:sequence) { FactoryGirl::Sequence.new(name, 1) { "magic" } }
+    before         { FactoryGirl.register_sequence(sequence) }
 
-    it "generates the sequence" do
-      proxy = stub("proxy", :set => nil)
-      @attr.add_to(proxy)
-      proxy.should have_received(:set).with(@name, "magic")
-    end
+    it { should_not be_association }
 
-    it "isn't an association" do
-      @attr.should_not be_association
+    it "generates the sequence" do
+      proxy.stubs(:set)
+      subject.add_to(proxy)
+      proxy.should have_received(:set).with(name, "magic")
     end
   end
 end
diff --git a/spec/factory_girl/attribute/sequence_spec.rb b/spec/factory_girl/attribute/sequence_spec.rb
index 15c44aa63..0c1b473c7 100644
--- a/spec/factory_girl/attribute/sequence_spec.rb
+++ b/spec/factory_girl/attribute/sequence_spec.rb
@@ -1,20 +1,19 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Sequence do
-  before do
-    @name     = :first_name
-    @sequence = :name
-    FactoryGirl.register_sequence(FactoryGirl::Sequence.new(@sequence, 5) { |n| "Name #{n}" })
-    @attr  = FactoryGirl::Attribute::Sequence.new(@name, @sequence)
-  end
+  let(:sequence_name) { :name }
+  let(:name)          { :first_name }
+  let(:sequence)      { FactoryGirl::Sequence.new(sequence_name, 5) { |n| "Name #{n}" } }
+  let(:proxy)         { stub("proxy") }
 
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  subject { FactoryGirl::Attribute::Sequence.new(name, sequence_name) }
+  before  { FactoryGirl.register_sequence(sequence) }
+
+  its(:name) { should == name }
 
   it "assigns the next value in the sequence" do
-    proxy = stub("proxy", :set => nil)
-    @attr.add_to(proxy)
-    proxy.should have_received(:set).with(@name, "Name 5")
+    proxy.stubs(:set)
+    subject.add_to(proxy)
+    proxy.should have_received(:set).with(name, "Name 5")
   end
 end
diff --git a/spec/factory_girl/attribute/static_spec.rb b/spec/factory_girl/attribute/static_spec.rb
index 9bf8ad425..ab5300e1a 100644
--- a/spec/factory_girl/attribute/static_spec.rb
+++ b/spec/factory_girl/attribute/static_spec.rb
@@ -1,29 +1,22 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute::Static do
-  before do
-    @name  = :first_name
-    @value = 'John'
-    @attr  = FactoryGirl::Attribute::Static.new(@name, @value)
-  end
+  let(:name)  { :first_name }
+  let(:value) { "John" }
+  let(:proxy) { stub("proxy") }
 
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  subject { FactoryGirl::Attribute::Static.new(name, value) }
 
-  it "should set its static value on a proxy" do
-    @proxy = stub("proxy", :set => nil)
-    @attr.add_to(@proxy)
-    @proxy.should have_received(:set).with(@name, @value)
-  end
+  its(:name) { should == name }
 
-  it "should raise an error when defining an attribute writer" do
-    lambda {
-      FactoryGirl::Attribute::Static.new('test=', nil)
-    }.should raise_error(FactoryGirl::AttributeDefinitionError)
+  it "sets its static value on a proxy" do
+    proxy.stubs(:set)
+    subject.add_to(proxy)
+    proxy.should have_received(:set).with(name, value)
   end
+end
 
-  it "should convert names to symbols" do
-    FactoryGirl::Attribute::Static.new('name', nil).name.should == :name
-  end
+describe FactoryGirl::Attribute::Static, "with a string name" do
+  subject    { FactoryGirl::Attribute::Static.new("name", nil) }
+  its(:name) { should == :name }
 end
diff --git a/spec/factory_girl/attribute_spec.rb b/spec/factory_girl/attribute_spec.rb
index 3b222d501..99716f0fc 100644
--- a/spec/factory_girl/attribute_spec.rb
+++ b/spec/factory_girl/attribute_spec.rb
@@ -1,42 +1,32 @@
 require 'spec_helper'
 
 describe FactoryGirl::Attribute do
-  before do
-    @name  = :user
-    @attr  = FactoryGirl::Attribute.new(@name)
-  end
-
-  it "should have a name" do
-    @attr.name.should == @name
-  end
+  let(:name)  { "user" }
+  let(:proxy) { stub("proxy") }
+  subject     { FactoryGirl::Attribute.new(name) }
 
-  it "isn't an association" do
-    @attr.should_not be_association
-  end
+  its(:name) { should == name.to_sym }
+  it { should_not be_association }
 
-  it "should do nothing when being added to a proxy" do
-    @proxy = stub("proxy", :set => nil)
-    @attr.add_to(@proxy)
-    @proxy.should have_received(:set).never
+  it "doesn't set any attributes on a proxy when added" do
+    proxy.stubs(:set)
+    subject.add_to(proxy)
+    proxy.should have_received(:set).never
   end
 
-  it "should raise an error when defining an attribute writer" do
+  it "raises an error when defining an attribute writer" do
     error_message = %{factory_girl uses 'f.test value' syntax rather than 'f.test = value'}
-    lambda {
+    expect {
       FactoryGirl::Attribute.new('test=')
-    }.should raise_error(FactoryGirl::AttributeDefinitionError, error_message)
-  end
-
-  it "should convert names to symbols" do
-    FactoryGirl::Attribute.new('name').name.should == :name
+    }.to raise_error(FactoryGirl::AttributeDefinitionError, error_message)
   end
 
-  it "should return nil when is compared with a non-attribute object" do
-    (@attr <=> "foo").should == nil
+  it "returns nil when compared to a non-attribute" do
+    (subject <=> "foo").should be_nil
   end
 
-  it "should use priority to perform comparisons" do
-    attr2 = FactoryGirl::Attribute.new('name')
-    (@attr <=> attr2).should == 0
+  it "uses priority to perform comparisons" do
+    second_attribute = FactoryGirl::Attribute.new('name')
+    (subject <=> second_attribute).should be_zero
   end
 end
diff --git a/spec/factory_girl/deprecated_spec.rb b/spec/factory_girl/deprecated_spec.rb
index b12f82997..e595b2618 100644
--- a/spec/factory_girl/deprecated_spec.rb
+++ b/spec/factory_girl/deprecated_spec.rb
@@ -1,27 +1,29 @@
 require 'spec_helper'
 
 describe "accessing an undefined method on Factory that is defined on FactoryGirl" do
-  let(:method_name) { :aliases }
+  let(:method_name)  { :aliases }
   let(:return_value) { 'value' }
-  let(:args) { [1, 2, 3] }
+  let(:args)         { [1, 2, 3] }
 
   before do
-    $stderr.stubs(:puts)
     FactoryGirl.stubs(method_name => return_value)
-
-    @result = Factory.send(method_name, *args)
   end
 
+  subject { Factory.send(method_name, *args) }
+
   it "prints a deprecation warning" do
+    $stderr.stubs(:puts)
+    subject
     $stderr.should have_received(:puts).with(anything)
   end
 
   it "invokes that method on FactoryGirl" do
+    subject
     FactoryGirl.should have_received(method_name).with(*args)
   end
 
   it "returns the value from the method on FactoryGirl" do
-    @result.should == return_value
+    subject.should == return_value
   end
 end
 
@@ -32,13 +34,7 @@
 end
 
 describe "accessing an undefined constant on Factory that is defined on FactoryGirl" do
-  before do
-    @result = Factory::VERSION
-  end
-
-  it "returns that constant on FactoryGirl" do
-    @result.should == FactoryGirl::VERSION
-  end
+  it { Factory::VERSION.should == FactoryGirl::VERSION }
 end
 
 describe "accessing an undefined constant on Factory that is undefined on FactoryGirl" do
@@ -46,4 +42,3 @@
     expect { Factory::BOGUS }.to raise_error(NameError, /Factory::BOGUS/)
   end
 end
-
diff --git a/spec/factory_girl/factory_spec.rb b/spec/factory_girl/factory_spec.rb
index 84aa9802e..f01dcb342 100644
--- a/spec/factory_girl/factory_spec.rb
+++ b/spec/factory_girl/factory_spec.rb
@@ -200,169 +200,147 @@
 end
 
 describe FactoryGirl::Factory, "when defined with a custom class" do
-  before do
-    @class   = Float
-    @factory = FactoryGirl::Factory.new(:author, :class => @class)
-  end
-
-  it "should use the specified class as the build class" do
-    @factory.build_class.should == @class
-  end
+  subject           { FactoryGirl::Factory.new(:author, :class => Float) }
+  its(:build_class) { should == Float }
 end
 
 describe FactoryGirl::Factory, "when defined with a class instead of a name" do
-  before do
-    @class   = ArgumentError
-    @name    = :argument_error
-    @factory = FactoryGirl::Factory.new(@class)
-  end
+  let(:factory_class) { ArgumentError }
+  let(:name)          { :argument_error }
 
-  it "should guess the name from the class" do
-    @factory.name.should == @name
-  end
+  subject { FactoryGirl::Factory.new(factory_class) }
 
-  it "should use the class as the build class" do
-    @factory.build_class.should == @class
-  end
+  its(:name)        { should == name }
+  its(:build_class) { should == factory_class }
 end
 
 describe FactoryGirl::Factory, "when defined with a custom class name" do
-  before do
-    @class   = ArgumentError
-    @factory = FactoryGirl::Factory.new(:author, :class => :argument_error)
-  end
-
-  it "should use the specified class as the build class" do
-    @factory.build_class.should == @class
-  end
+  subject           { FactoryGirl::Factory.new(:author, :class => :argument_error) }
+  its(:build_class) { should == ArgumentError }
 end
 
 describe FactoryGirl::Factory, "with a name ending in s" do
   include DefinesConstants
 
-  before do
-    define_class('Business')
-    @name    = :business
-    @class   = Business
-    @factory = FactoryGirl::Factory.new(@name)
-  end
+  let(:name)           { :business }
+  let(:business_class) { Business }
 
-  it "should have a factory name" do
-    @factory.name.should == @name
-  end
+  before  { define_class('Business') }
+  subject { FactoryGirl::Factory.new(name) }
 
-  it "should have a build class" do
-    @factory.build_class.should == @class
-  end
+  its(:name)        { should == name }
+  its(:build_class) { should == business_class }
 end
 
 describe FactoryGirl::Factory, "with a string for a name" do
-  before do
-    @name    = :string
-    @factory = FactoryGirl::Factory.new(@name.to_s) {}
-  end
-
-  it "should convert the string to a symbol" do
-    @factory.name.should == @name
-  end
+  let(:name) { :string }
+  subject    { FactoryGirl::Factory.new(name.to_s) }
+  its(:name) { should == name }
 end
 
 describe FactoryGirl::Factory, "for namespaced class" do
   include DefinesConstants
 
-  before do
-    define_class('Admin')
-    define_class('Admin::Settings')
+  let(:name)           { :settings }
+  let(:settings_class) { Admin::Settings }
 
-    @name  = :settings
-    @class = Admin::Settings
+  before do
+    define_class("Admin")
+    define_class("Admin::Settings")
   end
 
-  it "should build namespaced class passed by string" do
-    factory = FactoryGirl::Factory.new(@name.to_s, :class => @class.name)
-    factory.build_class.should == @class
+  context "with a namespaced class with Namespace::Class syntax" do
+    subject { FactoryGirl::Factory.new(name, :class => "Admin::Settings") }
+
+    it "sets build_class correctly" do
+      subject.build_class.should == settings_class
+    end
   end
 
-  it "should build Admin::Settings class from Admin::Settings string" do
-    factory = FactoryGirl::Factory.new(@name.to_s, :class => 'admin/settings')
-    factory.build_class.should == @class
+  context "with a namespaced class with namespace/class syntax" do
+    subject { FactoryGirl::Factory.new(name, :class => "admin/settings") }
+
+    it "sets build_class correctly" do
+      subject.build_class.should == settings_class
+    end
   end
 end
 
 describe FactoryGirl::Factory do
   include DefinesConstants
 
+  let(:factory_with_non_existant_strategy) do
+    FactoryGirl::Factory.new(:object, :default_strategy => :nonexistent) { }
+  end
+
+  let(:factory_with_stub_strategy) do
+    FactoryGirl::Factory.new(:object, :default_strategy => :stub)
+  end
+
+  let(:factory_without_strategy) do
+    FactoryGirl::Factory.new(:other_object)
+  end
+
+  let(:factory_with_build_strategy) do
+    FactoryGirl::Factory.new(:other_object, :default_strategy => :build)
+  end
+
   before do
-    define_class('User')
-    define_class('Admin', User)
+    define_class("User")
+    define_class("Admin", User)
   end
 
-  it "should raise an ArgumentError when trying to use a non-existent strategy" do
-    lambda {
-      FactoryGirl::Factory.new(:object, :default_strategy => :nonexistent) {}
-    }.should raise_error(ArgumentError)
+  it "raises an ArgumentError when trying to use a non-existent strategy" do
+    expect { factory_with_non_existant_strategy }.to raise_error(ArgumentError)
   end
 
-  it "should create a new factory with a specified default strategy" do
-    factory = FactoryGirl::Factory.new(:object, :default_strategy => :stub)
-    factory.default_strategy.should == :stub
+  it "creates a new factory with a specified default strategy" do
+    factory_with_stub_strategy.default_strategy.should == :stub
   end
 
-  describe 'defining a child factory without setting default strategy' do
+  describe "defining a child factory without setting default strategy" do
+    let(:parent) { factory_with_stub_strategy }
+    subject      { factory_without_strategy }
+
     before do
-      @parent = FactoryGirl::Factory.new(:object, :default_strategy => :stub)
-      @child = FactoryGirl::Factory.new(:child_object)
-      @child.inherit_from(@parent)
+      subject.inherit_from(parent)
     end
 
-    it "should inherit default strategy from its parent" do
-      @child.default_strategy.should == :stub
+    it "inherits default strategy from its parent" do
+      subject.default_strategy.should == :stub
     end
   end
 
-  describe 'defining a child factory with a default strategy' do
+  describe "defining a child factory with a default strategy" do
+    let(:parent) { factory_with_stub_strategy }
+    subject      { factory_with_build_strategy }
+
     before do
-      @parent = FactoryGirl::Factory.new(:object, :default_strategy => :stub)
-      @child = FactoryGirl::Factory.new(:child_object2, :default_strategy => :build)
-      @child.inherit_from(@parent)
+      subject.inherit_from(parent)
     end
 
-    it "should override the default strategy from parent" do
-      @child.default_strategy.should == :build
+    it "overrides the default strategy from parent" do
+      subject.default_strategy.should == :build
     end
   end
-
 end
 
 describe FactoryGirl::Factory, "human names" do
   context "factory name without underscores" do
-    subject           { FactoryGirl::Factory.new("user") }
+    subject           { FactoryGirl::Factory.new(:user) }
+    its(:names)       { should == [:user] }
     its(:human_names) { should == ["user"] }
   end
 
   context "factory name with underscores" do
-    subject           { FactoryGirl::Factory.new("happy_user") }
+    subject           { FactoryGirl::Factory.new(:happy_user) }
+    its(:names)       { should == [:happy_user] }
     its(:human_names) { should == ["happy user"] }
   end
 
   context "factory name with aliases" do
-    subject           { FactoryGirl::Factory.new("happy_user", :aliases => ["gleeful_user", "person"]) }
+    subject           { FactoryGirl::Factory.new(:happy_user, :aliases => [:gleeful_user, :person]) }
+    its(:names)       { should == [:happy_user, :gleeful_user, :person] }
     its(:human_names) { should == ["happy user", "gleeful user", "person"] }
   end
 end
-
-describe FactoryGirl::Factory, "with aliases" do
-  it "registers the aliases" do
-    name = :user
-    aliased_name = :guest
-    factory = FactoryGirl::Factory.new(:user, :aliases => [aliased_name])
-    factory.names.should =~ [name, aliased_name]
-  end
-
-  it "has human names" do
-    name = :user
-    aliased_name = :guest
-    factory = FactoryGirl::Factory.new(:user, :aliases => [aliased_name])
-    factory.human_names.should =~ [name.to_s, aliased_name.to_s]
-  end
-end
diff --git a/spec/factory_girl/find_definitions_spec.rb b/spec/factory_girl/find_definitions_spec.rb
index 7470bcf92..ec2756941 100644
--- a/spec/factory_girl/find_definitions_spec.rb
+++ b/spec/factory_girl/find_definitions_spec.rb
@@ -5,6 +5,7 @@
     FactoryGirl.stubs(:load)
     FactoryGirl.find_definitions
   end
+
   subject { FactoryGirl }
 end
 
@@ -23,7 +24,6 @@
   end
 end
 
-
 describe "definition loading" do
   def self.in_directory_with_files(*files)
     before do
diff --git a/spec/factory_girl/proxy_spec.rb b/spec/factory_girl/proxy_spec.rb
index f0bee091d..b78e80178 100644
--- a/spec/factory_girl/proxy_spec.rb
+++ b/spec/factory_girl/proxy_spec.rb
@@ -1,82 +1,77 @@
 require 'spec_helper'
 
 describe FactoryGirl::Proxy do
-  before do
-    @proxy = FactoryGirl::Proxy.new(Class.new)
-  end
+  subject { FactoryGirl::Proxy.new(Class.new) }
 
-  it "should do nothing when asked to set an attribute to a value" do
-    lambda { @proxy.set(:name, 'a name') }.should_not raise_error
+  it "doesn't raise when assigning a value to an attribute" do
+    expect { subject.set(:name, "a name") }.to_not raise_error
   end
 
-  it "should return nil when asked for an attribute" do
-    @proxy.get(:name).should be_nil
+  it "returns nil for an attribute without a value" do
+    subject.get(:name).should be_nil
   end
 
-  it "should call get for a missing method" do
-    @proxy.stubs(:get).with(:name).returns("it's a name")
-    @proxy.name.should == "it's a name"
+  it "calls get for a missing method" do
+    subject.stubs(:get).with(:name).returns("it's a name")
+    subject.name.should == "it's a name"
   end
 
-  it "should do nothing when asked to associate with another factory" do
-    lambda { @proxy.associate(:owner, :user, {}) }.should_not raise_error
+  it "doesn't raise when asked to associate with another factory" do
+    expect { subject.associate(:owner, :user, {}) }.to_not raise_error
   end
 
-  it "should raise an error when asked for the result" do
-    lambda { @proxy.result(nil) }.should raise_error(NotImplementedError)
+  it "raises an error when asking for the result" do
+    expect { subject.result(nil) }.to raise_error(NotImplementedError)
   end
 
   describe "when adding callbacks" do
-    before do
-      @first_block  = proc{ 'block 1' }
-      @second_block = proc{ 'block 2' }
-    end
-    it "should add a callback" do
-      @proxy.add_callback(:after_create, @first_block)
-      @proxy.callbacks[:after_create].should be_eql([@first_block])
+    let(:block_1) { proc { "block 1" } }
+    let(:block_2) { proc { "block 2" } }
+
+    it "adds a callback" do
+      subject.add_callback(:after_create, block_1)
+      subject.callbacks[:after_create].should be_eql([block_1])
     end
 
-    it "should add multiple callbacks of the same name" do
-      @proxy.add_callback(:after_create, @first_block)
-      @proxy.add_callback(:after_create, @second_block)
-      @proxy.callbacks[:after_create].should be_eql([@first_block, @second_block])
+    it "adds multiple callbacks of the same name" do
+      subject.add_callback(:after_create, block_1)
+      subject.add_callback(:after_create, block_2)
+      subject.callbacks[:after_create].should be_eql([block_1, block_2])
     end
 
-    it "should add multiple callbacks of different names" do
-      @proxy.add_callback(:after_create, @first_block)
-      @proxy.add_callback(:after_build,  @second_block)
-      @proxy.callbacks[:after_create].should be_eql([@first_block])
-      @proxy.callbacks[:after_build].should be_eql([@second_block])
+    it "adds multiple callbacks with different names" do
+      subject.add_callback(:after_create, block_1)
+      subject.add_callback(:after_build,  block_2)
+      subject.callbacks[:after_create].should be_eql([block_1])
+      subject.callbacks[:after_build].should be_eql([block_2])
     end
   end
 
   describe "when running callbacks" do
-    before do
-      @first_spy  = stub("call_in_create", :foo => true)
-      @second_spy = stub("call_in_create", :foo => true)
-    end
+    let(:object_1_within_callback) { stub("call_in_create", :foo => true) }
+    let(:object_2_within_callback) { stub("call_in_create", :foo => true) }
 
-    it "should run all callbacks with a given name" do
-      @proxy.add_callback(:after_create, proc{ @first_spy.foo })
-      @proxy.add_callback(:after_create, proc{ @second_spy.foo })
-      @proxy.run_callbacks(:after_create)
-      @first_spy.should have_received(:foo).once
-      @second_spy.should have_received(:foo).once
+    it "runs all callbacks with a given name" do
+      subject.add_callback(:after_create, proc { object_1_within_callback.foo })
+      subject.add_callback(:after_create, proc { object_2_within_callback.foo })
+      subject.run_callbacks(:after_create)
+      object_1_within_callback.should have_received(:foo).once
+      object_2_within_callback.should have_received(:foo).once
     end
 
-    it "should only run callbacks with a given name" do
-      @proxy.add_callback(:after_create, proc{ @first_spy.foo })
-      @proxy.add_callback(:after_build,  proc{ @second_spy.foo })
-      @proxy.run_callbacks(:after_create)
-      @first_spy.should have_received(:foo).once
-      @second_spy.should have_received(:foo).never
+    it "only runs callbacks with a given name" do
+      subject.add_callback(:after_create, proc { object_1_within_callback.foo })
+      subject.add_callback(:after_build,  proc { object_2_within_callback.foo })
+      subject.run_callbacks(:after_create)
+      object_1_within_callback.should have_received(:foo).once
+      object_2_within_callback.should have_received(:foo).never
     end
 
-    it "should pass in the instance if the block takes an argument" do
-      @proxy.instance_variable_set("@instance", @first_spy)
-      @proxy.add_callback(:after_create, proc{|spy| spy.foo })
-      @proxy.run_callbacks(:after_create)
-      @first_spy.should have_received(:foo).once
+    it "passes in the instance if the block takes an argument" do
+      subject.instance_variable_set("@instance", object_1_within_callback)
+      subject.add_callback(:after_create, proc {|spy| spy.foo })
+      subject.run_callbacks(:after_create)
+      object_1_within_callback.should have_received(:foo).once
     end
   end
 end
diff --git a/spec/factory_girl/registry_spec.rb b/spec/factory_girl/registry_spec.rb
index 771f816c6..83fbbcdf3 100644
--- a/spec/factory_girl/registry_spec.rb
+++ b/spec/factory_girl/registry_spec.rb
@@ -1,10 +1,16 @@
 require 'spec_helper'
 
 describe FactoryGirl::Registry do
-  let(:factory) { FactoryGirl::Factory.new(:object) }
+  let(:aliases)              { [:thing, :widget] }
+  let(:sequence)             { FactoryGirl::Sequence.new(:email) { |n| "somebody#{n}@example.com" } }
+  let(:factory)              { FactoryGirl::Factory.new(:object) }
+  let(:other_factory)        { FactoryGirl::Factory.new(:string) }
+  let(:factory_with_aliases) { FactoryGirl::Factory.new(:string, :aliases => aliases) }
 
   subject { FactoryGirl::Registry.new }
 
+  it { should be_kind_of(Enumerable) }
+
   it "finds a registered a factory" do
     subject.add(factory)
     subject.find(factory.name).should == factory
@@ -38,33 +44,18 @@
   end
 
   it "iterates registered factories" do
-    other_factory = FactoryGirl::Factory.new(:string)
     subject.add(factory)
     subject.add(other_factory)
-    result = []
-
-    subject.each do |value|
-      result << value
-    end
-
-    result.should =~ [factory, other_factory]
+    subject.to_a.should =~ [factory, other_factory]
   end
 
   it "iterates registered factories uniquely with aliases" do
-    other_factory = FactoryGirl::Factory.new(:string, :aliases => [:awesome])
     subject.add(factory)
-    subject.add(other_factory)
-    result = []
-
-    subject.each do |value|
-      result << value
-    end
-
-    result.should =~ [factory, other_factory]
+    subject.add(factory_with_aliases)
+    subject.to_a.should =~ [factory, factory_with_aliases]
   end
 
   it "registers an sequence" do
-    sequence = FactoryGirl::Sequence.new(:email) { |n| "somebody#{n}@example.com" }
     subject.add(sequence)
     subject.find(:email).should == sequence
   end
@@ -75,22 +66,15 @@
   end
 
   it "registers aliases" do
-    aliases = [:thing, :widget]
-    factory = FactoryGirl::Factory.new(:object, :aliases => aliases)
-    subject.add(factory)
+    subject.add(factory_with_aliases)
     aliases.each do |name|
-      subject.find(name).should == factory
+      subject.find(name).should == factory_with_aliases
     end
   end
 
-  it "is enumerable" do
-    should be_kind_of(Enumerable)
-  end
-
   it "clears registered factories" do
     subject.add(factory)
     subject.clear
-    subject.count.should == 0
+    subject.count.should be_zero
   end
 end
-
diff --git a/spec/factory_girl/sequence_spec.rb b/spec/factory_girl/sequence_spec.rb
index b6f174403..ebb5856d2 100644
--- a/spec/factory_girl/sequence_spec.rb
+++ b/spec/factory_girl/sequence_spec.rb
@@ -2,95 +2,47 @@
 
 describe FactoryGirl::Sequence do
   describe "a basic sequence" do
-    before do
-      @name = :test
-      @sequence = FactoryGirl::Sequence.new(@name) {|n| "=#{n}" }
-    end
-
-    it "has a name" do
-      @sequence.name.should == @name
-    end
+    let(:name) { :test }
+    subject    { FactoryGirl::Sequence.new(name) {|n| "=#{n}" } }
 
-    it "has names" do
-      @sequence.names.should == [@name]
-    end
-
-    it "should start with a value of 1" do
-      @sequence.next.should == "=1"
-    end
+    its(:name)             { should == name }
+    its(:names)            { should == [name] }
+    its(:next)             { should == "=1" }
+    its(:default_strategy) { should == :create }
 
-    it "responds to default_strategy" do
-      @sequence.default_strategy.should == :create
-    end
-
-    describe "after being called" do
-      before do
-        @sequence.next
-      end
-
-      it "should use the next value" do
-        @sequence.next.should == "=2"
-      end
+    describe "when incrementing" do
+      before     { subject.next }
+      its(:next) { should == "=2" }
     end
   end
 
   describe "a custom sequence" do
-    before do
-      @sequence = FactoryGirl::Sequence.new(:name, "A") {|n| "=#{n}" }
-    end
+    subject    { FactoryGirl::Sequence.new(:name, "A") {|n| "=#{n}" } }
+    its(:next) { should == "=A" }
 
-    it "should start with a value of A" do
-      @sequence.next.should == "=A"
-    end
-
-    describe "after being called" do
-      before do
-        @sequence.next
-      end
-
-      it "should use the next value" do
-        @sequence.next.should == "=B"
-      end
+    describe "when incrementing" do
+      before     { subject.next }
+      its(:next) { should == "=B" }
     end
   end
 
   describe "a basic sequence without a block" do
-    before do
-      @sequence = FactoryGirl::Sequence.new(:name)
-    end
-
-    it "should start with a value of 1" do
-      @sequence.next.should == 1
-    end
+    subject    { FactoryGirl::Sequence.new(:name) }
+    its(:next) { should == 1 }
 
-    describe "after being called" do
-      before do
-        @sequence.next
-      end
-
-      it "should use the next value" do
-        @sequence.next.should == 2
-      end
+    describe "when incrementing" do
+      before     { subject.next }
+      its(:next) { should == 2 }
     end
   end
 
   describe "a custom sequence without a block" do
-    before do
-      @sequence = FactoryGirl::Sequence.new(:name, "A")
-    end
-
-    it "should start with a value of A" do
-      @sequence.next.should == "A"
-    end
-
-    describe "after being called" do
-      before do
-        @sequence.next
-      end
+    subject    { FactoryGirl::Sequence.new(:name, "A") }
+    its(:next) { should == "A" }
 
-      it "should use the next value" do
-        @sequence.next.should == "B"
-      end
+    describe "when incrementing" do
+      before     { subject.next }
+      its(:next) { should == "B" }
     end
   end
 end
diff --git a/spec/factory_girl_spec.rb b/spec/factory_girl_spec.rb
index a7c463eee..5dd5177d5 100644
--- a/spec/factory_girl_spec.rb
+++ b/spec/factory_girl_spec.rb
@@ -1,17 +1,22 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe FactoryGirl do
-  let(:factory) { FactoryGirl::Factory.new(:object) }
+  let(:factory)  { FactoryGirl::Factory.new(:object) }
   let(:sequence) { FactoryGirl::Sequence.new(:email) }
+  let(:trait)    { FactoryGirl::Trait.new(:admin) }
 
-  it "finds a registered a factory" do
+  it "finds a registered factory" do
     FactoryGirl.register_factory(factory)
     FactoryGirl.factory_by_name(factory.name).should == factory
   end
 
-  it "finds a registered a sequence" do
+  it "finds a registered sequence" do
     FactoryGirl.register_sequence(sequence)
     FactoryGirl.sequence_by_name(sequence.name).should == sequence
   end
-end
 
+  it "finds a registered trait" do
+    FactoryGirl.register_trait(trait)
+    FactoryGirl.trait_by_name(trait.name).should == trait
+  end
+end