Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rewrite all tests to avoid timers #81

Open
mcollina opened this issue Mar 16, 2020 · 2 comments
Open

rewrite all tests to avoid timers #81

mcollina opened this issue Mar 16, 2020 · 2 comments

Comments

@mcollina
Copy link
Member

Timers are known to cause flaky test. It's important we fix this before it hit us.

@baterson
Copy link
Contributor

baterson commented Mar 25, 2020

Hello @mcollina I would like to take this Issue. You want to rewrite all tests with setTimeout calls but not all of the tests? If so I have one question.
In the case of this test:


test('responseTime', function (t) {
  var dest = split(JSON.parse)
  var logger = pinoHttp(dest)

  function handle (req, res) {
    logger(req, res)
    setTimeout(function () {
      res.end('hello world')
    }, 100)
  }

  expectResponseTime(t, dest, logger, handle)
})

I can rewrite it in this manner:

test('responseTime', function (t) {
  var dest = split(JSON.parse)
  var logger = pinoHttp(dest)

  setup(t, logger, function (err, server) {
    t.error(err)
    doGet(server)
  })

  dest.on('data', function (line) {
    t.ok(line.responseTime, 'responseTime is presented')
    t.end()
  })
})

But what about tests like this one when you need to wait some time to be ensure that nothing was logged

test('no auto logging with autoLogging set to false', function (t) {
  var dest = split(JSON.parse)
  var logger = pinoHttp({ autoLogging: false }, dest)
  var timeout

  setup(t, logger, function (err, server) {
    t.error(err)
    doGet(server)
  })

  dest.on('data', function (line) {
    t.ok(line.responseTime, 'responseTime is presented')
    t.end()
  })


  function handle (req, res) {
    logger(req, res)
    setTimeout(function () {
      res.end('hello world')
    }, 100)
  }

  dest.on('data', function (line) {
    clearTimeout(timeout)
    t.error(line)
    t.end()
  })

  setup(t, logger, function (err, server) {
    t.error(err)
    doGet(server)

    timeout = setTimeout(function () {
      t.end()
    }, 200)
  }, handle)
})

Could you give a tip on how to handle those cases with timeout please?

@mcollina
Copy link
Member Author

You can call t.fail() in case the something is logged: in that case an error will fail the full suite instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants