From c341f9308debb85507d84acde9851091b5c5549d Mon Sep 17 00:00:00 2001
From: Benjamin Zaslavsky <benjamin.zaslavsky@gmail.com>
Date: Sun, 24 Dec 2017 09:11:29 +0100
Subject: [PATCH 1/2] test: fix require-deps-deprecation for installed deps
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.
Also changed the deprecation test for v9.x: common.expectWarning was failing
because the required deps now throw ReferenceErrors when not properly called
internally in the right order.
PR-URL: https://github.com/nodejs/node/pull/17848
Fixes: https://github.com/nodejs/node/issues/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>
---
.../parallel/test-require-deps-deprecation.js | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 test/parallel/test-require-deps-deprecation.js
diff --git a/test/parallel/test-require-deps-deprecation.js b/test/parallel/test-require-deps-deprecation.js
new file mode 100644
index 00000000000000..b4af723b2ee8e6
--- /dev/null
+++ b/test/parallel/test-require-deps-deprecation.js
@@ -0,0 +1,52 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+// The v8 modules when imported leak globals. Disable global check.
+common.globalCheck = false;
+
+const deprecatedModules = [
+ 'node-inspect/lib/_inspect',
+ 'node-inspect/lib/internal/inspect_client',
+ 'node-inspect/lib/internal/inspect_repl',
+ 'v8/tools/SourceMap',
+ 'v8/tools/codemap',
+ 'v8/tools/consarray',
+ 'v8/tools/csvparser',
+ 'v8/tools/logreader',
+ 'v8/tools/profile',
+ 'v8/tools/profile_view',
+ 'v8/tools/splaytree',
+ 'v8/tools/tickprocessor',
+ 'v8/tools/tickprocessor-driver'
+];
+
+// Newly added deps that do not have a deprecation wrapper around it would
+// throw an error, but no warning would be emitted.
+const deps = [
+ 'acorn/dist/acorn',
+ 'acorn/dist/walk'
+];
+
+const throwingModules = () => {
+ for (const m of deprecatedModules) {
+ require(m);
+ }
+};
+
+assert.throws(throwingModules, ReferenceError);
+
+// 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) {
+ let path;
+ try {
+ path = require.resolve(m);
+ } catch (err) {
+ assert.ok(err.toString().startsWith('Error: Cannot find module '));
+ continue;
+ }
+ assert.notStrictEqual(path, m);
+}
From e93767fc040cef64cb14c5fd6c6503cefb904144 Mon Sep 17 00:00:00 2001
From: Benjamin Zaslavsky <benjamin.zaslavsky@gmail.com>
Date: Thu, 11 Jan 2018 15:02:08 +0100
Subject: [PATCH 2/2] test: removed deprecation tests
Fixes: https://github.com/nodejs/node/issues/17148
---
.../parallel/test-require-deps-deprecation.js | 24 -------------------
1 file changed, 24 deletions(-)
diff --git a/test/parallel/test-require-deps-deprecation.js b/test/parallel/test-require-deps-deprecation.js
index b4af723b2ee8e6..5e22b573c80ade 100644
--- a/test/parallel/test-require-deps-deprecation.js
+++ b/test/parallel/test-require-deps-deprecation.js
@@ -5,22 +5,6 @@ const assert = require('assert');
// The v8 modules when imported leak globals. Disable global check.
common.globalCheck = false;
-const deprecatedModules = [
- 'node-inspect/lib/_inspect',
- 'node-inspect/lib/internal/inspect_client',
- 'node-inspect/lib/internal/inspect_repl',
- 'v8/tools/SourceMap',
- 'v8/tools/codemap',
- 'v8/tools/consarray',
- 'v8/tools/csvparser',
- 'v8/tools/logreader',
- 'v8/tools/profile',
- 'v8/tools/profile_view',
- 'v8/tools/splaytree',
- 'v8/tools/tickprocessor',
- 'v8/tools/tickprocessor-driver'
-];
-
// Newly added deps that do not have a deprecation wrapper around it would
// throw an error, but no warning would be emitted.
const deps = [
@@ -28,14 +12,6 @@ const deps = [
'acorn/dist/walk'
];
-const throwingModules = () => {
- for (const m of deprecatedModules) {
- require(m);
- }
-};
-
-assert.throws(throwingModules, ReferenceError);
-
// 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.