From c268c8dfce325010f84dd4097ba8696c5f3a47c1 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Fri, 12 Aug 2016 10:49:27 +0200 Subject: [PATCH] set child process debug port to an available port - fixes #342 (#874) --- api.js | 302 ++++++++++++++++++++++---------------- lib/fork.js | 5 +- package.json | 1 + test/api.js | 46 ++++++ test/fixture/debug-arg.js | 5 + 5 files changed, 231 insertions(+), 128 deletions(-) create mode 100644 test/fixture/debug-arg.js diff --git a/api.js b/api.js index 794359b71..f19a51803 100644 --- a/api.js +++ b/api.js @@ -12,6 +12,7 @@ var debounce = require('lodash.debounce'); var ms = require('ms'); var AvaFiles = require('ava-files'); var autoBind = require('auto-bind'); +var getPort = require('get-port'); var AvaError = require('./lib/ava-error'); var fork = require('./lib/fork'); var CachingPrecompiler = require('./lib/caching-precompiler'); @@ -45,7 +46,7 @@ function Api(options) { util.inherits(Api, EventEmitter); module.exports = Api; -Api.prototype._runFile = function (file, runStatus) { +Api.prototype._runFile = function (file, runStatus, execArgv) { var hash = this.precompiler.precompileFile(file); var precompiled = {}; precompiled[file] = hash; @@ -54,7 +55,7 @@ Api.prototype._runFile = function (file, runStatus) { precompiled: precompiled }); - var emitter = fork(file, options); + var emitter = fork(file, options, execArgv); runStatus.observeFork(emitter); @@ -132,6 +133,49 @@ Api.prototype._run = function (files, _options) { return overwatch; }; +Api.prototype.computeForkExecArgs = function (files) { + var execArgv = this.options.testOnlyExecArgv || process.execArgv; + var debugArgIndex = -1; + + // --debug-brk is used in addition to --inspect to break on first line and wait + execArgv.some(function (arg, index) { + if (arg === '--inspect' || arg.indexOf('--inspect=') === 0) { + debugArgIndex = index; + return true; + } + return false; + }); + + var isInspect = debugArgIndex !== -1; + if (!isInspect) { + execArgv.some(function (arg, index) { + if (arg === '--debug' || arg === '--debug-brk' || arg.indexOf('--debug-brk=') === 0 || arg.indexOf('--debug=') === 0) { + debugArgIndex = index; + return true; + } + return false; + }); + } + + if (debugArgIndex === -1) { + return Promise.resolve([]); + } + + return Promise.map(files, getPort) + .then(function (ports) { + return ports.map(function (port) { + var forkExecArgv = execArgv.slice(); + var flagName = isInspect ? '--inspect' : '--debug'; + var oldValue = forkExecArgv[debugArgIndex]; + if (oldValue.indexOf('brk') > 0) { + flagName += '-brk'; + } + forkExecArgv[debugArgIndex] = flagName + '=' + port; + return forkExecArgv; + }); + }); +}; + Api.prototype._runNoPool = function (files, runStatus) { var self = this; var tests = new Array(self.fileCount); @@ -143,94 +187,97 @@ Api.prototype._runNoPool = function (files, runStatus) { }); }); - return new Promise(function (resolve) { - function run() { - if (self.options.match.length > 0 && !runStatus.hasExclusive) { - runStatus.handleExceptions({ - exception: new AvaError('Couldn\'t find any matching tests'), - file: undefined - }); - - resolve([]); - return; - } + return self.computeForkExecArgs(files) + .then(function (execArgvList) { + return new Promise(function (resolve) { + function run() { + if (self.options.match.length > 0 && !runStatus.hasExclusive) { + runStatus.handleExceptions({ + exception: new AvaError('Couldn\'t find any matching tests'), + file: undefined + }); + + resolve([]); + return; + } - var method = self.options.serial ? 'mapSeries' : 'map'; - var options = { - runOnlyExclusive: runStatus.hasExclusive - }; - - resolve(Promise[method](files, function (file, index) { - return tests[index].run(options).catch(function (err) { - // The test failed catastrophically. Flag it up as an - // exception, then return an empty result. Other tests may - // continue to run. - runStatus.handleExceptions({ - exception: err, - file: path.relative('.', file) - }); + var method = self.options.serial ? 'mapSeries' : 'map'; + var options = { + runOnlyExclusive: runStatus.hasExclusive + }; - return getBlankResults(); - }); - })); - } + resolve(Promise[method](files, function (file, index) { + return tests[index].run(options).catch(function (err) { + // The test failed catastrophically. Flag it up as an + // exception, then return an empty result. Other tests may + // continue to run. + runStatus.handleExceptions({ + exception: err, + file: path.relative('.', file) + }); + + return getBlankResults(); + }); + })); + } - // receive test count from all files and then run the tests - var unreportedFiles = self.fileCount; - var bailed = false; + // receive test count from all files and then run the tests + var unreportedFiles = self.fileCount; + var bailed = false; - files.every(function (file, index) { - var tried = false; + files.every(function (file, index) { + var tried = false; - function tryRun() { - if (!tried && !bailed) { - tried = true; - unreportedFiles--; + function tryRun() { + if (!tried && !bailed) { + tried = true; + unreportedFiles--; - if (unreportedFiles === 0) { - run(); + if (unreportedFiles === 0) { + run(); + } + } } - } - } - try { - var test = tests[index] = self._runFile(file, runStatus); + try { + var test = tests[index] = self._runFile(file, runStatus, execArgvList[index]); - test.on('stats', tryRun); - test.catch(tryRun); + test.on('stats', tryRun); + test.catch(tryRun); - return true; - } catch (err) { - bailed = true; + return true; + } catch (err) { + bailed = true; - runStatus.handleExceptions({ - exception: err, - file: path.relative('.', file) - }); + runStatus.handleExceptions({ + exception: err, + file: path.relative('.', file) + }); - resolve([]); + resolve([]); - return false; - } - }); - }).then(function (results) { - if (results.length === 0) { - // No tests ran, make sure to tear down the child processes. - tests.forEach(function (test) { - test.send('teardown'); - }); - } + return false; + } + }); + }).then(function (results) { + if (results.length === 0) { + // No tests ran, make sure to tear down the child processes. + tests.forEach(function (test) { + test.send('teardown'); + }); + } - return results; - }).then(function (results) { - // cancel debounced _onTimeout() from firing - if (self.options.timeout) { - runStatus._restartTimer.cancel(); - } + return results; + }).then(function (results) { + // cancel debounced _onTimeout() from firing + if (self.options.timeout) { + runStatus._restartTimer.cancel(); + } - runStatus.processResults(results); - return runStatus; - }); + runStatus.processResults(results); + return runStatus; + }); + }); }; function getBlankResults() { @@ -258,57 +305,60 @@ Api.prototype._runLimitedPool = function (files, runStatus, concurrency) { }); }); - return Promise.map(files, function (file) { - var handleException = function (err) { - runStatus.handleExceptions({ - exception: err, - file: path.relative('.', file) - }); - }; - - try { - var test = tests[file] = self._runFile(file, runStatus); - - return new Promise(function (resolve, reject) { - var runner = function () { - var options = { - // If we're looking for matches, run every single test process in exclusive-only mode - runOnlyExclusive: self.options.match.length > 0 + return self.computeForkExecArgs(files) + .then(function (execArgvList) { + return Promise.map(files, function (file, index) { + var handleException = function (err) { + runStatus.handleExceptions({ + exception: err, + file: path.relative('.', file) + }); }; - test.run(options) - .then(resolve) - .catch(reject); - }; - - test.on('stats', runner); - test.on('exit', function () { - delete tests[file]; - }); - test.catch(runner); - }).catch(handleException); - } catch (err) { - handleException(err); - } - }, {concurrency: concurrency}) - .then(function (results) { - // Filter out undefined results (usually result of caught exceptions) - results = results.filter(Boolean); - - // cancel debounced _onTimeout() from firing - if (self.options.timeout) { - runStatus._restartTimer.cancel(); - } - if (self.options.match.length > 0 && !runStatus.hasExclusive) { - // Ensure results are empty - results = []; - runStatus.handleExceptions({ - exception: new AvaError('Couldn\'t find any matching tests'), - file: undefined - }); - } - - runStatus.processResults(results); - return runStatus; - }); + try { + var test = tests[file] = self._runFile(file, runStatus, execArgvList[index]); + + return new Promise(function (resolve, reject) { + var runner = function () { + var options = { + // If we're looking for matches, run every single test process in exclusive-only mode + runOnlyExclusive: self.options.match.length > 0 + }; + test.run(options) + .then(resolve) + .catch(reject); + }; + + test.on('stats', runner); + test.on('exit', function () { + delete tests[file]; + }); + test.catch(runner); + }).catch(handleException); + } catch (err) { + handleException(err); + } + }, {concurrency: concurrency}) + .then(function (results) { + // Filter out undefined results (usually result of caught exceptions) + results = results.filter(Boolean); + + // cancel debounced _onTimeout() from firing + if (self.options.timeout) { + runStatus._restartTimer.cancel(); + } + + if (self.options.match.length > 0 && !runStatus.hasExclusive) { + // Ensure results are empty + results = []; + runStatus.handleExceptions({ + exception: new AvaError('Couldn\'t find any matching tests'), + file: undefined + }); + } + + runStatus.processResults(results); + return runStatus; + }); + }); }; diff --git a/lib/fork.js b/lib/fork.js index 52af036b4..7748c69d2 100644 --- a/lib/fork.js +++ b/lib/fork.js @@ -30,7 +30,7 @@ if (env.NODE_PATH) { .join(path.delimiter); } -module.exports = function (file, opts) { +module.exports = function (file, opts, execArgv) { opts = objectAssign({ file: file, baseDir: process.cwd(), @@ -43,7 +43,8 @@ module.exports = function (file, opts) { var ps = childProcess.fork(path.join(__dirname, 'test-worker.js'), [JSON.stringify(opts)], { cwd: path.dirname(file), silent: true, - env: env + env: env, + execArgv: execArgv || process.execArgv }); var relFile = path.relative('.', file); diff --git a/package.json b/package.json index 74cf11a32..dffb75cb2 100644 --- a/package.json +++ b/package.json @@ -119,6 +119,7 @@ "figures": "^1.4.0", "find-cache-dir": "^0.1.1", "fn-name": "^2.0.0", + "get-port": "^2.1.0", "has-flag": "^2.0.0", "ignore-by-default": "^1.0.0", "is-ci": "^1.0.7", diff --git a/test/api.js b/test/api.js index d084a8350..1062c3f1e 100644 --- a/test/api.js +++ b/test/api.js @@ -1022,3 +1022,49 @@ function generateTests(prefix, apiCreator) { ]); }); } + +function generatePassDebugTests(execArgv, expectedInspectIndex) { + test('pass ' + execArgv.join(' ') + ' to fork', function (t) { + t.plan(expectedInspectIndex === -1 ? 3 : 2); + + var api = new Api({testOnlyExecArgv: execArgv}); + return api.computeForkExecArgs(['foo.js']) + .then(function (result) { + t.true(result.length === 1); + if (expectedInspectIndex === -1) { + t.true(result[0].length === 1); + t.true(/--debug=\d+/.test(result[0][0])); + } else { + t.true(/--inspect=\d+/.test(result[0][expectedInspectIndex])); + } + }); + }); +} + +function generatePassDebugIntegrationTests(execArgv) { + test('pass ' + execArgv.join(' ') + ' to fork', function (t) { + t.plan(1); + + var api = new Api({testOnlyExecArgv: execArgv}); + return api.run([path.join(__dirname, 'fixture/debug-arg.js')]) + .then(function (result) { + t.is(result.passCount, 1); + }); + }); +} + +generatePassDebugTests(['--debug=0'], -1); +generatePassDebugTests(['--debug'], -1); + +generatePassDebugTests(['--inspect=0'], 0); +generatePassDebugTests(['--inspect'], 0); + +generatePassDebugTests(['--inspect=0', '--debug-brk'], 0); +generatePassDebugTests(['--inspect', '--debug-brk'], 0); + +generatePassDebugTests(['--debug-brk', '--inspect=0'], 1); +generatePassDebugTests(['--debug-brk', '--inspect'], 1); + +// --inspect cannot be tested because released node doesn't support it +generatePassDebugIntegrationTests(['--debug=0']); +generatePassDebugIntegrationTests(['--debug']); diff --git a/test/fixture/debug-arg.js b/test/fixture/debug-arg.js new file mode 100644 index 000000000..cbee6f5ab --- /dev/null +++ b/test/fixture/debug-arg.js @@ -0,0 +1,5 @@ +import test from '../../'; + +test(t => { + t.true(process.execArgv[0].indexOf('--debug') === 0); +});