giant robots smashing into other giant robots

Written by thoughtbot

dancroak

Short, explicit test setups

You probably know about this Factory Girl definition syntax:

FactoryGirl.define do
  factory :user do
    name 'Connie Customer'
  end
end

But did you know about this Factory Girl invocation syntax?

setup do
  @user = create(:user)
end

Or:

setup do
  @user = build(:user)
end

Or:

setup do
  post :create, user: attributes_for(:user)
end

It’s in there.

Configuration for Test::Unit / Shoulda:

class ActiveSupport::TestCase
  include FactoryGirl::Syntax::Methods
end

Configuration for RSpec:

RSpec.configure do |config|
  config.include FactoryGirl::Syntax::Methods
end

Configuration for Cucumber:

World FactoryGirl::Syntax::Methods

Written by .

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!

lolconomy

This week in open source

paperclip

Much of the work over the past week was done in paperclip, so now you can upload files to your Rails apps with more flare and style!

It now supports an option for keeping old files, so you can pass :keep_old_files to has_attached_file and, when you destroy an attachment (@user.avatar.destroy) it won’t actually delete the underlying files (345ec74). Many people find this useful for S3 storage, which hints that there may be a deeper problem elsewhere. This is thanks to Eike Bernhardt (teefax) but was originally written by Philippe Creux (pcreux).

Christoph Lupprich (kitto) saw a quick way to speed up the #public_url method for Fog storage using AWS as the provider, so he did (989ec0e).

I’ve long wanted a migration helper, and Daniel Schierbeck (dasch) wrote it with some git cleanup from Alexey Mahotkin (693b528, b922111, f82c0d9, f3eacd2, e0a6732, ffbfc24, 500f1bb, b70ffbc, 65a6ae8). It looks like this:

class AddAvatarColumnsToUser < ActiveRecord::Migration
  def self.up
    change_table :users do |t|
      t.has_attached_file :avatar
    end
  end

  def self.down
    drop_attached_file :users, :avatar
  end
end

There were also some important internal changes. For example, Prem Sichanugrist (sikachu) replaced the AWS::S3 gem with AWS::SDK (1df1b03, 81129ad, 88a8af9, 75f413d, 2cf7378). He called out AWS (amazonwebservices) and Trevor Rowe (trevorrowe) for helping, and John Joseph Bachir (jjb) updated the docs appropriately (308f1a0).

Prem also got Paperclip passing on Rubinius (001fd99).

In bug fixes, base URLs with a ? but no = will produce Paperclip URLs using ? (128d664 and bb22be3). Prem thinks that’s the right behavior now, but it’s tricky to nail down.

Nick Padgett (npadgett) found an edgecase where we were calling strip on a non-string object, sometimes, and fixed that (34913f1).

Dimitrij Denissenko (dim) handled the case where using the :id_partition pattern in a URL or path pattern raises a NoMethodError on an unsaved resource (ac82244). He and I are now enemies for life for his use of nil.

Prem fixed another bug: if you have a path or URL pattern with :class in it it will show a warning. He removed this warning (b4ff2c5).

Prem worked with Steve Richert (laserlemon) to show the Gemnasium results in the README (3e20907 and 777ac90), and Prem also updated the README to be more readable (adcd03c).

fake_braintree

The fake credit card processor, fake_braintree, hit 0.0.6 (eed875e and 0934f1e) this week as Ben Orenstein (r00k) added support for discounted subscriptions (757c0aa) and Gabe Berke-Williams (gabebw) exposed the transactions that have run (8dde09c and ab93137).

suspenders

Taking a stance on whether we play along with the little Open Directory Project game, suspenders now defaults to NOODP on every page which, as Matt Jankowski (mjankowski) points out, tells Web crawlers to never bother looking for ODP details (6275d0f).

shoulda-matchers

The sweet shoulda-matchers collection of RSpec matchers now has more accurate error messages for the allow_value matcher (25c2623), thanks to Clemens Helm (clemenshelm). It uses the underlying internationalization information to generate this.

bourbon

Another documentation update on bourbon as Phil LaPier (plapier) explained that multiple background images with shorthand notation are unsupported (798aa1c), after clarifying that multiple background images themselves have fancy comma-separated syntax (aa66831).

jdclayton

Test Rake Tasks Like a BOSS

Testing Rake tasks is one of the most painful things I do as a Ruby developer. Even after extracting all the code out into a separate class (which helps a lot), I still want to make sure I test that the right classes got called correctly with the right arguments.

I wanted the subject to be the task, where I could call invoke, check its prerequisites, etc.

describe "cron:hourly" do
  its(:prerequisites) { should include("reports:users") }
end

