From 5d8519a87088f2858491fe2218caf4dae9b4c41c Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 1 Mar 2017 08:16:48 +0100 Subject: [PATCH 1/8] test: add checkCrypto to tls-connect.js Currently when node is build --without-ssl and the test are run, the test that use the tls-connect fixture will throw an Error saying that Node was not built with ssl support. The error is thrown when the tls module is required causing the test that use it to fail unstead of being skipped when there is no ssl support to test. This commit calls checkCrypto before requiring the tls module so that an error is not thrown and the test skipped instead. --- test/fixtures/tls-connect.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/fixtures/tls-connect.js b/test/fixtures/tls-connect.js index 72f83736bf370e..6cb0f0f9494d35 100644 --- a/test/fixtures/tls-connect.js +++ b/test/fixtures/tls-connect.js @@ -6,6 +6,9 @@ const common = require('../common'); const fs = require('fs'); const join = require('path').join; +// Check if Node was compiled --without-ssl and if so exit early +// as the require of tls will otherwise throw an Error. +checkCrypto(); const tls = require('tls'); const util = require('util'); @@ -19,6 +22,7 @@ function checkCrypto() { return exports; } + exports.assert = require('assert'); exports.debug = util.debuglog('test'); exports.tls = tls; From f6a00270c4217d376705e7ba0fcbd5c4a5c1e0e2 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 1 Mar 2017 13:35:09 +0100 Subject: [PATCH 2/8] test: make test-parallel pass witout-ssl This commit enables the test-parallel target to pass when the build is configured --without-ssl To configure and run: $ ./configure --without-ssl --debug && make -j8 test-parallel The update to test/testspy/__init__.py is for test-cluster-inspector-debug-port.js which passes '--inspect={PORT} flag to the node process which causes the process to exit as the inspector is not enabled when ssl is disabled. The suggested solution here is to simply remove the flag when v8_enable_inspector is 0 and then have a check in the test like the others test in this commit. --- test/common.js | 7 +++++++ test/inspector/test-inspector.js | 3 ++- .../test-cluster-inspector-debug-port.js | 1 + test/sequential/test-debugger-debug-brk.js | 1 + test/testpy/__init__.py | 18 +++++++++++++++++- 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/test/common.js b/test/common.js index 6c7ea387e38904..9f73e2e8820ec3 100644 --- a/test/common.js +++ b/test/common.js @@ -638,3 +638,10 @@ exports.expectsError = function expectsError({code, type, message}) { return true; }; }; + +exports.skipIfInspectorDisabled = function skipIfInspectorDisabled() { + if (!exports.hasCrypto) { + exports.skip('missing ssl support so inspector is disabled'); + process.exit(0); + } +}; diff --git a/test/inspector/test-inspector.js b/test/inspector/test-inspector.js index a1c69cb6fbb2fe..5cad934e7a51f9 100644 --- a/test/inspector/test-inspector.js +++ b/test/inspector/test-inspector.js @@ -1,5 +1,6 @@ 'use strict'; -require('../common'); +const common = require('../common'); +common.skipIfInspectorDisabled(); const assert = require('assert'); const helper = require('./inspector-helper.js'); diff --git a/test/parallel/test-cluster-inspector-debug-port.js b/test/parallel/test-cluster-inspector-debug-port.js index f2f8db9b5d5890..f0e0f58a8655c9 100644 --- a/test/parallel/test-cluster-inspector-debug-port.js +++ b/test/parallel/test-cluster-inspector-debug-port.js @@ -1,6 +1,7 @@ 'use strict'; // Flags: --inspect={PORT} const common = require('../common'); +common.skipIfInspectorDisabled(); const assert = require('assert'); const cluster = require('cluster'); const debuggerPort = common.PORT; diff --git a/test/sequential/test-debugger-debug-brk.js b/test/sequential/test-debugger-debug-brk.js index f04af4544f88a2..f5a69b91d6b536 100644 --- a/test/sequential/test-debugger-debug-brk.js +++ b/test/sequential/test-debugger-debug-brk.js @@ -1,5 +1,6 @@ 'use strict'; const common = require('../common'); +common.skipIfInspectorDisabled(); const assert = require('assert'); const spawn = require('child_process').spawn; diff --git a/test/testpy/__init__.py b/test/testpy/__init__.py index 367346f6e1509d..84a802c41b4314 100644 --- a/test/testpy/__init__.py +++ b/test/testpy/__init__.py @@ -29,6 +29,7 @@ import os from os.path import join, dirname, exists import re +import ast FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)") @@ -64,7 +65,22 @@ def GetCommand(self): # PORT should match the definition in test/common.js. env = { 'PORT': int(os.getenv('NODE_COMMON_PORT', '12346')) } env['PORT'] += self.thread_id * 100 - result += flags_match.group(1).strip().format(**env).split() + flag = flags_match.group(1).strip().format(**env).split() + # The following block reads config.gypi to extract the v8_enable_inspector + # value. This is done to check if the inspector is disabled in which case + # the '--inspect' flag cannot be passed to the node process as it will + # that will cause node to exit and report the test as failed. The use case + # is currently when Node is configured --without-ssl and the tests should + # still be runnable but skip any tests that require ssl (which includes the + # inspector related tests). + with open('config.gypi', 'r') as f: + s = f.read() + config_gypi = ast.literal_eval(s) + v8_disable_inspector = config_gypi['variables']['v8_enable_inspector'] == 0 + if flag[0].startswith('--inspect') and v8_disable_inspector: + print('Skipping as inspector is disabled') + else: + result += flag files_match = FILES_PATTERN.search(source); additional_files = [] if files_match: From c9c6c2beb6f1dfa89a8c71d35cc6b0daf2da5721 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 1 Mar 2017 14:30:36 +0100 Subject: [PATCH 3/8] test: make addons/openssl-binding pass without-ssl Add checks to the compilation of this addon and also a check when the test is run to see if ssl is enabled or not. If it is not enabled then skip this test. --- test/addons/openssl-binding/binding.gyp | 8 ++++++-- test/addons/openssl-binding/test.js | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/addons/openssl-binding/binding.gyp b/test/addons/openssl-binding/binding.gyp index 672f84bb860a9d..bafde41348ce3a 100644 --- a/test/addons/openssl-binding/binding.gyp +++ b/test/addons/openssl-binding/binding.gyp @@ -2,8 +2,12 @@ 'targets': [ { 'target_name': 'binding', - 'sources': ['binding.cc'], - 'include_dirs': ['../../../deps/openssl/openssl/include'], + 'conditions': [ + ['node_use_openssl=="true"', { + 'sources': ['binding.cc'], + 'include_dirs': ['../../../deps/openssl/openssl/include'], + }] + ] }, ] } diff --git a/test/addons/openssl-binding/test.js b/test/addons/openssl-binding/test.js index a146ffe5c68f1b..452f59f752f5e5 100644 --- a/test/addons/openssl-binding/test.js +++ b/test/addons/openssl-binding/test.js @@ -1,6 +1,10 @@ 'use strict'; const common = require('../../common'); +if (!common.hasCrypto) { + common.skip('missing crypto'); + process.exit(0); +} const assert = require('assert'); const binding = require(`./build/${common.buildType}/binding`); const bytes = new Uint8Array(1024); From a7b0c2ef0c8a5e3d0bff9e268f6dba718d5a59d8 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 2 Mar 2017 14:11:14 +0100 Subject: [PATCH 4/8] remove empty line added by mistake --- test/fixtures/tls-connect.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/fixtures/tls-connect.js b/test/fixtures/tls-connect.js index 6cb0f0f9494d35..a961f709915d18 100644 --- a/test/fixtures/tls-connect.js +++ b/test/fixtures/tls-connect.js @@ -22,7 +22,6 @@ function checkCrypto() { return exports; } - exports.assert = require('assert'); exports.debug = util.debuglog('test'); exports.tls = tls; From 4084f4b448dd12c5808aa59348b00cd951edc8a4 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 2 Mar 2017 14:22:35 +0100 Subject: [PATCH 5/8] remove checkCrypto function --- test/fixtures/tls-connect.js | 15 ++++++--------- test/parallel/test-tls-socket-default-options.js | 5 ----- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/test/fixtures/tls-connect.js b/test/fixtures/tls-connect.js index a961f709915d18..41ba46a7593e44 100644 --- a/test/fixtures/tls-connect.js +++ b/test/fixtures/tls-connect.js @@ -8,19 +8,16 @@ const fs = require('fs'); const join = require('path').join; // Check if Node was compiled --without-ssl and if so exit early // as the require of tls will otherwise throw an Error. -checkCrypto(); +if (!common.hasCrypto) { + common.skip('missing crypto'); + process.exit(0); +} const tls = require('tls'); const util = require('util'); -module.exports = exports = checkCrypto; - -function checkCrypto() { - if (!common.hasCrypto) { - common.skip('missing crypto'); - process.exit(0); - } +module.exports = exports = function() { return exports; -} +}; exports.assert = require('assert'); exports.debug = util.debuglog('test'); diff --git a/test/parallel/test-tls-socket-default-options.js b/test/parallel/test-tls-socket-default-options.js index 24b7a5d34ec0fb..edf16a6bffc78d 100644 --- a/test/parallel/test-tls-socket-default-options.js +++ b/test/parallel/test-tls-socket-default-options.js @@ -9,11 +9,6 @@ const { connect, keys, tls } = require(join(common.fixturesDir, 'tls-connect')); -if (!common.hasCrypto) { - common.skip('missing crypto'); - return; -} - test(undefined, (err) => { assert.strictEqual(err.message, 'unable to verify the first certificate'); }); From f6d7cb000a4afc536ca3f1e008c69ce52bad7bb1 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 2 Mar 2017 14:24:46 +0100 Subject: [PATCH 6/8] correct comment that does not make sense --- test/testpy/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/testpy/__init__.py b/test/testpy/__init__.py index 84a802c41b4314..2114263daece2d 100644 --- a/test/testpy/__init__.py +++ b/test/testpy/__init__.py @@ -69,7 +69,7 @@ def GetCommand(self): # The following block reads config.gypi to extract the v8_enable_inspector # value. This is done to check if the inspector is disabled in which case # the '--inspect' flag cannot be passed to the node process as it will - # that will cause node to exit and report the test as failed. The use case + # cause node to exit and report the test as failed. The use case # is currently when Node is configured --without-ssl and the tests should # still be runnable but skip any tests that require ssl (which includes the # inspector related tests). From b440006cf8137ef21d0fbbd2e20d27fdd5440559 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 2 Mar 2017 15:31:16 +0100 Subject: [PATCH 7/8] move reading of v8_enable_inspector to tools/test.py --- test/testpy/__init__.py | 12 ++++-------- tools/test.py | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/test/testpy/__init__.py b/test/testpy/__init__.py index 2114263daece2d..d7ec88992e2366 100644 --- a/test/testpy/__init__.py +++ b/test/testpy/__init__.py @@ -73,14 +73,10 @@ def GetCommand(self): # is currently when Node is configured --without-ssl and the tests should # still be runnable but skip any tests that require ssl (which includes the # inspector related tests). - with open('config.gypi', 'r') as f: - s = f.read() - config_gypi = ast.literal_eval(s) - v8_disable_inspector = config_gypi['variables']['v8_enable_inspector'] == 0 - if flag[0].startswith('--inspect') and v8_disable_inspector: - print('Skipping as inspector is disabled') - else: - result += flag + if flag[0].startswith('--inspect') and self.context.v8_enable_inspector == 0: + print('Skipping as inspector is disabled') + else: + result += flag files_match = FILES_PATTERN.search(source); additional_files = [] if files_match: diff --git a/tools/test.py b/tools/test.py index d6715937b60da3..deeb7aeffe2e02 100755 --- a/tools/test.py +++ b/tools/test.py @@ -43,6 +43,7 @@ import multiprocessing import errno import copy +import ast from os.path import join, dirname, abspath, basename, isdir, exists from datetime import datetime @@ -867,7 +868,8 @@ class Context(object): def __init__(self, workspace, buildspace, verbose, vm, args, expect_fail, timeout, processor, suppress_dialogs, - store_unexpected_output, repeat, abort_on_timeout): + store_unexpected_output, repeat, abort_on_timeout, + v8_enable_inspector): self.workspace = workspace self.buildspace = buildspace self.verbose = verbose @@ -880,6 +882,7 @@ def __init__(self, workspace, buildspace, verbose, vm, args, expect_fail, self.store_unexpected_output = store_unexpected_output self.repeat = repeat self.abort_on_timeout = abort_on_timeout + self.v8_enable_inspector = v8_enable_inspector def GetVm(self, arch, mode): if arch == 'none': @@ -912,6 +915,19 @@ def RunTestCases(cases_to_run, progress, tasks, flaky_tests_mode): progress = PROGRESS_INDICATORS[progress](cases_to_run, flaky_tests_mode) return progress.Run(tasks) +def GetV8InspectorEnabledFlag(): + # The following block reads config.gypi to extract the v8_enable_inspector + # value. This is done to check if the inspector is disabled in which case + # the '--inspect' flag cannot be passed to the node process as it will + # cause node to exit and report the test as failed. The use case + # is currently when Node is configured --without-ssl and the tests should + # still be runnable but skip any tests that require ssl (which includes the + # inspector related tests). + with open('config.gypi', 'r') as f: + s = f.read() + config_gypi = ast.literal_eval(s) + return config_gypi['variables']['v8_enable_inspector'] + # ------------------------------------------- # --- T e s t C o n f i g u r a t i o n --- @@ -1587,7 +1603,8 @@ def Main(): options.suppress_dialogs, options.store_unexpected_output, options.repeat, - options.abort_on_timeout) + options.abort_on_timeout, + GetV8InspectorEnabledFlag()) # Get status for tests sections = [ ] From 3addfd0f5ef9cdc7ce2e5a59c18651966771f949 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 3 Mar 2017 09:24:54 +0100 Subject: [PATCH 8/8] remove function call when requiring tls-connect --- test/fixtures/tls-connect.js | 4 ---- test/parallel/test-tls-addca.js | 2 +- test/parallel/test-tls-ca-concat.js | 2 +- test/parallel/test-tls-cert-chains-concat.js | 2 +- test/parallel/test-tls-cert-chains-in-ca.js | 2 +- test/parallel/test-tls-connect-secure-context.js | 2 +- test/parallel/test-tls-peer-certificate.js | 2 +- 7 files changed, 6 insertions(+), 10 deletions(-) diff --git a/test/fixtures/tls-connect.js b/test/fixtures/tls-connect.js index 41ba46a7593e44..a434a0316d6fb5 100644 --- a/test/fixtures/tls-connect.js +++ b/test/fixtures/tls-connect.js @@ -15,10 +15,6 @@ if (!common.hasCrypto) { const tls = require('tls'); const util = require('util'); -module.exports = exports = function() { - return exports; -}; - exports.assert = require('assert'); exports.debug = util.debuglog('test'); exports.tls = tls; diff --git a/test/parallel/test-tls-addca.js b/test/parallel/test-tls-addca.js index 7a6f9a77516e22..f3f5e5b8dea107 100644 --- a/test/parallel/test-tls-addca.js +++ b/test/parallel/test-tls-addca.js @@ -8,7 +8,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, keys, tls -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); const contextWithoutCert = tls.createSecureContext({}); const contextWithCert = tls.createSecureContext({}); diff --git a/test/parallel/test-tls-ca-concat.js b/test/parallel/test-tls-ca-concat.js index 65c837bed9dc37..0c1908049be830 100644 --- a/test/parallel/test-tls-ca-concat.js +++ b/test/parallel/test-tls-ca-concat.js @@ -7,7 +7,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, keys -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); connect({ client: { diff --git a/test/parallel/test-tls-cert-chains-concat.js b/test/parallel/test-tls-cert-chains-concat.js index d53edef89842b9..a90ed67997ee22 100644 --- a/test/parallel/test-tls-cert-chains-concat.js +++ b/test/parallel/test-tls-cert-chains-concat.js @@ -7,7 +7,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, debug, keys -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); // agent6-cert.pem includes cert for agent6 and ca3 connect({ diff --git a/test/parallel/test-tls-cert-chains-in-ca.js b/test/parallel/test-tls-cert-chains-in-ca.js index 3b4e78919fb8b4..03fae36cb74a57 100644 --- a/test/parallel/test-tls-cert-chains-in-ca.js +++ b/test/parallel/test-tls-cert-chains-in-ca.js @@ -7,7 +7,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, debug, keys -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); // agent6-cert.pem includes cert for agent6 and ca3, split it apart and diff --git a/test/parallel/test-tls-connect-secure-context.js b/test/parallel/test-tls-connect-secure-context.js index d1552a62169207..ef063c2ed3a41d 100644 --- a/test/parallel/test-tls-connect-secure-context.js +++ b/test/parallel/test-tls-connect-secure-context.js @@ -6,7 +6,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, keys, tls -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); connect({ client: { diff --git a/test/parallel/test-tls-peer-certificate.js b/test/parallel/test-tls-peer-certificate.js index eb5be6dcb2241b..8c56e72af14632 100644 --- a/test/parallel/test-tls-peer-certificate.js +++ b/test/parallel/test-tls-peer-certificate.js @@ -6,7 +6,7 @@ const common = require('../common'); const join = require('path').join; const { assert, connect, debug, keys -} = require(join(common.fixturesDir, 'tls-connect'))(); +} = require(join(common.fixturesDir, 'tls-connect')); connect({ client: {rejectUnauthorized: false},