Skip to content

Commit

Permalink
test: fix require-deps-deprecation for installed deps
Browse files Browse the repository at this point in the history
Test test-require-deps-deprecation.js was failing when user already had
node installed with acorn in require.resolve range.

Modified test to acknowledge the possibility and throw only if acorn is
found in the deps directory.

PR-URL: #17848
Fixes: #17148
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Tiriel authored and apapirovski committed Jan 3, 2018
1 parent e6a401e commit 30892c8
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion test/parallel/test-require-deps-deprecation.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ for (const m of deprecatedModules) {
} catch (err) {}
}

// Instead of checking require, check that resolve isn't pointing toward a
// built-in module, as user might already have node installed with acorn in
// require.resolve range.
// Ref: https://github.com/nodejs/node/issues/17148
for (const m of deps) {
assert.throws(() => { require(m); }, /^Error: Cannot find module/);
let path;
try {
path = require.resolve(m);
} catch (err) {
assert.ok(err.toString().startsWith('Error: Cannot find module '));
continue;
}
assert.notStrictEqual(path, m);
}

0 comments on commit 30892c8

Please sign in to comment.