lukechilds/node - node - Gitea: Git with a cup of tea

16694 Commits

131 Branches

446 Tags

325 MiB

Tree: 21b2440176

Commit Graph

Author	SHA1	Message	Date
Rich Trott	c2e838ee13	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: ```js const validatorFunction = common.expectsError({code: 'ELOOP', type: Error, message: 'foo'}); ``` This, by comparison, is harder to grok: ```js const validatorFunction = common.expectsError('ELOOP', Error, 'foo'); ``` And this is especially wat-inducing: ```js 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. PR-URL: https://github.com/nodejs/node/pull/11512 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>	8 years ago
Rich Trott	1f911e1cd1	test: use common.expectsError() Use common.expectsError() instead of rolling own validation function in test-http-request-invalid-method-error.js. common.expectsError() didn't exist when the test was written, but now it does. PR-URL: https://github.com/nodejs/node/pull/11408 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>	8 years ago
Ashton Kinslow	c5ccc50d1f	test: test for http.request() invalid method error Adds a test for when an invalid http method is passed into http.request() to verify an error is thrown, that the error is the correct type, and the error message is correct. PR-URL: https://github.com/nodejs/node/pull/10080 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>	8 years ago

Author

SHA1

Message

Date

Rich Trott

c2e838ee13

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:

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

This, by comparison, is harder to grok:

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

And this is especially wat-inducing:

```js
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.

PR-URL: https://github.com/nodejs/node/pull/11512
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

8 years ago

Rich Trott

1f911e1cd1

test: use common.expectsError()

Use common.expectsError() instead of rolling own validation function in
test-http-request-invalid-method-error.js. common.expectsError() didn't
exist when the test was written, but now it does.

PR-URL: https://github.com/nodejs/node/pull/11408
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

8 years ago

Ashton Kinslow

c5ccc50d1f

test: test for http.request() invalid method error

Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: https://github.com/nodejs/node/pull/10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

8 years ago

3 Commits (21b24401768aa111142522fd56a49f8de805fd74)