Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 045b37b

Browse filesBrowse files
BridgeARMylesBorins
BridgeAR
authored and
MylesBorins
committed
test: add eslint rule to verify assertion input
The input for `assert.deepStrictEqual` and similar expect the actual input first and the expected input as second argument. This verifies that this is actually done correct in our tests. This is important so the possible error message actually makes sense. PR-URL: #20718 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 3fd6724 commit 045b37b
Copy full SHA for 045b37b

File tree

Expand file treeCollapse file tree

4 files changed

+37
-10
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+37
-10
lines changed
Open diff view settings
Collapse file

‎test/.eslintrc.yaml‎

Copy file name to clipboardExpand all lines: test/.eslintrc.yaml
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,33 @@ rules:
1919
## common module is mandatory in tests
2020
node-core/required-modules: [error, common]
2121

22+
no-restricted-syntax:
23+
# Config copied from .eslintrc.js
24+
- error
25+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='doesNotThrow']"
26+
message: "Please replace `assert.doesNotThrow()` and add a comment next to the code instead."
27+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='rejects'][arguments.length<2]"
28+
message: "assert.rejects() must be invoked with at least two arguments."
29+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.1.type='Literal']:not([arguments.1.regex])"
30+
message: "Use an object as second argument of assert.throws()"
31+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='throws'][arguments.length<2]"
32+
message: "assert.throws() must be invoked with at least two arguments."
33+
- selector: "CallExpression[callee.name='setTimeout'][arguments.length<2]"
34+
message: "setTimeout() must be invoked with at least two arguments."
35+
- selector: "CallExpression[callee.name='setInterval'][arguments.length<2]"
36+
message: "setInterval() must be invoked with at least 2 arguments."
37+
- selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
38+
message: "Use new keyword when throwing an Error."
39+
# Specific to /test
40+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='notDeepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
41+
message: "The first argument should be the `actual`, not the `expected` value."
42+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='notStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
43+
message: "The first argument should be the `actual`, not the `expected` value."
44+
- selector: "CallExpression[callee.object.name='assert'][callee.property.name='deepStrictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
45+
message: "The first argument should be the `actual`, not the `expected` value."
46+
# TODO: Activate the `strictEqual` rule as soon as it produces less churn.
47+
# - selector: "CallExpression[callee.object.name='assert'][callee.property.name='strictEqual'][arguments.0.type='Literal']:not([arguments.1.type='Literal']):not([arguments.1.type='ObjectExpression']):not([arguments.1.type='ArrayExpression'])"
48+
# message: "The first argument should be the `actual`, not the `expected` value."
2249
# Global scoped methods and vars
2350
globals:
2451
WebAssembly: false
Collapse file

‎test/addons-napi/test_conversions/test.js‎

Copy file name to clipboardExpand all lines: test/addons-napi/test_conversions/test.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ assert.deepStrictEqual(new String(''), test.toObject(''));
118118
assert.deepStrictEqual(new Number(0), test.toObject(0));
119119
assert.deepStrictEqual(new Number(Number.NaN), test.toObject(Number.NaN));
120120
assert.deepStrictEqual(new Object(testSym), test.toObject(testSym));
121-
assert.notDeepStrictEqual(false, test.toObject(false));
122-
assert.notDeepStrictEqual(true, test.toObject(true));
123-
assert.notDeepStrictEqual('', test.toObject(''));
124-
assert.notDeepStrictEqual(0, test.toObject(0));
121+
assert.notDeepStrictEqual(test.toObject(false), false);
122+
assert.notDeepStrictEqual(test.toObject(true), true);
123+
assert.notDeepStrictEqual(test.toObject(''), '');
124+
assert.notDeepStrictEqual(test.toObject(0), 0);
125125
assert.ok(!Number.isNaN(test.toObject(Number.NaN)));
126126

127127
assert.strictEqual('', test.toString(''));
Collapse file

‎test/parallel/test-child-process-exec-env.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-env.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ function after(err, stdout, stderr) {
3434
console.log(`error!: ${err.code}`);
3535
console.log(`stdout: ${JSON.stringify(stdout)}`);
3636
console.log(`stderr: ${JSON.stringify(stderr)}`);
37-
assert.strictEqual(false, err.killed);
37+
assert.strictEqual(err.killed, false);
3838
} else {
3939
success_count++;
40-
assert.notStrictEqual('', stdout);
40+
assert.notStrictEqual(stdout, '');
4141
}
4242
}
4343

@@ -56,7 +56,7 @@ child.stdout.on('data', function(chunk) {
5656

5757
process.on('exit', function() {
5858
console.log('response: ', response);
59-
assert.strictEqual(1, success_count);
60-
assert.strictEqual(0, error_count);
59+
assert.strictEqual(success_count, 1);
60+
assert.strictEqual(error_count, 0);
6161
assert.ok(response.includes('HELLO=WORLD'));
6262
});
Collapse file

‎test/parallel/test-crypto.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-crypto.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ assert(tlsCiphers.every((value) => noCapitals.test(value)));
129129
validateList(tlsCiphers);
130130

131131
// Assert that we have sha1 and sha256 but not SHA1 and SHA256.
132-
assert.notStrictEqual(0, crypto.getHashes().length);
132+
assert.notStrictEqual(crypto.getHashes().length, 0);
133133
assert(crypto.getHashes().includes('sha1'));
134134
assert(crypto.getHashes().includes('sha256'));
135135
assert(!crypto.getHashes().includes('SHA1'));
@@ -139,7 +139,7 @@ assert(!crypto.getHashes().includes('rsa-sha1'));
139139
validateList(crypto.getHashes());
140140

141141
// Assume that we have at least secp384r1.
142-
assert.notStrictEqual(0, crypto.getCurves().length);
142+
assert.notStrictEqual(crypto.getCurves().length, 0);
143143
assert(crypto.getCurves().includes('secp384r1'));
144144
assert(!crypto.getCurves().includes('SECP384R1'));
145145
validateList(crypto.getCurves());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.