giant robots smashing into other giant robots

Written by thoughtbot

lolconomy

This week in open source

bourbon

So this bourbon gem…people seem to like it: there was a RailsCast about it and the principle author (Phil LaPier) will be speaking at Frontend United about it.

This week people cleaned up the obsolete CSS attributes: Thibaut (Thibaut) removed -moz-inline-block (d73ea46), Chad Mazzola removed both -ms-border-radius and -o-border-radius (10a5908), and Phil LaPier (plapier) removed -moz-box-orient (6331e26). Gabe Berke-Williams (gabebw) fixed support for Rails 2 (eacd5ee and e9347e7) and, lesson learned, removed Gemfile.lock (671ad33).

factory_girl

Version 2.6.3 of factory_girl is out, all y’all. In this action-packed version we got a cucumber step named e.g. Given the following post exists (note the lack of : at the end), via cj (cj) (d4b8cac and 48afe24). Barun Singh (barunio) fixed a bug where the factory’s traits weren’t compiled in the very first time the factory was used (68ca50f), plus a factory can use all the traits on any ancestor (f14a8cf). Joshua Clayton (joshuaclayton) is now listed as an author (07d2834), so congrats to him and his hard work. As part of that hard work he discovered that the vintage syntax broke in MRI 1.9.2-p318, and then fixed it (a7acc3e).

This just in! Version 3.0.0.rc1 is now public! It breaks everything, again! Please try it and open issues with what breaks:

gem 'factory_girl', '3.0.0.rc1'

bourne

Oh hey version 1.1.1 of bourne is out (650afb0)! Bourne is an extension to mocha that adds test spies so your tests can read like normal tests. Tristan Dunn (tristandunn) added support for mocha 0.10.5 (286d8f9) and fixed a long-standing error message that occurs when you forget to stub a method before spying on it (17dc7d2).

Gabe Berke-Williams (gabebw), who pushed the release, also did some maintainance: adding Travis CI notifications to the README (7d6eb37), removing Gemfile.lock (950a445), and some general code cleanups (4bbe610 and b0f1f99).

shoulda-matchers

The big deal in shoulda-matchers over the past week was when Fujimura Daisuke (fujimura) added the ability to specify the key for the flash message in the set_the_flash matcher (0e0339e, ef866e2, and fd4aa53):

it { should set_the_flash[:alert].to("Password doesn't match") }

Gabe Berke-Williams (gabebw) did his usual maintainance of converting the docs to use Markdown (85c37c4), cleaning up the ModelBuilder that’s used in the tests (31595b0), and showing off how often our build is broken in the README using Travis CI (eebb806), which he also did to shoulda proper (7d805d0).

cocaine

Also victim to the Travis CI treatment was cocaine, via Gabe Berke-Williams (gabebw) again (ac47b6f and 490c406).

paperclip

The paperclip project is gearing up for a groundbreaking (maybe app-breaking?) release, but in the meantime Mike Boone (boone) fixed an infinitely-growing PATH environment variable (06d69af) while Mike Burns (mike-burns)—also known as: me—totally broke backward compatibility by changing the default :path and :url configuration setting (26f4d40). These new settings avoid overwriting files on different models and also scales to more than 1024 instances of the same model.

capybara-webkit

In capybara-webkit news, Joe Ferris (jferris) controversially allowed the user to interact with invisible elements (02f2a8a), and caught the fact that Capybara.timeout is deprecated (4d954b7).

jdclayton

Inject that Rails Configuration Dependency!

Setting up configuration settings in a Rails app has been fairly straightforward for a while now:

# config/environments/development.rb
Doit::Application.configure do
  config.default_creator = "Person 1"
end

# config/environments/test.rb
Doit::Application.configure do
  config.default_creator = "Test Person"
end

# config/environments/production.rb
Doit::Application.configure do
  config.default_creator = "John Doe"
end

To access this setting, call Doit::Application.config.default_creator within your app. Pretty straightforward, right?

