`instanceof` expect.fail() is not UnexpectedError
onetom opened this issue · 7 comments
Unlike the assertions thrown by the built-in Node.js assert
, the error thrown by unexpected
can not be detected using the instanceof
operator.
The reason I'm looking into this, because I was trying to understand how node-tap distinguishes assertion errors from other errors.
I was expecting this script to yield true
for both cases:
const expect = require('unexpected')
const UnexpectedError = require('unexpected/lib/UnexpectedError');
const assert = require('assert')
const L = console.log.bind(console)
try {
assert.fail('asd')
} catch (e) {
L(e instanceof assert.AssertionError)
}
try {
expect.fail('qwe')
} catch (e) {
L(e instanceof UnexpectedError)
}
but it resulted in:
true
false
Let me know if my expectation is unreasonable and recommend a better way of telling assertion errors apart from any other error.
Before I embarked on exploring unexpected
I was under the false impression that JS runtimes provide some default, global AssertionError
, which test frameworks can rely on for separating assertion failures from test code runtime errors. I wonder why did I think this... Maybe because other languages work like this? Is there any initiative for JavaScript to standardize an assertion mechanism?
The reason why this doesn't work is that we recently added a transpilation setup (since 10.34.2), so in newer versions you'd have to use:
const UnexpectedError = require('unexpected/build/lib/UnexpectedError');
But we don't consider the internal file names a stable part of the API, so that could change in the future.
Here's how we identify UnexpectedError
instances in the library:
Lines 860 to 867 in 774539a
The this.baseType.identify
thing boils down to a generic Error
instance detection, implemented here:
Lines 120 to 123 in 774539a
Is there any initiative for JavaScript to standardize an assertion mechanism?
Not that I know of. I'm also only aware of require('assert').AssertionError
in node.js. We could try to use that when in node.js, but it'd be a bit weird as it isn't available in the browser where we'd then have another inheritance chain.
Chai has its own assertion-error
module, which does not attempt to inherit from node's AssertionError
:
const AssertionError = require('assert').AssertionError;
const expect = require('chai').expect;
try {
expect(false).to.equal(true);
} catch (err) {
console.log(err instanceof AssertionError); // false
}
Jest's expect
implementation throws instances of its own JestAssertionError
that aren't subclasses of node's AssertionError
either.
expect.js
produces regular Error
instances.
So unfortunately it seems like there's no consensus around any of this stuff :(
@papandreou thanks for the quick feedback.
i need to give more thoughts to the topic, but some level of "fix" to this issue could simply be some documentation enhancement.
in the meantime, here are my initial results of integrating unexpected
into node-tap
:
https://github.com/enumatech/sprites/blob/f1fe6af1e84404f5ad6c70212bb43e0015278867/examples/paywall/test/custom-tap.js#L25-L63
i pondered a lot today about how should it work, because if i treat expectations async all the time, that leads to more verbose test code and chatty test output (because async node-tap tests are implemented as an implicit subtest wrapper; resulting in duplicate output and an extra nesting layer)
it was surprising to realize that a except
can both throw an exception and reject the returned promise...
my latest idea is to detect synchronous assertions based on wether the promise returned by expect
is already fulfilled or not. if it's pending, i would do the subtest wrapping, but if it's resolved, i wouldn't.
not sure how reliable is that approach... is there a way to query this more directly somehow?
it was surprising to realize that a except can both throw an exception and reject the returned promise...
Yeah, that was the compromise we ended up making when we introduced support for async assertions. At that time, async/await wasn't a thing yet, so taking the leap into "always async" didn't seem like an option. And it would probably still be too radical to break pretty much the only contract that all JS assertion libraries support: That an error results in an exception being thrown. We have experimented a bit with an alternative idea where the user wouldn't need to care, but I think it'll require buy-in from test runners to get off the ground.
my latest idea is to detect synchronous assertions based on wether the promise returned by expect is already fulfilled or not. if it's pending, i would do the subtest wrapping, but if it's resolved, i wouldn't.
not sure how reliable is that approach... is there a way to query this more directly somehow?
Unexpected's promises are based on bluebird 2.9.34, so they include support for the PromiseInspection
interface. So you can:
(async () => {
const promise = expect(setImmediate, 'to call the callback');
console.log(promise.isPending()); // true
console.log(promise.isFulfilled()); // false
console.log(promise.isRejected()); // false
await new Promise(resolve => setTimeout(resolve, 10));
console.log(promise.isPending()); // false
console.log(promise.isFulfilled()); // true
console.log(promise.isRejected()); // false
})();
And you can extract the error from rejected promises using promise.reason()
.
Thanks, I think I know how to integrate it with node-tap
seamlessly, then!
I've also tested native the Promise
behaviour (in Node.js v11.5.0) to see if there is any case where I would misinterpret expect
's behaviour:
const t = require('tap')
const expect = require('unexpected')
async function run() {
await t.test('expect', async t => {
const err = (msg = '<boom>') => Error(msg)
const exn = () => { throw err() }
await t.test('sync', async t => {
t.ok(expect(123, 'to be', 123).isFulfilled())
t.throws(() => expect(123, 'to be', 999))
t.ok(expect(exn, 'to throw').isFulfilled())
t.throws(() => expect(exn, 'not to throw'))
})
await t.test('async', async t => {
const resolved = (x = 123) => Promise.resolve(x)
const rejected = (x = err()) => Promise.reject(x)
const nop = () => 0
t.ok(expect(resolved, 'to be fulfilled').isPending())
await t.resolves(expect(resolved, 'to be fulfilled'))
t.ok(expect(resolved, 'to be rejected').catch(nop).isPending())
await t.rejects(expect(resolved, 'to be rejected'))
t.ok(expect(rejected, 'to be rejected').isPending())
await t.resolves(expect(rejected, 'to be rejected'))
t.ok(expect(rejected, 'to be fulfilled').catch(nop).isPending())
await t.rejects(expect(rejected, 'to be fulfilled'))
})
})
}
run().catch(t.threw)
I can't resist promoting to try it with the unobtrusive Nix package manager:
curl https://nixos.org/nix/install | sh
nix-shell -p nodejs-11_x
mkdir /tmp/xxx; cd /tmp/xxx
npm init -y; time npm i -D tap unexpected
# copy the code to your clipboard
pbpaste >> test.js
node test.js
Just saying, because I think it's quite elegant (and very reproducible) :)
Seems like this got resolved, so I'll close. Please get in touch if you have any further questions :)