Jeff Kreeftmeijer

Stop hacking, start fixing


While reading through Navvy’s job specs, I found some really ugly Time comparisons:

it 'should set the created_at date' do
  Navvy::Job.enqueue(Cow, :speak, true, false)
  job = first_job
  job.created_at.should be_instance_of Time
  job.created_at.should <=
  job.created_at.should > - 10

it 'should set the run_at date to about 16 seconds from now' do
  failed_job = Navvy::Job.enqueue(Cow, :speak, 'name' => 'Betsy')
  failed_job.stub!(:times_failed).and_return 2
  now =
  job = failed_job.retry
  job.run_at.to_i.should == (now + 16).to_i

it 'should mark the job as complete when keep is true' do
  Navvy::Job.keep = true
  jobs =
  job_count.should == 1
  jobs.first.started_at.should be_instance_of Time
  jobs.first.completed_at.should be_instance_of Time

Are you laughing? That’s fine, it’s funny. But please think about the last time you did something like this while you’re rolling over the floor in laughter. You’re probably just as guilty as I am.

You’re doing it wrong

Let’s go over some problems, starting with the first code snippet.

Navvy::Job.enqueue creates a new job in the database and we want to be sure it sets the created_at field. So we create a job and check it’s created_at value. Great.

We don’t know exactly how long it will take to create the job, so we create it and check it the created_at is an instance of Time, if it’s less than and less than 10 seconds ago.

It never happened to me, but what would happen if — for some strange reason — the job would take more than 10 seconds to be created? In that case something is obviously wrong, but it has nothing to do with the created_at value. That’ll probably be just fine.

The second one is nasty as well. We want to rerun the job after sixteen seconds if it already failed twice, so we create a job and mock out the .times_failed method to return 2. After that, we set the time variable to We call .retry on the job and expect the run_at value to be 16 seconds more than our time variable.

Again: wrong. Theoretically, it could be possible that .retry takes more than a second. In that case, our test will fail.

In the last snippet, I seem to have given up hope. The test just checks if started_at and failed_at are instances of Time. It doesn’t really care what it is, it could be ten years in the past. Useless.

I guess I should have told him to “Freeze”.

No more Timecop quotes, promise.

I did this commit to clean up the mess a bit, using @jtrupiano’s Timecop to “freeze” time:

before do
  Timecop.freeze(Time.local(2010, 1, 1))

after do

Since Timecop froze, it’ll always return January 1st 2010, midnight. This means the first code snippet I showed you can be changed to this:

it 'should set the created_at date' do
  Navvy::Job.enqueue(Cow, :speak, true, false)
  job = first_job
  job.created_at.should ==

I didn’t know Timecop existed when I started building Navvy, but I didn’t search for it either. Instead, I wrote ugly and brittle tests that happened to do the job.

My point is: If you run into a problem like this, find a solution. Somebody out there probably had the same problem before. If not, think of a great solution yourself and release it. Somebody might run into the same problem in the future and thank you for the amazing work you did to make their life easier:

Thanks John, Timecop is awesome!