class TodoItem < ActiveRecord::Base
  before_create :assign_default_creator, unless: :creator?

  private

  def assign_default_creator
    self.creator = Doit::Application.config.default_creator
  end
end

Let’s imagine you have a todo item and you want to test that the value gets assigned if no creator is provided.

Imagine how you’d test this.

describe TodoItem do
  it "assigns the default creator when no creator is assigned" do
    Doit::Application.config.stub(:default_creator).and_return("default creator from config")
    subject.save
    subject.creator.should == "default creator from config"
  end

  it "does not assign the default creator if it has been set" do
    Doit::Application.config.stub(:default_creator).and_return("default creator from config")
    subject.creator = "Jane Doe"
    subject.save
    subject.creator.should == "Jane Doe"
  end
end

There’s a few things that are gross. First, we’re referencing Doit::Application in the model spec. Second, we’re stubbing in both examples. The latter is a low-hanging fruit so it can be extracted.

describe TodoItem do
  it "assigns the default creator when no creator is assigned" do
    config_default_creator_returns("default creator from config")
    subject.save
    subject.creator.should == "default creator from config"
  end

  it "does not assign the default creator if it has been set" do
    config_default_creator_returns("default creator from config")
    subject.creator = "Jane Doe"
    subject.save
    subject.creator.should == "Jane Doe"
  end

  def config_default_creator_returns(value)
    Doit::Application.config.stub(:default_creator).and_return(value)
  end
end

Now, interacting with Doit::Application is confined to one method. Some people may stub the application in an RSpec before block, but I don’t like doing that because one of the specs cares about the stubbed value. I want that stub right in the example so it’s obvious that the stub and assertion are close (in number of lines).

Even though Doit::Application is confined to one call in the spec, I really don’t like that the spec cares about its config at all. What I’d love to do is assign a custom configuration on my TodoItem in my test intsead of caring about Doit::Application and having to stub on config. I can do this with dependency injection.

Right now, TodoItem has a dependency on Doit::Application.config. Dependency injection would mean TodoItem gets a class_attribute :config that defaults to Doit::Application.config but can be overridden (say, in our tests).

describe TodoItem do
  it "assigns the default creator when no creator is assigned" do
    subject.config = stub("config", default_creator: "default creator from config")
    subject.save
    subject.creator.should == "default creator from config"
  end

  it "does not assign the default creator if it has been set" do
    subject.config = stub("config", default_creator: "default creator from config")
    subject.creator = "Jane Doe"
    subject.save
    subject.creator.should == "Jane Doe"
  end
end

With the class attribute, I’m able to override config on the instance and replace it with a stub that has a default_creator method, which I’ve assigned to the string I expect. I was able to remove my reference of Doit::Application from the spec. Perfect!

Here’s the model code:

class TodoItem < ActiveRecord::Base
  class_attribute :config
  self.config = Doit::Application.config

  before_create :assign_default_creator, unless: :creator?

  private

  def assign_default_creator
    self.creator = config.default_creator
  end
end

The callback assign_default_creator now doesn’t care about Doit::Application.config, only that config has a default_creator method.

This post actually stemmed from my interaction with a developer in Factory Girl GitHub Issues who asked a pretty interesting question about reloading classes in Factory Girl after removing the constant and loading the Ruby file again (it seemed like a code smell and not an issue with Factory Girl).

At the end of the thread, I suggested he attend my Test-Driven Rails workshop next week, January 30th and 31st, because I’ll be talking about RSpec and dependency injection (among other things like Cucumber, how, when, and what to test in a Rails app). It’s perfect for Rails developers who are interested in writing more cleaner, more stable applications.

See you there!

jdclayton

Factory Girl 2.5 Gets Custom Constructors

Today marks a big day in the life of Factory Girl; you can now override the constructor for factories! This is great news for people who’ve been upset that they can’t use Factory Girl with objects who have constructors with required arguments, as Factory Girl would previously just call new without passing any arguments.

Here’s the syntax:

