timecop icon indicating copy to clipboard operation
timecop copied to clipboard

Tests off by one second

Open mockdeep opened this issue 10 years ago • 9 comments

I'm using Timecop to freeze time, but every few test runs the expectations are off by a single second:

it 'updates archived_at with the current time' do
  time = Time.zone.now
  Timecop.freeze(time) do
    campaign.update_attributes(:archived => "true")
    expect(campaign.archived_at.to_i).to eq time.to_i
  end
end

Failure output:

Failure/Error: expect(campaign.archived_at.to_i).to eq time.to_i
    expected: 1395083538
         got: 1395083539

And the code under test:

  def archived=(archived)
    self.archived_at = archived_value(archived)
  end

  def archived_value(archived)
    if archived == 'true' || archived == true
      Time.zone.now
    else
      nil
    end
  end

It happens more commonly on CI where tests are run in parallel. Is there another approach I should be taking?

mockdeep avatar Apr 08 '14 18:04 mockdeep

s/millisecond/second/g

yaauie avatar Apr 08 '14 20:04 yaauie

Ah, yes. Seconds. Updated.

mockdeep avatar Apr 08 '14 20:04 mockdeep

How are you running tests in parallel? Timecop is not threadsafe, so this may be the primary source of your failure.

yaauie avatar Apr 08 '14 20:04 yaauie

Not sure how TDDium handles its parallelization, though I have seen this problem in single threaded runs, too.

mockdeep avatar Apr 08 '14 22:04 mockdeep

Actually, I wonder if this explains it: http://blog.tddium.com/2011/08/07/rails-time-comparisons-devil-details-etc/

Basically, Ruby time is in nanosecond precision. My guess is that while we're truncating by calling to_i it's getting rounded up when inserted into Postgres.

mockdeep avatar Apr 08 '14 22:04 mockdeep

One solution that seems like it will work then is:

it 'updates archived_at with the current time' do
  timestamp = Time.zone.now.to_i
  Timecop.freeze(Time.at(timestamp)) do
    campaign.update_attributes(:archived => "true")
    expect(campaign.archived_at.to_i).to eq time.to_i
  end
end

mockdeep avatar Apr 08 '14 22:04 mockdeep

Or even cleaner:

it 'updates archived_at with the current time' do
  time = Time.zone.now.round
  Timecop.freeze(time) do
    campaign.update_attributes(:archived => "true")
    expect(campaign.archived_at).to eq time
  end
end

Do you think it would be worth adding the .round to parse_time in Timecop?

mockdeep avatar Apr 08 '14 23:04 mockdeep

Calling Time#round in parse_time leads to unexpected results too; consider:

it 'handles the most basic use case' do
  freeze_time = Time.now
  Timecop.freeze(freeze_time) do
    expect(Time.now).to eq freeze_time
  end
end

This would never succeed, since freeze_time is not rounded, but Time.now would be set to a rounded version of freeze_time.

yaauie avatar Apr 08 '14 23:04 yaauie

Ah, yeah. Hmm... Well I was using .to_i to get past any time comparison issues, but maybe I'll just try removing that from my specs and see how it goes. An alternate approach I may try is adding a method like:

def freeze_time(time = Time.zone.now)
  time = time.round
  Timecop.freeze(time) { yield(time) }
end

That way I can just pass the rounded time back through to the block.

mockdeep avatar Apr 08 '14 23:04 mockdeep