Removal of timecop ⏲ 👮🏼

Dev
1

Yesterday I committed a change that removed the timecop gem and replaced it with these lines of code:

  class TrackTimeStub
    def self.stubbed
      false
    end
  end

  def freeze_time(now=Time.now)
    datetime = DateTime.parse(now.to_s)
    time = Time.parse(now.to_s)

    if block_given?
      raise "nested freeze time not supported" if TrackTimeStub.stubbed
    end

    DateTime.stubs(:now).returns(datetime)
    Time.stubs(:now).returns(time)
    Date.stubs(:today).returns(datetime.to_date)
    TrackTimeStub.stubs(:stubbed).returns(true)

    if block_given?
      begin
        yield
      ensure
        unfreeze_time
      end
    end
  end

  def unfreeze_time
    DateTime.unstub(:now)
    Time.unstub(:now)
    Date.unstub(:today)
    TrackTimeStub.unstub(:stubbed)
  end

Migrating from timecop to our freeze_time helper should be a breeze as it already supports the block form. So most changes are as simple as:

Timecop.freeze(1.day.from_now) do
    no_longer_living_on_borrowed_time!
end

# to

freeze_time(1.day.from_now) do
    no_longer_living_on_borrowed_time!
end

I manually changed every usage of Timecop in both core and core plugins to use the new pattern. It took me less than an hour.

Why was this done?

I guess the first thing people may ask when confronted with this "Sam!? Wheels why reinvent them? Cheese, why move it?

The catalyst for the change was 3 days of an unstable test suite, caused by misuse of the Timecop library. But that was not the only issue I had.

Timecop API is too wide

The timecop gem offered too many options and too much flexibility, for example, want to do:

Timecop.travel 1.day.ago do
    Timecop.travel 1.day.ago do
       Timecop.travel 1.day.ago do
       end
    end
end

Nesting can very quickly lead to incredibly hard to understand specs. Additionally, it had a bunch of fanciness we never used including date rounding, thread safe operation and time stack walking.

Timecop API required consumers to remember to “unfreeze” time, which is called return

before do
   Timecop.freeze(1.day.ago)
end

after do
   # forget this and you leak state into your test suite and break it 
   # cause time is frozen after your test
   Timecop.return
end

Our API uses mocha which always cleans up, no need to remember anything.

Less dependencies make me happy

I don’t see why carry a whole gem dependency for 20 or so lines of code we control and can trivially debug. We already have a mocking library I don’t see why we need to introduce a second one.

There is no reason to have 2 time travel APIs

It is confusing to have 2 ways of mocking time. Developers should not be force to choose between 2 APIs that achieve essentially the same goal.

Additionally, Discourse uses mocha for all object mocking. You end up having 2 mocking libraries and if you mock Time with with both you can very quickly get very unexpected results.

15 Likes