FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tools,test: enforce assert.ifError with lint rule · nodejs/node@3adda4b · GitHub

/ node Public

Commit 3adda4b

Browse files
not-an-aardvark
authored and
Italo A. Casas
committed
tools,test: enforce assert.ifError with lint rule
This adds an ESLint rule to enforce the use of `assert.ifError(err)` instead of `if (err) throw err;` in tests. PR-URL: #10671 Ref: #10543 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Michal Zasso <targos@protonmail.com>
1 parent 727c5e3 commit 3adda4b

10 files changed

Lines changed: 63 additions & 21 deletions

‎test/.eslintrc.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
rules:
44
## common module is mandatory in tests
55
required-modules: [2, common]
6+
prefer-assert-iferror: 2
67
prefer-assert-methods: 2

‎test/parallel/test-crypto-pbkdf2.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ assert.strictEqual(key.toString('hex'), expected);
5252

5353
crypto.pbkdf2('password', 'salt', 32, 32, 'sha256', common.mustCall(ondone));
5454
function ondone(err, key) {
55-
if (err) throw err;
55+
assert.ifError(err);
5656
assert.strictEqual(key.toString('hex'), expected);
5757
}
5858

‎test/parallel/test-fs-long-path.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const common = require('../common');
33
const fs = require('fs');
44
const path = require('path');
5+
const assert = require('assert');
56