describe "reports:users" do
  before { ReportGenerator.stubs(:generate) }

  its(:prerequisites) { should include("environment") }

  it "generates the report" do
    subject.invoke
    ReportGenerator.should have_received(:generate).with()
  end
end

RSpec has shared contexts, so I set off to find an easy, straightforward way to test Rake tasks.

# spec/support/shared_contexts/rake.rb
require "rake"

shared_context "rake" do
  let(:rake)      { Rake::Application.new }
  let(:task_name) { self.class.top_level_description }
  let(:task_path) { "lib/tasks/#{task_name.split(":").first}" }
  subject         { rake[task_name] }

  def loaded_files_excluding_current_rake_file
    $".reject {|file| file == Rails.root.join("#{task_path}.rake").to_s }
  end

  before do
    Rake.application = rake
    Rake.application.rake_require(task_path, [Rails.root.to_s], loaded_files_excluding_current_rake_file)

    Rake::Task.define_task(:environment)
  end
end

This shared context is doing a lot, so I’ll walk through some of the odd areas and explain what’s happening.

The second let (task_name) is grabbing the top level description. That means it’ll use the text we pass to describe to calculate the task we’re going to run.

describe("reports:user") { } # subject is Rake::Task["reports:user"]

task_path is the path to the file itself, relative to Rails.root. We can infer path based off of the description, so for the describe above, it’ll assume the rake task is in lib/tasks/reports.rake.

Thirdly, loaded_files_excluding_current_rake_file - this requires a bit of explanation, even with that really descriptive method name. Rake is kind of a pain in certain cases; The rake_require method takes three arguments: the path to the task, an array of directories to look for that path, and a list of all the files previously loaded. rake_require takes loaded paths into account, so we exclude the path to the task we’re testing so we have the task available. This only matters when you’re running more than one test on a rake task, but there’s no harm in doing this every time we test so that there aren’t odd edge cases out there.

