jcoglan / rspec-eventmachine

RSpec extensions for testing EventMachine code
26 stars 8 forks source link

EM::FakeClock ignores previously set timers #5

Open Intrepidd opened 10 years ago

Intrepidd commented 10 years ago

Consider the following code :

require 'faye'
require 'rspec/em'

Steps = RSpec::EM.async_steps do

  def clock_stub(&callback)
    clock.stub
    callback.call
  end

  def timer(time, proc, &callback)
    EM.add_timer(time) do
      proc.call
    end
    callback.call
  end

  def clock_tick(time, &callback)
    clock.tick(time)
    callback.call
  end

end

describe 'foo' do

  include RSpec::EM::FakeClock
  include Steps

  it 'fails' do
    EM.add_timer(2) do
      puts "not called"
    end
    clock_stub
    timer(1, proc { puts "hello world"})
    timer(1, proc { puts "hello world 2 "})
    clock_tick 5
  end

end

The output will be :

hello world
hello world 2

Indeed, when stubbing the timers, there is no check about current timers and converting them.

Is this something you would be interested in ?

Here is my use case :

I'm trying to write specs about a presence plugin for faye, that will send disconnection events xxx seconds after the server registers a disconnect if the user has not connected back.

I don't want my specs to last long and really wait 15 secs for example.

I tried using EM::FakeClock but even if I tick 300 seconds, the faye server (launched in with the specs as a Thin Server won't register the disconnection event.

I think some faye servers timers that were present before the stub are not called yet and the specs end early.

I'll try with stubbing the clock before starting the server and then ticking when needed but it would be really easier if the already present timers where converted.

What do you think ?

jcoglan commented 10 years ago

I'm not convinced by this. Conceptually, the fact that clock.stub is an explicit call suggests that the user can draw a line between timers that are captured and those that are not, whereas making it convert all pre-existing timers would remove that boundary. It would make things that currently possible -- running code that uses real timers, before stubbing the clock out and switching to a fake scheduler -- impossible.

But there's also the question of implementation. EM does not, as far as I know, expose an API for querying existing timers (and I don't think it should since that would leak state). So, in order to be aware of timers that were registered before clock.stub, we would have to monkey-patch EM.{add,cancel}_timer as soon as rspec/eventmachine was loaded. That simply pushes the capturing question off of clock.stub and onto require 'rspec/eventmachine', and there is still time during your program when timers will be missed.

But even if those timers were intercepted, how would you capture them into the fake scheduler? Calls to add_timer prior to clock.stub would have to be allowed to fall through to the real implementation, at which point they become scheduled inside EM. If we logged those timers and added them into the fake scheduler on clock.stub, they would still exist inside EM. They cannot be captured in such a way as to prevent EM from eventually executing them.

So for both conceptual and implementation reasons I think this is fraught with problems, but I'm open to being convinced otherwise if you can see a clean way to do this.