tristandunn / pusher-fake

A fake Pusher server for development and testing.
https://rubygems.org/gems/pusher-fake
MIT License
173 stars 32 forks source link

Requiring to add sleep 1 before triggering Pusher event #57

Closed fikrikarim closed 5 years ago

fikrikarim commented 5 years ago

Hi @tristandunn nice to see you again.

I love this gem, but I see you don't have any enough opened issue. So I'll try to create one 😂

So now, our spec that use a Pusher.trigger, will have a chance to fail. We remedy this by using Rspec retry. But for some branch it fails at a higher chance than usual, and making our CI fails.

Weird thing is, when we tried to add:

sleep 1

before Pusher.trigger, all of the spec will pass.

We tried to use the one on the example:

Timeout.timeout(5) do
  state = nil

  state = page.evaluate_script('Pusher.instances[0].connection.state') until state == 'connected'
end

but it still fails randomly. To be honest I'm still investigating the issue right now and still not sure whether the issue come from our codebase or from this gem.

I'm not sure what cause the issue or anything. Thanks anyway @tristandunn for the gem 👍

tristandunn commented 5 years ago

Could you expand on what you're expecting after you call the trigger? You'd generally what to use Capybara finders to check for an updated document instead of monitoring the JS state, since the finders will automatically wait.

The timeout is admittedly kind of a hack to simplify the example. Depending on how you're running the tests, it's also possible the local ports aren't matching up and your trigger is never reaching the browser. You may need to define fixed ports in the test environment.

if defined?(PusherFake)
  PusherFake.configure do |configuration|
    configuration.web_options[:port]    = ENV.fetch("PUSHER_FAKE_WEB_PORT")
    configuration.socket_options[:port] = ENV.fetch("PUSHER_FAKE_SOCKET_PORT")
  end
end
fikrikarim commented 5 years ago

Thanks for the response. Here's what our typical spec looks like:

sign_in user
visit trade_path
sleep 1
Pusher.trigger("private-#{user.sn}", :trade, type: :update_detail)

expect(page).to have_content I18n.t('otc.trade.detail')

The problem is if we don't include the sleep 1, then sometimes the spec will fail. We're not sure why it fails yet.

If we remove or change the sleep 1 to a method like:

def wait_for_pusher
  Timeout.timeout(5) do
    state = nil

    state = page.evaluate_script('Pusher.instances[0].connection.state') until state == 'connected' 
  end
end

It fails randomly. I think the problem might not be in this gem. I'll close this issue soon, if you're not sure what's happening here too. Thanks 😄

tristandunn commented 5 years ago

Asynchronous actions can be difficult to test within Capybara. Some of the methods automatically wait, up to 2 seconds I think, before failing. They removed the method they had for waiting with the recommendation of Timeout which I what I used to be safe.

Closing this since it hasn't been updated in a while. Feel free to reopen if you have new details.