Browse Source

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: https://github.com/nodejs/node/pull/10671
Ref: https://github.com/nodejs/node/pull/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>
v6
Teddy Katz 8 years ago
committed by James M Snell
parent
commit
5d31448844
  1. 1
      test/.eslintrc
  2. 2
      test/parallel/test-crypto-pbkdf2.js
  3. 5
      test/parallel/test-fs-long-path.js
  4. 2
      test/parallel/test-fs-readfile-pipe.js
  5. 2
      test/parallel/test-fs-symlink-dir-junction-relative.js
  6. 8
      test/parallel/test-fs-symlink-dir-junction.js
  7. 12
      test/parallel/test-preload.js
  8. 5
      test/pummel/test-regress-GH-814.js
  9. 5
      test/pummel/test-regress-GH-814_2.js
  10. 42
      tools/eslint-rules/prefer-assert-iferror.js

1
test/.eslintrc

@ -3,4 +3,5 @@
rules: rules:
## common module is mandatory in tests ## common module is mandatory in tests
required-modules: [2, common] required-modules: [2, common]
prefer-assert-iferror: 2
prefer-assert-methods: 2 prefer-assert-methods: 2

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

@ -52,7 +52,7 @@ assert.strictEqual(key.toString('hex'), expected);
crypto.pbkdf2('password', 'salt', 32, 32, 'sha256', common.mustCall(ondone)); crypto.pbkdf2('password', 'salt', 32, 32, 'sha256', common.mustCall(ondone));
function ondone(err, key) { function ondone(err, key) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(key.toString('hex'), expected); assert.strictEqual(key.toString('hex'), expected);
} }

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

@ -2,6 +2,7 @@
const common = require('../common'); const common = require('../common');
const fs = require('fs'); const fs = require('fs');
const path = require('path'); const path = require('path');
const assert = require('assert');
if (!common.isWindows) { if (!common.isWindows) {
common.skip('this test is Windows-specific.'); common.skip('this test is Windows-specific.');
@ -21,10 +22,10 @@ console.log({
}); });
fs.writeFile(fullPath, 'ok', common.mustCall(function(err) { fs.writeFile(fullPath, 'ok', common.mustCall(function(err) {
if (err) throw err; assert.ifError(err);
fs.stat(fullPath, common.mustCall(function(err, stats) { fs.stat(fullPath, common.mustCall(function(err, stats) {
if (err) throw err; assert.ifError(err);
})); }));
})); }));

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

