t.throws(promise, stringOrRegexp) should only test against errors

[jamestalmage]

If you are rejecting a Promise, you should be doing so with an Error object.

If you are testing that a promise will be rejected, we currently allow this:

t.throws(somePromise, /messageRegexp/);
t.throws(somePromise, 'expectedMessage');

That makes sense.

However. If the promise is rejected with a string:

Promise.reject('someErrorMessage');

We magically handle detecting it's not an error and test the expectation directly against the string instead of error.message.

I think this is silly. We are accommodating bad practice (rejecting with non-errors), and creating an ambiguous assertion conditions:

// There is no way to differentiate these using t.throws
var promiseA = Promise.reject('foo');
var promiseB = Promise.reject(new Error('foo'));

I propose we drop the magic string behavior.

If they specify the second arg, we use it as an expectation against rejectionReason.message only.

[vadimdemedes]

👏👏👏

[sindresorhus]

👍 👍 👍

[corinna000]

I was looking into this and had a couple of questions.

It looks like @jamestalmage would like AVA to have an opinion about Promises that fail to reject with Errors.

• If the conditional logic testing for an Error is removed and we throw fnError, the test still passes if the Promise only rejects with a string.
• As far as I can tell this would be the only assertion that would have an opinion about the SUT. How should AVA notify the developer?
• An Observable converted to a Promises will now be expect the the onError function to throw an Error. Is that right?

[novemberborn]

@corinna000 the way I read it is that t.throws() lets you assert something about the error message. If the error doesn't have a message then your assertion should fail. Right now a string is treated as the error message, and it shouldn't.

[novemberborn]

@jamestalmage

We magically handle detecting it's not an error and test the expectation directly against the string instead of error.message.

Looking at the assert code just now it seems we convert non-Error rejection reasons to Error instances, with the reason being cast to the message string. Then the expectations are applied against that message string. This has your described effect when the rejection reason is a string, but may have other effects if the reason is a different type.

IMO we should pass the rejection reason to core-assert and let it determine whether it matches, without any conversion.