From 76af4f0d05e3e1eded391dd2f3db71bb4480429a Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 25 Apr 2019 00:46:40 +0200
Subject: [PATCH] tools: prohibit `assert.doesNotReject()` in Node.js core
This makes sure we do not use `assert.doesNotReject()` anywhere in
our code base. This is just a simple wrapper that catches the
rejection and then rejects it again in case of an error. Thus, it is
not useful to do that.
The error message for `assert.doesNotThrow()` is also improved.
PR-URL: https://github.com/nodejs/node/pull/27402
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
.eslintrc.js | 6 ++-
doc/api/assert.md | 2 +
test/parallel/test-assert-async.js | 68 +++++++++++++-------------
test/parallel/test-fs-readdir-types.js | 5 +-
4 files changed, 43 insertions(+), 38 deletions(-)
diff --git a/.eslintrc.js b/.eslintrc.js
index 9aa90c6366232d..cfada1f65c181e 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -184,7 +184,11 @@ module.exports = {
},
{
selector: "CallExpression[callee.property.name='doesNotThrow']",
- message: 'Please replace `assert.doesNotThrow()` and add a comment next to the code instead.',
+ message: 'Do not use `assert.doesNotThrow()`. Write the code without the wrapper and add a comment instead.',
+ },
+ {
+ selector: "CallExpression[callee.property.name='doesNotReject']",
+ message: 'Do not use `assert.doesNotReject()`. Write the code without the wrapper and add a comment instead.',
},
{
selector: "CallExpression[callee.property.name='rejects'][arguments.length<2]",
diff --git a/doc/api/assert.md b/doc/api/assert.md
index 9028d21f0d9df3..0a7d35f105e48e 100644
--- a/doc/api/assert.md
+++ b/doc/api/assert.md
@@ -454,6 +454,7 @@ function. See [`assert.throws()`][] for more details.
Besides the async nature to await the completion behaves identically to
[`assert.doesNotThrow()`][].
+<!-- eslint-disable no-restricted-syntax -->
```js
(async () => {
await assert.doesNotReject(
@@ -465,6 +466,7 @@ Besides the async nature to await the completion behaves identically to
})();
```
+<!-- eslint-disable no-restricted-syntax -->
```js
assert.doesNotReject(Promise.reject(new TypeError('Wrong value')))
.then(() => {
diff --git a/test/parallel/test-assert-async.js b/test/parallel/test-assert-async.js
index 140bd05d1a3c8f..8aad1d865c097e 100644
--- a/test/parallel/test-assert-async.js
+++ b/test/parallel/test-assert-async.js
@@ -114,11 +114,36 @@ promises.push(assert.rejects(
}
));
+{
+ const handler = (generated, actual, err) => {
+ assert.strictEqual(err.generatedMessage, generated);
+ assert.strictEqual(err.code, 'ERR_ASSERTION');
+ assert.strictEqual(err.actual, actual);
+ assert.strictEqual(err.operator, 'rejects');
+ assert(/rejects/.test(err.stack));
+ return true;
+ };
+ const err = new Error();
+ promises.push(assert.rejects(
+ assert.rejects(Promise.reject(null), { code: 'FOO' }),
+ handler.bind(null, true, null)
+ ));
+ promises.push(assert.rejects(
+ assert.rejects(Promise.reject(5), { code: 'FOO' }, 'AAAAA'),
+ handler.bind(null, false, 5)
+ ));
+ promises.push(assert.rejects(
+ assert.rejects(Promise.reject(err), { code: 'FOO' }, 'AAAAA'),
+ handler.bind(null, false, err)
+ ));
+}
+
// Check `assert.doesNotReject`.
{
// `assert.doesNotReject` accepts a function or a promise
// or a thenable as first argument.
- const promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
+ /* eslint-disable no-restricted-syntax */
+ let promise = assert.doesNotReject(() => new Map(), common.mustNotCall());
promises.push(assert.rejects(promise, {
message: 'Expected instance of Promise to be returned ' +
'from the "promiseFn" function but got instance of Map.',
@@ -149,9 +174,7 @@ promises.push(assert.rejects(
code: 'ERR_INVALID_RETURN_VALUE'
})
);
-}
-{
const handler1 = (err) => {
assert(err instanceof assert.AssertionError,
`${err.name} is not instance of AssertionError`);
@@ -173,7 +196,7 @@ promises.push(assert.rejects(
const rejectingFn = async () => assert.fail();
- let promise = assert.doesNotReject(rejectingFn, common.mustCall(handler1));
+ promise = assert.doesNotReject(rejectingFn, common.mustCall(handler1));
promises.push(assert.rejects(promise, common.mustCall(handler2)));
promise = assert.doesNotReject(rejectingFn(), common.mustCall(handler1));
@@ -181,39 +204,16 @@ promises.push(assert.rejects(
promise = assert.doesNotReject(() => assert.fail(), common.mustNotCall());
promises.push(assert.rejects(promise, common.mustCall(handler1)));
-}
-promises.push(assert.rejects(
- assert.doesNotReject(123),
- {
- code: 'ERR_INVALID_ARG_TYPE',
- message: 'The "promiseFn" argument must be one of type ' +
- 'Function or Promise. Received type number'
- }
-));
-
-{
- const handler = (generated, actual, err) => {
- assert.strictEqual(err.generatedMessage, generated);
- assert.strictEqual(err.code, 'ERR_ASSERTION');
- assert.strictEqual(err.actual, actual);
- assert.strictEqual(err.operator, 'rejects');
- assert(/rejects/.test(err.stack));
- return true;
- };
- const err = new Error();
promises.push(assert.rejects(
- assert.rejects(Promise.reject(null), { code: 'FOO' }),
- handler.bind(null, true, null)
- ));
- promises.push(assert.rejects(
- assert.rejects(Promise.reject(5), { code: 'FOO' }, 'AAAAA'),
- handler.bind(null, false, 5)
- ));
- promises.push(assert.rejects(
- assert.rejects(Promise.reject(err), { code: 'FOO' }, 'AAAAA'),
- handler.bind(null, false, err)
+ assert.doesNotReject(123),
+ {
+ code: 'ERR_INVALID_ARG_TYPE',
+ message: 'The "promiseFn" argument must be one of type ' +
+ 'Function or Promise. Received type number'
+ }
));
+ /* eslint-enable no-restricted-syntax */
}
// Make sure all async code gets properly executed.
diff --git a/test/parallel/test-fs-readdir-types.js b/test/parallel/test-fs-readdir-types.js
index 96a3b73098728d..78f1b0d4e1b627 100644
--- a/test/parallel/test-fs-readdir-types.js
+++ b/test/parallel/test-fs-readdir-types.js
@@ -72,13 +72,12 @@ fs.readdir(readdirDir, {
assertDirents(dirents);
}));
-// Check the promisified version
-assert.doesNotReject(async () => {
+(async () => {
const dirents = await fs.promises.readdir(readdirDir, {
withFileTypes: true
});
assertDirents(dirents);
-});
+})();
// Check for correct types when the binding returns unknowns
const UNKNOWN = constants.UV_DIRENT_UNKNOWN;