ben-page / node-test

A simple, asynchronous test runner for Node.js.
https://www.npmjs.com/package/node-test
MIT License
4 stars 1 forks source link

Problems handling errors in async tests? #16

Closed ghost closed 8 years ago

ghost commented 8 years ago

Seems like I got issues with handling errors in async tests. It's probably just me doing something wrong! Here's an example:

 test('example', function (t, state, done) => {
    curl('https://google.com', function (er, data) {
      t.equal(data, 'fooBar')
      done(er)
    })
  });

If the above callback is passed a truthy er, then done(er) will fail and print the error.

But… it seems that done will never be called and the er never printed 99% of the time, because the t.equal call will raise first, probably with an unhelpful undefined !== 'fooBar' failure that hides the real cause.

So what if node-test's done callback API checked for errors first, then proxied the callback function with the assertions. That way any error could fail fast when er is set.

    curl('https://google.com', done.when(function (er, data) {
       t.equal(data, 'fooBar')
    })

And in cases where a truthy er is expected, triggering failure when er is falsey:

 curl('https://google.com', done.whenErrors(function (er, data) {
        t.equal(data, 'fooBar')
    })
ghost commented 8 years ago

Barring the integration of more fully-fledged abstractions like using Promise to handle the same situation, What about providing an API like onSuccess and onError?

I write this because I guess one of the purposes with node-test is to deal with idiomatic Node.js code, and this will only work for idiomatic code.

For non-idiomatic code the plain done callback will still work

ben-page commented 8 years ago

Yes, this is a very big problem. My first impulse is to provide the callback proxying functionality (as you described). But I haven't spent much time thinking on this or really the callback API at all. I primarily use Promises.

How would onSuccess and onError address this?

ghost commented 8 years ago

I guess if you let the done callback check the errors first, then proxy the callback function with the assertions. That way any error could fail fast when er is set.

    curl('https://google.com', done.when(function (er, data) {
      t.equal(data, 'foo')
    })

And in cases where a truthy er is expected, triggering failure when er is falsey:

    curl('https://google.com', done.whenErrors(function (er, data) {
      t.equal(data, 'foo')
    })

However this would only work for idiomatic Node.js code. For non-idiomatic code they can still use the plain done. And onSuccess and onError would be used here, but they need to be symmetrical. Barring the integration of more fully-fledged abstractions like using Promise to handle the same situation, I think this kind of API is ideal.

ben-page commented 8 years ago

In the latest beta, I added t.async() to wrap asynchronous callbacks. I'll release this soon.