@ -15,7 +15,7 @@ const dataExpected = fs.readFileSync(__filename, 'utf8');
if (process.argv[2] === 'child') { if (process.argv[2] === 'child') {
fs.readFile('/dev/stdin', function(er, data) { fs.readFile('/dev/stdin', function(er, data) {
if (er) throw er; assert.ifError(er);
process.stdout.write(data); process.stdout.write(data);
}); });
return; return;

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

@ -15,7 +15,7 @@ common.refreshTmpDir();
// Test fs.symlink() // Test fs.symlink()
fs.symlink(linkData, linkPath1, 'junction', common.mustCall(function(err) { fs.symlink(linkData, linkPath1, 'junction', common.mustCall(function(err) {
if (err) throw err; assert.ifError(err);
verifyLink(linkPath1); verifyLink(linkPath1);
})); }));

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

@ -14,18 +14,18 @@ console.log('linkData: ' + linkData);
console.log('linkPath: ' + linkPath); console.log('linkPath: ' + linkPath);
fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) { fs.symlink(linkData, linkPath, 'junction', common.mustCall(function(err) {
if (err) throw err; assert.ifError(err);
fs.lstat(linkPath, common.mustCall(function(err, stats) { fs.lstat(linkPath, common.mustCall(function(err, stats) {
if (err) throw err; assert.ifError(err);
assert.ok(stats.isSymbolicLink()); assert.ok(stats.isSymbolicLink());
fs.readlink(linkPath, common.mustCall(function(err, destination) { fs.readlink(linkPath, common.mustCall(function(err, destination) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(destination, linkData); assert.strictEqual(destination, linkData);
fs.unlink(linkPath, common.mustCall(function(err) { fs.unlink(linkPath, common.mustCall(function(err) {
if (err) throw err; assert.ifError(err);
assert(!common.fileExists(linkPath)); assert(!common.fileExists(linkPath));
assert(common.fileExists(linkData)); assert(common.fileExists(linkData));
})); }));

12
test/parallel/test-preload.js

@ -34,7 +34,7 @@ const fixtureThrows = fixture('throws_error4.js');
// test preloading a single module works // test preloading a single module works
childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB, childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB,
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\n'); assert.strictEqual(stdout, 'A\nB\n');
}); });
@ -42,7 +42,7 @@ childProcess.exec(nodeBinary + ' ' + preloadOption([fixtureA]) + ' ' + fixtureB,
childProcess.exec( childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA, fixtureB]) + ' ' + fixtureC, nodeBinary + ' ' + preloadOption([fixtureA, fixtureB]) + ' ' + fixtureC,
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\nC\n'); assert.strictEqual(stdout, 'A\nB\nC\n');
} }
); );
@ -63,7 +63,7 @@ childProcess.exec(
childProcess.exec( childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA]) + '-e "console.log(\'hello\');"', nodeBinary + ' ' + preloadOption([fixtureA]) + '-e "console.log(\'hello\');"',
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(stdout, 'A\nhello\n'); assert.strictEqual(stdout, 'A\nhello\n');
} }
); );
@ -110,7 +110,7 @@ childProcess.exec(
nodeBinary + ' ' + preloadOption([fixtureA]) + nodeBinary + ' ' + preloadOption([fixtureA]) +
'-e "console.log(\'hello\');" ' + preloadOption([fixtureA, fixtureB]), '-e "console.log(\'hello\');" ' + preloadOption([fixtureA, fixtureB]),
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.strictEqual(stdout, 'A\nB\nhello\n'); assert.strictEqual(stdout, 'A\nB\nhello\n');
} }
); );
@ -131,7 +131,7 @@ childProcess.exec(
nodeBinary + ' ' + '--require ' + fixture('cluster-preload.js') + ' ' + nodeBinary + ' ' + '--require ' + fixture('cluster-preload.js') + ' ' +
fixture('cluster-preload-test.js'), fixture('cluster-preload-test.js'),
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.ok(/worker terminated with code 43/.test(stdout)); assert.ok(/worker terminated with code 43/.test(stdout));
} }
); );
@ -142,7 +142,7 @@ childProcess.exec(
nodeBinary + ' ' + '--expose_debug_as=v8debug ' + '--require ' + nodeBinary + ' ' + '--expose_debug_as=v8debug ' + '--require ' +
fixture('cluster-preload.js') + ' ' + 'cluster-preload-test.js', fixture('cluster-preload.js') + ' ' + 'cluster-preload-test.js',
function(err, stdout, stderr) { function(err, stdout, stderr) {
if (err) throw err; assert.ifError(err);
assert.ok(/worker terminated with code 43/.test(stdout)); assert.ok(/worker terminated with code 43/.test(stdout));
} }
); );

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

@ -2,6 +2,7 @@
// Flags: --expose_gc // Flags: --expose_gc
const common = require('../common'); const common = require('../common');
const assert = require('assert');
function newBuffer(size, value) { function newBuffer(size, value) {
var buffer = Buffer.allocUnsafe(size); var buffer = Buffer.allocUnsafe(size);
@ -59,7 +60,5 @@ var timeToQuit = Date.now() + 8e3; //Test during no more than this seconds.
function cb(err, written) { function cb(err, written) {
if (err) { assert.ifError(err);
throw err;
}
} }

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

@ -2,6 +2,7 @@
// Flags: --expose_gc // Flags: --expose_gc
const common = require('../common'); const common = require('../common');
const assert = require('assert');
const fs = require('fs'); const fs = require('fs');
const testFileName = require('path').join(common.tmpDir, 'GH-814_test.txt'); const testFileName = require('path').join(common.tmpDir, 'GH-814_test.txt');
@ -64,9 +65,7 @@ function writer() {
function writerCB(err, written) { function writerCB(err, written) {
//console.error('cb.'); //console.error('cb.');
if (err) { assert.ifError(err);
throw err;
}
} }

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

@ -0,0 +1,42 @@
/**
* @fileoverview Prohibit the `if (err) throw err;` pattern
* @author Teddy Katz
*/
'use strict';
module.exports = {
create(context) {
const sourceCode = context.getSourceCode();
function hasSameTokens(nodeA, nodeB) {
const aTokens = sourceCode.getTokens(nodeA);
const bTokens = sourceCode.getTokens(nodeB);
return aTokens.length === bTokens.length &&
aTokens.every((token, index) => {
return token.type === bTokens[index].type &&
token.value === bTokens[index].value;
});
}
return {
IfStatement(node) {
const firstStatement = node.consequent.type === 'BlockStatement' ?
node.consequent.body[0] :
node.consequent;
if (
firstStatement &&
firstStatement.type === 'ThrowStatement' &&
hasSameTokens(node.test, firstStatement.argument)
) {
context.report({
node: firstStatement,
message: 'Use assert.ifError({{argument}}) instead.',
data: {argument: sourceCode.getText(node.test)}
});
}
}
};
}
};
Loading…
Cancel
Save