Finally, I define the :environment task (which most tasks defined in a Rails app will have as a prerequisite, since it’ll load the Rails stack for accessing models and code within lib without any additional work.

That’s the shared context in a nutshell; here’s what it allows us to do.

The tasks:

# lib/tasks/reports.rake
namespace :reports do
  desc "Generate users report"
  task :users => :environment do
    data = User.all
    ReportGenerator.generate("users", UsersReport.new(data).to_csv)
  end

  desc "Generate purchases report"
  task :purchases => :environment do
    data = Purchase.valid
    ReportGenerator.generate("purchases", PurchasesReport.new(data).to_csv)
  end

  desc "Generate all reports"
  task :all => [:users, :purchases]
end

And the tests:

# spec/lib/tasks/reports_rake_spec.rb
describe "reports:users" do
  include_context "rake"

  let(:csv)          { stub("csv data") }
  let(:report)       { stub("generated report", :to_csv => csv) }
  let(:user_records) { stub("user records for report") }

  before do
    ReportGenerator.stubs(:generate)
    UsersReport.stubs(:new => report)
    User.stubs(:all => user_records)
  end

  its(:prerequisites) { should include("environment") }

  it "generates a registrations report" do
    subject.invoke
    ReportGenerator.should have_received(:generate).with("users", csv)
  end

  it "creates the users report with the correct data" do
    subject.invoke
    UsersReport.should have_received(:new).with(user_records)
  end
end

describe "reports:purchases" do
  include_context "rake"

  let(:csv)              { stub("csv data") }
  let(:report)           { stub("generated report", :to_csv => csv) }
  let(:purchase_records) { stub("purchase records for report") }

  before do
    ReportGenerator.stubs(:generate)
    PurchasesReport.stubs(:new => report)
    Purchase.stubs(:valid => purchase_records)
  end

  its(:prerequisites) { should include("environment") }

  it "generates an purchases report" do
    subject.invoke
    ReportGenerator.should have_received(:generate).with("purchases", csv)
  end

  it "creates the purchase report with the correct data" do
    subject.invoke
    PurchasesReport.should have_received(:new).with(purchase_records)
  end
end

describe "reports:all" do
  include_context "rake"

  its(:prerequisites) { should include("users") }
  its(:prerequisites) { should include("purchases") }
end

Some people may say, “This is overkill! I tested the classes in other areas!” To me, that’s just like saying, “I’ve written unit and functional tests so I don’t need to write integration tests.” If you have a rake task that needs to be run (cron on Heroku, for example), would you leave that code untested? I wouldn’t.

Have you extracted out a pattern for testing Rake tasks? I’d love to hear about it; maybe a patch to RSpec is in order!

qrush

Testing Cron on Heroku

Deploying cron to Heroku is really…pleasant. Click “Daily” or “Hourly” cron and without any tedious setup of scripts, ensuring output is logged, or referring to CronWTF. Testing it though, is a pain! No longer should this be the case.

Cron is serious business.

Outside of Heroku, splitting up daily and hourly cron tasks is easy: just have a script/cron_hourly and script/cron_daily in your Rails app, and have fun configuring that on your server. On Heroku, it’s handled in one Rake task. Here’s an example from the Dev Center:

desc "This task is called by the Heroku cron add-on"
task :cron => :environment do
  if Time.now.hour % 4 == 0 # run every four hours
    puts "Updating feed..."
    NewsFeed.update
    puts "done."
  end

if Time.now.hour == 0 # run at midnight
    User.send_reminders
  end
end

Two things stand out here. First, checking for hourly/daily tasks is done by looking at Time.now. Second, that’s a lot of logic to put in a Rake task, and not write a test!

We’ve talked about testing Rake integration before, and we’re going to use a similar pattern here: extract the Rake task into a model. For Radish, our Rakefile now has:

desc "Run cron job"
task :cron => :environment do
  Cron.run
end

Our Cron class now has to handle the daily and hourly tasks. For now, this is all done in the run class level method. For Radish, the method has to:

  • Archive data hourly for our new historical graphs
  • Activate accounts daily, which will prevent a reminder email from being sent out.

The test

I ended up just using RSpec to test this out. Timecop helps with freezing time in the right place, and Bourne gives us test spies to make sure the right methods get called. Here’s the test I ended up with:

require 'spec_helper'

describe Cron do
  before do
    Account.stubs(:activate)
    Archive.stubs(:store)
  end

  let!(:project1) { Factory(:project) }
  let!(:project2) { Factory(:project) }

  after do
    Timecop.return
  end

  it "runs nightly" do
    Timecop.freeze(Time.now.midnight)
    Cron.run

    Account.should have_received(:activate)
    Archive.should_not have_received(:store)
  end

  it "runs hourly" do
    now = Time.now.midnight + 1.hour
    Timecop.freeze(now)
    Cron.run

    Account.should_not have_received(:activate)
    Archive.should have_received(:store).with(project1, now, now - 1.hour)
    Archive.should have_received(:store).with(project2, now, now - 1.hour)
  end
end

Let’s start from the top of the test here. The before block uses Bourne to stub out the two class level methods on other models in the application, so we can assert they were called later. We then hook up two let! blocks for two projects, which we will use later. let! as opposed to let in RSpec will force those blocks to be evaluated for each test run instead of being lazy evaluated when they are referenced. Finally, since we’re going to freeze time for each test, we have to return to the system time in the after block.

Our two tests verify our daily and hourly scenarios. The first freezes time at midnight, which may not be exactly when Heroku runs our cron job, but all we care about is that the daily task runs only once. The hourly test freezes time at 1:00AM and checks that only the hourly task gets run, and not the daily.

I could have gone a little more gung-ho on this test, perhaps running through an entire 24 hours and making sure the daily task was only called once, but this was good enough.

The implementation

Here’s what I ended up with in my Cron model:

class Cron
  def self.run
    now = Time.now

    Project.find_each do |project|
      Archive.store(project, now, now - 1.hour)
    end

    if now.hour == 0
      Account.activate
    end
  end
end

The implementation ended up to be pretty simple: grab the time, archive always (since the task is run hourly), and if it’s run in the 12:00AM hour, activate accounts.

Pushing this code down into a model makes more sense now…too much code that deals with models and not test data or factories in a Rake task always smells a bit funky to me. It’s also much easier to refactor the code now that we’re in a real model and we have a testing feedback loop in place. For instance, the Project.find_each loop could easily be extracted into the Project class.

Hacking your time zone

During this process I learned of a UNIX trick that can help with testing this locally: the TZ flag. The appropriately named UNIX Power Tools puts it best:

The TZ environment variable is a little obscure, but it can be very useful. It tells UNIX what time zone you’re in.

Most of the time scripts will get this from your environment, but you can override it. Here’s a simple way to test this:

% TZ=UTC+5 ruby -e "puts Time.now"
2011-07-05 11:30:48 -0500

% TZ=UTC-8 ruby -e "puts Time.now"
2011-07-06 00:30:42 +0800

% TZ=UTC ruby -e "puts Time.now" 
2011-07-05 16:30:53 +0000

So basically, if you want to force it to be midnight when running a test or from a small script, you can use this environment variable to add/subtract time from your current time zone.

Less of a pain

Testing cron is now actually feasible, and now you can be assured your task will work without waiting an entire hour or day to find out. Which of course, means you can ship it faster!