67
if (!common.isWindows) {
78
common.skip('this test is Windows-specific.');
@@ -21,10 +22,10 @@ console.log({
2122
});
2223

2324
fs.writeFile(fullPath, 'ok', common.mustCall(function(err) {
24-
if (err) throw err;
25+
assert.ifError(err);
2526

2627
fs.stat(fullPath, common.mustCall(function(err, stats) {
27-
if (err) throw err;
28+
assert.ifError(err);
2829
}));
2930
}));
3031

‎test/parallel/test-fs-readfile-pipe.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const dataExpected = fs.readFileSync(__filename, 'utf8');
1515

1616
if (process.argv[2] === 'child') {
1717
fs.readFile('/dev/stdin', function(er, data) {
18-
if (er) throw er;
18+
assert.ifError(er);
1919
process.stdout.write(data);
2020
});
2121
return;

‎test/parallel/test-fs-symlink-dir-junction-relative.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ common.refreshTmpDir();
1515

1616
// Test fs.symlink()
1717
fs.symlink(linkData, linkPath1, 'junction', common.mustCall(function(err) {
18-
if (err) throw err;
18+
assert.ifError(err);
1919
verifyLink(linkPath1);
2020
}));
2121

‎test/parallel/test-fs-symlink-dir-junction.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,18 @@ console.log('linkData: ' + linkData);
1414
console.log('linkPath: ' + linkPath);
1515

1616
fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) {
17-
if (err) throw err;
17+
assert.ifError(err);
1818

1919
fs.lstat(linkPath, common.mustCall(function(err, stats) {
20-
if (err) throw err;
20+
assert.ifError(err);
2121
assert.ok(stats.isSymbolicLink());
2222

2323
fs.readlink(linkPath, common.mustCall(function(err, destination) {
24-
if (err) throw err;
24+
assert.ifError(err);
2525
assert.strictEqual(destination, linkData);
2626

2727
fs.unlink(linkPath, common.mustCall(function(err) {
28-
if (err) throw err;
28+
assert.ifError(err);
2929
assert(!common.fileExists(linkPath));
3030
assert(common.fileExists(linkData));
3131
}));

‎test/parallel/test-preload.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ const fixtureThrows = fixture('throws_error4.js');
3434
// test preloading a single module works
3535
childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB,
3636
function(err, stdout, stderr) {
37-
if (err) throw err;
37+
assert.ifError(err);
3838
assert.strictEqual(stdout, 'A\nB\n');
3939
});
4040

4141
// test preloading multiple modules works
4242
childProcess.exec(
4343
nodeBinary + ' ' + preloadOption([fixtureA, fixtureB]) + ' ' + fixtureC,
4444
function(err, stdout, stderr) {
45-
if (err) throw err;
45+
assert.ifError(err);
4646
assert.strictEqual(stdout, 'A\nB\nC\n');
4747
}
4848
);
@@ -63,7 +63,7 @@ childProcess.exec(
6363
childProcess.exec(
6464
nodeBinary + ' ' + preloadOption([fixtureA]) + '-e "console.log(\'hello\');"',
6565
function(err, stdout, stderr) {
66-
if (err) throw err;
66+
assert.ifError(err);
6767
assert.strictEqual(stdout, 'A\nhello\n');
6868
}
6969
);
@@ -110,7 +110,7 @@ childProcess.exec(
110110
nodeBinary + ' ' + preloadOption([fixtureA]) +
111111
'-e "console.log(\'hello\');" ' + preloadOption([fixtureA, fixtureB]),
112112
function(err, stdout, stderr) {
113-
if (err) throw err;
113+
assert.ifError(err);
114114
assert.strictEqual(stdout, 'A\nB\nhello\n');
115115
}
116116
);
@@ -131,7 +131,7 @@ childProcess.exec(
131131
nodeBinary + ' ' + '--require ' + fixture('cluster-preload.js') + ' ' +
132132
fixture('cluster-preload-test.js'),
133133
function(err, stdout, stderr) {
134-
if (err) throw err;
134+
assert.ifError(err);
135135
assert.ok(/worker terminated with code 43/.test(stdout));
136136
}
137137
);
@@ -142,7 +142,7 @@ childProcess.exec(
142142
nodeBinary + ' ' + '--expose_debug_as=v8debug ' + '--require ' +
143143
fixture('cluster-preload.js') + ' ' + 'cluster-preload-test.js',
144144
function(err, stdout, stderr) {
145-
if (err) throw err;
145+
assert.ifError(err);
146146
assert.ok(/worker terminated with code 43/.test(stdout));
147147
}
148148
);

‎test/pummel/test-regress-GH-814.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Flags: --expose_gc
33

44
const common = require('../common');
5+
const assert = require('assert');
56

67
function newBuffer(size, value) {
78
var buffer = Buffer.allocUnsafe(size);
@@ -59,7 +60,5 @@ var timeToQuit = Date.now() + 8e3; //Test during no more than this seconds.
5960

6061

6162
function cb(err, written) {
62-
if (err) {
63-
throw err;
64-
}
63+
assert.ifError(err);
6564
}

‎test/pummel/test-regress-GH-814_2.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Flags: --expose_gc
33

44
const common = require('../common');
5+
const assert = require('assert');
56

67
const fs = require('fs');
78
const testFileName = require('path').join(common.tmpDir, 'GH-814_test.txt');
@@ -64,9 +65,7 @@ function writer() {
6465

6566
function writerCB(err, written) {
6667
//console.error('cb.');
67-
if (err) {
68-
throw err;
69-
}
68+
assert.ifError(err);
7069
}
7170

7271

‎tools/eslint-rules/prefer-assert-iferror.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @fileoverview Prohibit the `if (err) throw err;` pattern
3+
* @author Teddy Katz
4+
*/
5+
6+
'use strict';
7+
8+
module.exports = {
9+
create(context) {
10+
const sourceCode = context.getSourceCode();
11+
12+
function hasSameTokens(nodeA, nodeB) {
13+
const aTokens = sourceCode.getTokens(nodeA);
14+
const bTokens = sourceCode.getTokens(nodeB);
15+
16+
return aTokens.length === bTokens.length &&
17+
aTokens.every((token, index) => {
18+
return token.type === bTokens[index].type &&
19+
token.value === bTokens[index].value;
20+
});
21+
}
22+
23+
return {
24+
IfStatement(node) {
25+
const firstStatement = node.consequent.type === 'BlockStatement' ?
26+
node.consequent.body[0] :
27+
node.consequent;
28+
if (
29+
firstStatement &&
30+
firstStatement.type === 'ThrowStatement' &&
31+
hasSameTokens(node.test, firstStatement.argument)
32+
) {
33+
context.report({
34+
node: firstStatement,
35+
message: 'Use assert.ifError({{argument}}) instead.',
36+
data: {argument: sourceCode.getText(node.test)}
37+
});
38+
}
39+
}
40+
};
41+
}
42+
};

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL