ava: Simplify t.throws()

Alternative proposal.

Make it t.throws(fn, matchers, [message]);, where matchers is an object of different matchers that all has to match. Matchers can be string or regex.

This could easily be an addition to supporting string/regex/constructor in the second argument, for backwards compatibility.

For example:

t.throws(() => foo(), {
    constructor: RangeError,
    message: /unicorn pasta is/
});

Compared to the current way:

const err = t.throws(() => foo(), /unicorn pasta is/);
t.true(err instanceof RangeError);

Possible matchers constructor, message. Could also consider supporting name for when you don’t have easy access to the constructor function. Maybe also stack if you want to ensure something is part of it, though I don’t think I’ve ever needed that.

Benefits of this is that it makes everything explicit. No more wondering for users what t.throws(() => foo(), 'Unicorn') matches against - Is it the name, type, or message. There’s also benefit in being able to specify all the constraints directly instead of in separate calls later, like potential for a better diff.

---

We should also have much better validation logic to prevent mistakes. For example, using is-error-constructor on the constructor matcher.

It would still return the error for extra assertions, right?

Yes

Just support only constructors in the second argument, since that’s the only type that is non-ambiguous for what it is matching. Delegate everything else to t.is(), t.regex(), etc.

It creates verbose code for common cases though. The most common case is wanting to ensure the error is the correct type and has the correct message. I think we should optimize for that.

@StoneCypher When we implement t.throws() in AVA directly we’ll make sure it explains why a thrown value still causes the assertion to fail.

@AlexTes

I’d actually lean against Sindre’s suggestion of possibly adding a name matcher. Can I ask, when would a name be preferred over a constructor?

I don’t personally need this feature much, but for sake of argument…

Say myApp() uses a database under the hood and potentially rejects with various DatabaseErrors. You probably want to make assertions about failure cases for myApp() working with the database, but the relevant error constructor does not live in your app and very well may not be exposed by the database driver library (it is uncommon to export all such classes).

You still need to distinguish between different types of errors and, without the constructor, name is the preferred way to ID an error (should be more stable than message). Changes to these are arguably semver major, so you should probably test them. This is the best you can do without the constructor.

test(async (t) => {
    const option = {
        databaseName : 'somethingThatDoesNotExist'
    };
    t.throws(myApp(option), {
        name : 'DatabaseNotFoundError'
    });
});

@AlexTes

Can I ask, when would a name be preferred over a constructor?

It’s usually better to compare errors using their .name. if (error instanceof MyError) doesn’t necessarily work across VM contexts, for example. Libraries might also not bother exporting their error constructors.

String Currently unsupported by Node. Is this one to be an exact match of the string representation of the error? Or just the message of the error

It’s currently implemented as comparing the message. We should do the same with the regular expression argument, IMO.

Historically the problem with the throws assertion is that it tries to do too many things, making it hard to understand what you’re actually testing. Running a regular expression against the string representation of an error is an example of that. I didn’t even know that’s how it worked!

Personally I don’t think we should maintain backwards compatibility, given how confusing throws is.

@AlexTes we can go with @sindresorhus’ proposal: https://github.com/avajs/ava/issues/1047#issuecomment-261483195

Match constructor by reference, name by equality, and message either by testing a regular expression or string equality.

We should also (and I don’t remember whether this is already something that we’re doing) fail and show an error when the thrown error is a string and not an Error.

Yes that’s implied.

From what I see, you’re proposing the removal of the message too? That would make it inconsistent with the other APIs. Not sure I agree here.

Oh no! Forgot to add that…

I think that rules out syntax like t.throws(fn, SyntaxError, 'string'), since that collides with t.throws(fn, SyntaxError, 'message'). Padding it is just annoying (t.throws(fn, SyntaxError, undefined, 'message'). Though you would need to pad t.throws(fn, undefined, 'message'). I think that’s OK though.

Conclusions so far:

Does t.throws(fn, Constructor) require the thrown value to be an Error?

Yes.

Should t.throws(fn, 'string') and t.throws(fn, /regexp/) be supported at all, or would it be preferable to dereference the error and then use another assertion?

We’ll support this interface.

Is there a need for t.throws(fn, SyntaxError, 'string') and t.throws(fn, SyntaxError, /regexp/)

Leaning towards not supporting this.

---

@vdemedes

Does anybody have experience with / examples of asserting Error instances across contexts?

Didn’t completely understand the question, mind providing a quick example?

const err1 = new Error()
const err2 = vm.runInNewContext('new Error()')
err1 instanceof Error // true
err2 instanceof Error // false

---

@fearphage yea AVA just uses Node.js’ API, and it gets messy fast. Hence this proposal.