hoodiehq / hoodie-connection-status

:dog: connection status API for the browser
Apache License 2.0
5 stars 10 forks source link

Start and stop checking #19

Closed jennazee closed 8 years ago

jennazee commented 8 years ago

Addresses https://github.com/hoodiehq/hoodie-client-connection-status/issues/13

gr2m commented 8 years ago

Hey @jennazee, I had a look at your wip PR, and I know why you got stuck with the tests, same happened to me.

lolex and Promises don’t like each other. If there is a promise, it doesn’t get resolve / rejected immediately, so the code execution gets continued, clock.uninstall() gets called although the tested code is still running. I lost hours on this myself, I'm sorry, I should have warn you.

In your case, you can mock the check method (instead of nets & cache.set) by doing this in startChecking.js

var internals = module.exports.internals = {}
internals.check = require('./check')

Then you can do this in the tests

// simulate that `.check()` resolves
simple.mock(startChecking.internals, 'check').callFn(function () {
  return { then: function (success) { success() } }
})
// or simulate that `.check()` rejects e.g. because of server error
simple.mock(startChecking.internals, 'check').callFn(function () {
  return { then: function (success, error) { error() } }
})

You also need to start the setTimeout again once check(state, options) resolve, otherwise the interval would only send one request, but we want it to send requests continuously

So the handleInterval method should look something like this

function handleInterval (state, options) {
  if (options.interval) {
    // we use setTimeout on purpose, we don't want to send requests each
    // x seconds, but rather set a timeout for x seconds after each response
    // but we use `interval` as variable as the effect is the same
    state.interval = setTimeout(function () {
      var checkAgain = handleInterval.bind(null, state, options)
      internals.check(state, options).then(checkAgain, checkAgain)
    }, options.interval, state, options)
    return
  }
}

Then the test could look something like this:

test('startChecking() with successful check', function (t) {
  t.plan(2)

  var clock = lolex.install(0)
  var emitter = {emit: function () {}}

  var state = {
    method: 'HEAD',
    url: 'https://example.com/ping',
    emitter: emitter
  }

  simple.mock(startChecking.internals, 'check').callFn(function () {
    return { then: function (success) { success() } }
  })

  startChecking(state, {
    interval: 1000
  })

  t.ok(state.interval, 'state.interval is set')
  clock.tick(2000)
  t.is(startChecking.internals.check.callCount, 2, '2 requests sent')

  clearTimeout(state.interval)
  simple.restore()
  clock.uninstall()
})

Does that make sense? I’m happy to help you get this finished, but I can understand if you don’t have time for this right now :)

jennazee commented 8 years ago

@gr2m I believe I finished the thing!

janecakemaster commented 8 years ago

:fireworks:

gr2m commented 8 years ago

:+1:

for squashing, let’s do two commits

  1. Commit all test files test: ... prefix
  2. Commit all test files feat: ... prefix
gr2m commented 8 years ago

merged via 33d9e98