# app/models/report_generator.rb
class ReportGenerator
  def initialize(name, data)
    @name = name
    @data = data
  end

  # ...
end

# spec/factories.rb
FactoryGirl.define do
  factory :report_generator do
    ignore do
      name "Generic Report"
      data { {:foo => "bar", :baz => "buzz"} }
    end

    initialize_with { ReportGenerator.new(name, data) }
  end

  # ...
end

Note that I wrapped the name and data attributes in an ignore block. Factory Girl doesn’t differentiate between attributes passed in the custom constructor and normal attributes to assign, so moving them to the ignore block ensures that I don’t instantiate the report generator and then attempt to assign name and data again.

Grab a copy of 2.5.0 and start using Factory Girl with your other objects today!

jdclayton

Factory Girl 2.4 Goes Meta

Factory Girl has been getting some hardcore internal refactorings over the past few months. Traits are a great example of something that’s started very bare-bones with a few caveats and been transformed to one of my favorite features of the Factory Girl syntax. It required a pretty decent chunk of refactoring and there were bugs for quite a while due to the way attributes in general were handled.

The Old Way

Internally, Factory Girl has four different types of attributes (Dynamic, Static, Association, and Sequence); when compiling attributes, we were sorting attributes by the order attributes were added, moving all static attributes to the beginning of the list (with a priority attribute). This worked for 99% of all cases but did have bugs (the most recent involved traits and dynamic attributes). I wanted to resolve this once and for all.

The New Way

Factory Girl now creates a new class per factory (with some awesome usage of Class.new) and defines methods on that class for each attribute declared in the factory files. This means Factory Girl doesn’t have to worry about sorting attributes anymore; every attribute is effectively lazily-evaluated so it’ll return the correct value every time. Factory Girl also uses inheritance for these anonymous classes, resulting in a pretty significant speedup of factory interaction (namely because we were copying all parent attributes and callbacks to each child to mimic inheritance before).

What Does It Mean?

Factory Girl is now faster!

Here’s some benchmarks from Factory Girl 2.3.0:

                                user     system      total        real
build x10000                4.920000   0.010000   4.930000 (  5.028088)
trait build x10000          6.180000   0.010000   6.190000 (  6.296030)
inline trait build x10000   5.870000   0.010000   5.880000 (  5.989512)
deep build x10000           8.100000   0.020000   8.120000 (  8.285039)
attributes_for x10000       3.200000   0.010000   3.210000 (  3.289043)
create x500                 1.010000   0.320000   1.330000 (  4.186271)

And from Factory Girl 2.4.0:

                                user     system      total        real
build x10000                3.050000   0.000000   3.050000 (  3.121359)
trait build x10000          3.260000   0.010000   3.270000 (  3.353590)
inline trait build x10000   6.700000   0.040000   6.740000 (  7.037460)
deep build x10000           3.400000   0.010000   3.410000 (  3.503984)
attributes_for x10000       0.820000   0.010000   0.830000 (  0.886641)
create x500                 0.710000   0.310000   1.020000 (  3.980102)

Grab a copy of Factory Girl 2.4.0 and speed up your suite!

jdclayton

Design Patterns in the Wild: Null Object

I knocked out a pretty decent refactoring of some of the internals of Factory Girl this past weekend. In one of my commits, I used the Null Object pattern to simplify some conditional logic that was spread across a class.

What’s the Null Object Pattern?

The Null Object pattern describes the use of an object to define the concept of “null” behavior. Typically, a null object will implement a similar interface to a similar object but not actually do anything.

In the instance of Factory Girl, there’s a FactoryGirl::Factory object and a FactoryGirl::NullFactory object; both have a common interface by responding to the instance methods defined_traits, callbacks, attributes, compile, default_strategy, and class_name. These methods are the core of a FactoryGirl::Factory and it’s important that the methods exist on the NullFactory (we’ll be calling these methods in FactoryGirl::Factory).

How to Use the Pattern

In Factory Girl, the Factory object deals with taking a handful of declared attributes and running it, resulting in an instance of a class with values assigned. Factory Girl supports the concept of parent factories; attributes, callbacks, and other features get inherited, but a parent isn’t required. Here’s the code before the change; as you can see, there’s a ton of checking to see if the parent exists.

module FactoryGirl
  class Factory
    def default_strategy
      @default_strategy || (parent && parent.default_strategy) || :create
    end

    def compile
      if parent
        parent.defined_traits.each {|trait| define_trait(trait) }
        parent.compile
      end
      attribute_list.ensure_compiled
    end

    protected

    def class_name
      @class_name || (parent && parent.class_name) || name
    end

    def attributes
      compile
      AttributeList.new(@name).tap do |list|
        traits.each do |trait|
          list.apply_attribute_list(trait.attributes)
        end

        list.apply_attribute_list(attribute_list)
        list.apply_attribute_list(parent.attributes) if parent
      end
    end

    def callbacks
      [traits.map(&:callbacks), @definition.callbacks].tap do |result|
        result.unshift(*parent.callbacks) if parent
      end.flatten
    end

    private

    def parent
      return unless @parent
      FactoryGirl.factory_by_name(@parent)
    end
  end
end

Not only does it add extra lines of code (and every line of code is a liability), but it also forces other developers reading the code to remember if a parent exists. This context-switching across five different methods makes it hard to remember what the actual behavior of each method is doing because certain things may or may not be executed.

To use the pattern, all I did was create a NullFactory object and implement the interface I knew I needed to get rid of all the conditionals. For each method, I returned a “sensible” result; nil for class_name, default_strategy, and compile, and I delegated the remaining few methods (defined_traits, callbacks, and attributes) to definition.

module FactoryGirl
  class NullFactory
    attr_reader :definition

    def initialize
      @definition = Definition.new
    end

    delegate :defined_traits, :callbacks, :attributes, :to => :definition

    def compile; end
    def default_strategy; end
    def class_name; end
  end
end

Testing is pretty straightforward since the behavior is straightforward.

describe FactoryGirl::NullFactory do
  it { should delegate(:defined_traits).to(:definition) }
  it { should delegate(:callbacks).to(:definition) }
  it { should delegate(:attributes).to(:definition) }

  its(:compile)          { should be_nil }
  its(:default_strategy) { should be_nil }
  its(:class_name)       { should be_nil }
end

Now, the private instance method will always return something that behaves like a FactoryGirl::Factory. Perfect.

  def parent
    if @parent # the only conditional to determine if a parent exists
      FactoryGirl.factory_by_name(@parent)
    else
      NullFactory.new
    end
  end

Results

Here are those methods after introducing the Null Object pattern.

module FactoryGirl
  class Factory
    def default_strategy
      @default_strategy || parent.default_strategy || :create
    end

    def compile
      parent.defined_traits.each {|trait| define_trait(trait) }
      parent.compile
      attribute_list.ensure_compiled
    end

    protected

    def class_name
      @class_name || parent.class_name || name
    end

    def attributes
      compile
      AttributeList.new(@name).tap do |list|
        traits.each do |trait|
          list.apply_attribute_list(trait.attributes)
        end

        list.apply_attribute_list(attribute_list)
        list.apply_attribute_list(parent.attributes)
      end
    end

    def callbacks
      [parent.callbacks, traits.map(&:callbacks), @definition.callbacks].flatten
    end

    private

    def parent
      if @parent
        FactoryGirl.factory_by_name(@parent)
      else
        NullFactory.new
      end
    end
  end
end

The commit in Factory Girl can be found here. As you can see, the logic is simplified greatly across all the methods because there’s no more conditional checking. The developer reading this code doesn’t have to care if a parent is assigned or not because he can be sure that the parent, regardless of what it is, will behave in the correct manner when that method is executed.

A couple of months ago, Gabe and I implemented the same pattern in Kumade by introducing a NoopPackager.

Have you used the Null Object pattern recently? If you haven’t, your code is probably ripe for some Null Object pattern disruption!