t.notThrows(promise) should return an "empty" promise

[novemberborn]

This is as expected:

const value = Symbol()
t.notThrows(() => value) === undefined

However:

const value = Symbol()
(await t.notThrows(Promise.resolve(value))) === value

t.notThrows(promise) should return a promise, but for an undefined value.

The offending line is at

ava/lib/assert.js

Line 124 in dcdfbee

return fn

.

The Flow and TypeScript definitions should be updated too:

• ava/index.js.flow

  Line 63 in dcdfbee

  notThrows<U>(value: PromiseLike<U>, message?: string): Promise<U>;

• ava/types/base.d.ts

  Line 68 in dcdfbee

  notThrows<U>(value: PromiseLike<U>, message?: string): Promise<U>;

[sholladay]

I think I'm using this somewhere. Is it really incorrect? t.throws() returns the error, this being the inverse seems reasonable. What harm does it do?

[jfmengels]

t.notThrows(promise) should return a promise, but for an undefined value.

Why should it actually? Isn't it more useful if you get the Promise's value, so that you can make more assertions on it? I don't see the value in having it always return undefined.

[sholladay]

Isn't it more useful if you get the Promise's value, so that you can make more assertions on it?

Yeah, exactly.

Here is at least one place I am using it: https://github.com/sholladay/test-engine/blob/ba56e03eea3a148d431cf021b9624dd2ef2d8406/test.js#L61

Happy to change if there is a reason this is bad, though.

[sindresorhus]

Why should it actually? Isn't it more useful if you get the Promise's value, so that you can make more assertions on it? I don't see the value in having it always return undefined.

I think that would be surprising and inconsistent. This is assertion about errors, not the value. Also, you already have the promise, so what's really the benefit of having its returned. Promises are only resolved once anyways, so you can just do await promise and get the already resolved value without any wait.

[sholladay]

const prom = getProm();
await t.notThrows(prom);
t.truthy(await prom);
t.true(typeof (await prom) === 'string');

vs

const val = await t.notThrows(getProm());
t.truthy(val);
t.true(typeof val === 'string');

[sindresorhus]

I would rather say:

const prom = getProm();
await t.notThrows(prom);
t.is(typeof (await prom), 'string');

vs

const val = await t.notThrows(getProm());
t.is(typeof val, 'string');

I think the former, while a tiny bit more verbose, is much clearer.

[novemberborn]

IMO the parallel should be with the the form of notThrows that is passed a function:

const val = t.notThrows(() => ({foo: 'bar'}))
t.deepEqual(val, {foo: 'bar'})

Is that a sensible thing to do, or is it not? Currently it doesn't work, and we should resolve that inconsistency one way or the other.