tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

Async tests leaking into others: mocha#done equivalent? #364

Closed frankandrobot closed 4 years ago

frankandrobot commented 7 years ago

In mocha and jasmine, you can pass a done function. If I'm not mistaken, the way it works is that tests will block until the done function gets fired. The use case is obviously because async tests will otherwise tests "leak" with each other. This is exactly what happens in the code below. The async function fires 3 times but I accidentally set the plan to 1. However, instead of causing the test to fail, it makes the subsequent test fail.

test('kefir#scan fires extra event for initialValue', function(t) {
  // we plan only 1 (whoops)
  t.plan(1)

  let ctr = 0
  const sequence = kefir.sequentially(50, [1, 2])

 //all you need to know is that this is an async function that fires 3 times
 //instead of failing w/ more than the expected calls, this test makes the subsequent test fail!
  sequence.scan((_, i) => i, -1)
    .onValue(x => {
      switch(ctr++) {
        case 0:
          t.equal(x, -1) // initial value
          break
        case 1:
          t.equal(x, 1)
          break
        case 2:
          t.equal(x, 2)
          t.end()
          break
        default:
          t.equal(0, 1) //should never be called
      }
    })
})
ljharb commented 7 years ago

That's because in mocha, tests are synchronous by default - so you have to return a promise or use done to tell it to wait.

In tape, tests are asynchronous by default - you have to explicitly end them, or use .plan to tell tape how many assertions to wait for.

Accidentally setting the plan to 1 would be in the same category as "forgetting to invoke done" - i'm not really sure how tape would be able to handle that.

frankandrobot commented 7 years ago

So you're saying that tape is an asynchronous test runner that only really works with synchronous code? :)

Is there a way to make the tests run synchronously? That would solve this problem, performance be damned.

ljharb commented 7 years ago

No, it works perfectly with asynchronous code. I'm saying that a human programmer error (like putting "1" when you mean "3") can't be fixed by tape, because tape can't read your mind and know when you messed up.

That would not solve this problem if onValue is asynchronous - it would just make your test pass instantly, and fail the other tests, every time.

frankandrobot commented 7 years ago

One could argue that if tests stop being independent because of a simple fat finger, that would make tape a poor choice in a large, real world project.

Having said that though, is this the right pattern when working with async code? (That is, don't use t.plan at all?)

promise.then(t.end).catch(t.fail); // call either t.end or t.fail when the async call ends
ljharb commented 7 years ago

Yes, as long as promise only settles when the test is otherwise finished, that will work fine.