Skip to content

test: change common.expectsError() signature

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

Merge request reports

Loading