test: change common.expectsError() signature

[Trott]

One downside to common.expectsError() is that it increases the
abstractions people have to learn about in order to work with even
simple tests. Whereas before, all they had to know about is
assert.throws(), now they have to also know about
common.expectsError(). This is very different (IMO) from
common.mustCall() in that the latter has an intuitively understandable
name, accepts arguments as one would expect, and (in most cases) doesn't
actually require reading documentation or code to figure out what it's
doing. With common.expectsError(), there's a fair bit of magic. Like,
it's not obvious what the first argument would be. Or the second. Or the
third. You just have to know.

This PR changes the arguments accepted by common.expectsError() to a
single settings object. Someone coming across this has a hope of
understanding what's going on without reading source or docs:

const validatorFunction = common.expectsError({code: 'ELOOP',
                                               type: Error,
                                               message: 'foo'});

This, by comparison, is harder to grok:

const validatorFunction = common.expectsError('ELOOP',
                                               Error,
                                               'foo');

And this is especially wat-inducing:

common.expectsError(undefined, undefined, 'looped doodad found');

It's likely that only people who work with tests frequently can be
expected to remember the three arguments and their order. By comparison,
remembering that the error code is code and the message is message
might be manageable.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

test

[Trott]

Note that this is backwards compatible with the current API as there are open PRs that use that API. However, the end goal is to move everything to the API introduced here (which won't be difficult to do).

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request/6554/

[gibfahn]

Note that this is backwards compatible with the current API as there are open PRs that use that API. However, the end goal is to move everything to the API introduced here (which won't be difficult to do).

I get the need for backwards compat, but if we're going to change this we should remove the old way pretty soon (say a week or two after this lands). We don't want two suggested ways of doing it, that'll cause more confusion.

EDIT: I'd much rather we just directly change it as @jasnell suggests though.

[jasnell]

I'd rather not worry about the backwards API compatibility here. Just change the signature and the existing PRs can be updated

[joyeecheung]

+1 to just changing the signature

[Trott]

OK, I removed the backwards-compatibility.

CI: https://ci.nodejs.org/job/node-test-pull-request/6589/

[jasnell]

landed in c2e838e

[jasnell]

marking don't land on v4 and v6 since the common.expectsError() has not been backported yet