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 <= Time.now
job.created_at.should > Time.now - 10
end
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 = Time.now
job = failed_job.retry
job.run_at.to_i.should == (now + 16).to_i
end
it 'should mark the job as complete when keep is true' do
Navvy::Job.keep = true
jobs = Navvy::Job.next
jobs.first.run
job_count.should == 1
jobs.first.started_at.should be_instance_of Time
jobs.first.completed_at.should be_instance_of Time
end
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 Time.now 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 Time.now. 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))
end
after do
Timecop.return
end
Since Timecop froze Time.now, 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 == Time.now
end
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!