[11.x] test-require-deps-deprecation fails on --without-ssl builds

@richardlau

It seems a deprecation warning is emitted twice for such builds while requireing 'node-inspect/lib/internal/inspect_client'. https://ci.nodejs.org/job/node-test-commit-linux-containered/11111/nodes=ubuntu1604_sharedlibs_withoutssl_x64/console

I would like to ignore that test for now as it does not seem critical to block the release. Any other opinions? @nodejs/releasers @nodejs/tsc

We have only just enabled the --without-ssl builds in the last week: nodejs/build#1574

The test in question doesn't exist in master (removed in #25138 which is semver-major). I kicked off https://ci.nodejs.org/job/node-test-commit-linux-containered/11117/ against v11.x (i.e. 11.10.1) and the test failed there so it's not a new regression in this proposed release so I'm okay with ignoring this for now but we do need to address it so the builds pass for the next 11.x release.

Originally posted by @richardlau in #26322 (comment)

So for the --without-ssl builds, parallel/test-require-deps-deprecation:

I was not able to reproduce the issue locally. I suggest we just remove the tests in v11. Any objections?

It does reproduce for me.

Regarding the --without-ssl issue, perhaps we can add a crypto check:

patch

$ git diff
diff --git a/test/parallel/test-require-deps-deprecation.js b/test/parallel/test-require-deps-deprecation.js
index 5bee24a5db..aba84b5af7 100644
--- a/test/parallel/test-require-deps-deprecation.js
+++ b/test/parallel/test-require-deps-deprecation.js
@@ -4,9 +4,6 @@ const common = require('../common');
 const assert = require('assert');

 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',
@@ -19,6 +16,12 @@ const deprecatedModules = [
   'v8/tools/tickprocessor-driver'
 ];

+if (common.hasCrypto) {
+  deprecatedModules.push('node-inspect/lib/_inspect',
+                         'node-inspect/lib/internal/inspect_client',
+                         'node-inspect/lib/internal/inspect_repl');
+}
+
 // Newly added deps that do not have a deprecation wrapper around it would
 // throw an error, but no warning would be emitted.
 const deps = [

For posterity, git bisect points to b280d90 being the first bad commit.

Prior to b280d90:

-bash-4.2$ ./node
> require('node-inspect/lib/_inspect')
Thrown:
Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto support
    at assertCrypto (internal/util.js:101:11)
    at crypto.js:31:1
    at NativeModule.compile (internal/bootstrap/loaders.js:341:5)
    at Function.NativeModule.require (internal/bootstrap/loaders.js:213:16)
    at NativeModule.requireForDeps (internal/bootstrap/loaders.js:228:23)
    at internal/deps/node-inspect/lib/internal/inspect_client.js:24:16
    at NativeModule.compile (internal/bootstrap/loaders.js:341:5)
    at Function.NativeModule.require (internal/bootstrap/loaders.js:213:16)
    at NativeModule.requireForDeps (internal/bootstrap/loaders.js:228:23)
> (node:3877) [DEP0084] DeprecationWarning: Requiring Node.js-bundled 'node-inspect/lib/_inspect' module is deprecated. Please install the necessary module locally.

> .exit

After we get an extra second deprecation warning which then causes the test to fail because the test is expecting a 1:1 mapping between the list of modules in it and warnings:

-bash-4.2$ ./node
> require('node-inspect/lib/_inspect')
Thrown:
Error [ERR_NO_CRYPTO]: Node.js is not compiled with OpenSSL crypto support
    at assertCrypto (internal/util.js:101:11)
    at crypto.js:31:1
    at NativeModule.compile (internal/bootstrap/loaders.js:317:5)
    at Function.NativeModule.require (internal/bootstrap/loaders.js:220:7)
    at NativeModule.requireWithFallbackInDeps (internal/bootstrap/loaders.js:239:23)
    at internal/deps/node-inspect/lib/internal/inspect_client.js:24:16
    at NativeModule.compile (internal/bootstrap/loaders.js:317:5)
    at NativeModule.require (internal/bootstrap/loaders.js:220:7)
> (node:5615) [DEP0084] DeprecationWarning: Requiring Node.js-bundled 'node-inspect/lib/_inspect' module is deprecated. Please install the necessary module locally.
(node:5615) [DEP0084] DeprecationWarning: Requiring Node.js-bundled 'node-inspect/lib/internal/inspect_client' module is deprecated. Please install the necessary module locally.
.exit
-bash-4.2$

node/test/parallel/test-require-deps-deprecation.js

Lines 6 to 20 in 3c277d7

    
           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' 
        
           ];

node/test/parallel/test-require-deps-deprecation.js

Lines 29 to 32 in 3c277d7

    
           common.expectWarning('DeprecationWarning', deprecatedModules.map((m) => { 
        
             return [`Requiring Node.js-bundled '${m}' module is deprecated. ` + 
        
                    'Please install the necessary module locally.', 'DEP0084']; 
        
           }));

Of the modules in the list only node-inspect/lib/_inspect appears to be affected. The test passes if that module is removed from the list.

diff --git a/test/parallel/test-require-deps-deprecation.js b/test/parallel/
index 5bee24a..e2c8e9c 100644
--- a/test/parallel/test-require-deps-deprecation.js
+++ b/test/parallel/test-require-deps-deprecation.js
@@ -4,7 +4,6 @@ const common = require('../common');
 const assert = require('assert');

 const deprecatedModules = [
-  'node-inspect/lib/_inspect',
   'node-inspect/lib/internal/inspect_client',
   'node-inspect/lib/internal/inspect_repl',
   'v8/tools/SourceMap',

test: fix test by removing node-inspect/lib/_inspect

Navigation Menu

Repository navigation

[11.x] test-require-deps-deprecation fails on `--without-ssl` builds #26480

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	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'
	];

	common.expectWarning('DeprecationWarning', deprecatedModules.map((m) => {
	return [`Requiring Node.js-bundled '${m}' module is deprecated. ` +
	'Please install the necessary module locally.', 'DEP0084'];
	}));

[11.x] test-require-deps-deprecation fails on --without-ssl builds #26480

Description

Activity

richardlau commented on Mar 6, 2019

BridgeAR commented on Mar 6, 2019

richardlau commented on Mar 6, 2019

danbev commented on Mar 7, 2019

richardlau commented on Mar 7, 2019

richardlau commented on Mar 7, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[11.x] test-require-deps-deprecation fails on `--without-ssl` builds #26480