From 9d663eb57058991c5b3558cae0bfe7ae62a3b6ac Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Sun, 19 Feb 2017 16:27:42 -0500 Subject: [PATCH] Leverage command return value instead of process.exit. Works around an upstream issue in ember-cli 2.13 canary versions, and is quite a bit easier to test. Also tweaked SIGINT handling to queue up a cancel and avoid additional work. This is because in ember-cli 2.9+ we use `capture-exit` so that `process.exit()` does not immediately schedule an exit any longer (so the semantics that we intended with the prior SIGINT logic were generally broken already). --- lib/tasks/try-each.js | 37 +++++------ test/tasks/try-each-test.js | 127 ++++++++++-------------------------- 2 files changed, 54 insertions(+), 110 deletions(-) diff --git a/lib/tasks/try-each.js b/lib/tasks/try-each.js index 8ad9bc12..8c8cafbf 100644 --- a/lib/tasks/try-each.js +++ b/lib/tasks/try-each.js @@ -19,18 +19,11 @@ module.exports = CoreObject.extend({ dependencyManagerAdapters: dependencyManagerAdapters }); - var shutdownCount = 0; + task._canceling = false; task._on('SIGINT', function() { - if (shutdownCount === 0) { - shutdownCount++; - task.ui.writeLine('\nGracefully shutting down from SIGINT (Ctrl-C)'); - return task.ScenarioManager.cleanup().then(function() { - task._exit(); - }); - } else { - task.ui.writeLine('\nOk, but it\'s going to be a mess'); - task._exit(); - } + task._canceling = true; + task.ui.writeLine('\nGracefully shutting down from SIGINT (Ctrl-C)'); + return task.ScenarioManager.cleanup() }); return task.ScenarioManager.setup().then(function() { @@ -40,7 +33,7 @@ module.exports = CoreObject.extend({ return task._optionallyCleanup(options).then(function() { debug('Output results'); task._printResults(results); - task._exitAsAppropriate(results); + return task._exitAsAppropriate(results); }); }).catch(function(err) { task.ui.writeLine(chalk.red('Error!')); @@ -48,14 +41,19 @@ module.exports = CoreObject.extend({ task.ui.writeLine(chalk.red(err)); task.ui.writeLine(chalk.red(err.stack)); } - task._exit(1); + return 1; // Signifies exit code }); }, _runCommandForThisScenario: function(scenario) { var task = this; + + if (task._canceling) { return; } + return task.ScenarioManager.changeTo(scenario) .then(function(scenarioDependencyState) { + if (task._canceling) { return; } + process.env.EMBER_TRY_CURRENT_SCENARIO = scenario.name; task._writeHeader('Scenario: ' + scenario.name); var command = task._determineCommandFor(scenario); @@ -69,6 +67,8 @@ module.exports = CoreObject.extend({ debug('With:\n', runResults); return task._runCommand({commandArgs: command, commandOptions: task._commandOptions()}).then(function(result) { + if (task._canceling) { return; } + runResults.result = result; task._writeFooter('Result: ' + result); return RSVP.resolve(runResults); @@ -130,7 +130,8 @@ module.exports = CoreObject.extend({ var outcomes = results.map(function(result) { return result.result || result.allowedToFail; }); - this._exitBasedOnCondition(outcomes.indexOf(false) > -1); + + return this._exitBasedOnCondition(outcomes.indexOf(false) > -1); }, _optionallyCleanup: function(options) { @@ -151,11 +152,9 @@ module.exports = CoreObject.extend({ }, _exitBasedOnCondition: function(condition) { - if (condition) { - this._exit(1); - } else { - this._exit(0); - } + var exitCode = condition ? 1 : 0; + debug('Exit %s', exitCode); + return exitCode; }, _exit: function(code) { diff --git a/test/tasks/try-each-test.js b/test/tasks/try-each-test.js index 2db25086..64a399c2 100644 --- a/test/tasks/try-each-test.js +++ b/test/tasks/try-each-test.js @@ -139,21 +139,17 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, project: {root: tmpdir}, config: legacyConfig, - _on: function() {}, - _exit: mockedExit + _on: function() {} }); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(legacyConfig.scenarios, {}).then(function() { + return tryEachTask.run(legacyConfig.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario default: SUCCESS'); expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); @@ -188,21 +184,17 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(1); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, project: {root: tmpdir}, config: legacyConfig, - _on: function() {}, - _exit: mockedExit + _on: function() {} }); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(legacyConfig.scenarios, {}).then(function() { + return tryEachTask.run(legacyConfig.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(1); expect(output).to.include('Scenario default: FAIL'); expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); @@ -233,23 +225,19 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, project: {root: tmpdir}, config: config, - _on: function() {}, - _exit: mockedExit + _on: function() {} }); writeJSONFile('package.json', fixturePackage); fs.mkdirSync('node_modules'); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); expect(output).to.include('Scenario with-bower-resolutions: SUCCESS'); @@ -281,23 +269,19 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(1); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, project: {root: tmpdir}, config: config, - _on: function() {}, - _exit: mockedExit + _on: function() {} }); writeJSONFile('package.json', fixturePackage); fs.mkdirSync('node_modules'); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(1); expect(output).to.include('Scenario first: FAIL'); expect(output).to.include('Scenario second: SUCCESS'); expect(output).to.include('Scenario with-bower-resolutions: SUCCESS'); @@ -340,10 +324,6 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, @@ -352,12 +332,12 @@ describe('tryEach', function() { commandArgs: ['ember', 'serve'], commandOptions: { timeout: { length: 20000, isSuccess: true }}, dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario first: SUCCESS'); expect(passedInOptions).to.equal(true, 'Should pass the options all the way down to run'); }).catch(function(err) { @@ -396,10 +376,6 @@ describe('tryEach', function() { var outputFn = function(log) { output.push(log); }; - var exitCode; - var mockedExit = function(code) { - exitCode = code; - }; var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ @@ -407,11 +383,10 @@ describe('tryEach', function() { project: {root: tmpdir}, config: config, dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { expect(output).to.include('Scenario first: FAIL (Allowed)'); expect(output).to.include('Scenario second: FAIL (Allowed)'); expect(output).to.include('2 scenarios failed (2 allowed)'); @@ -450,10 +425,6 @@ describe('tryEach', function() { var outputFn = function(log) { output.push(log); }; - var exitCode; - var mockedExit = function(code) { - exitCode = code; - }; var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ @@ -461,11 +432,10 @@ describe('tryEach', function() { project: {root: tmpdir}, config: config, dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { expect(output).to.include('Scenario first: FAIL'); expect(output).to.include('Scenario second: FAIL (Allowed)'); expect(output).to.include('2 scenarios failed (1 allowed)'); @@ -505,10 +475,6 @@ describe('tryEach', function() { var outputFn = function(log) { output.push(log); }; - var exitCode; - var mockedExit = function(code) { - exitCode = code; - }; var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ @@ -516,11 +482,10 @@ describe('tryEach', function() { project: {root: tmpdir}, config: config, dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); expect(output).to.include('All 2 scenarios succeeded'); @@ -566,10 +531,6 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, @@ -577,11 +538,11 @@ describe('tryEach', function() { config: config, commandArgs: [], dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); @@ -616,10 +577,6 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, @@ -627,11 +584,11 @@ describe('tryEach', function() { config: config, commandArgs: ['ember', 'serve'], dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario first: SUCCESS'); expect(ranPassedInCommand).to.equal(true, 'Should run the passed in command'); }).catch(function(err) { @@ -687,21 +644,18 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, project: {root: tmpdir}, config: config, dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); + expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); expect(output).to.include('Scenario different: SUCCESS'); @@ -732,10 +686,6 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var TryEachTask = require('../../lib/tasks/try-each'); var tryEachTask = new TryEachTask({ ui: {writeLine: outputFn}, @@ -743,11 +693,11 @@ describe('tryEach', function() { config: config, commandArgs: ['ember', 'help', '--json', 'true'], dependencyManagerAdapters: [new StubDependencyAdapter()], - _on: function() {}, - _exit: mockedExit + _on: function() {} }); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(output).to.include('Scenario first: SUCCESS', 'Passing scenario means options were passed along'); }).catch(function(err) { console.log(err); @@ -774,10 +724,6 @@ describe('tryEach', function() { output.push(log); }; - var mockedExit = function(code) { - expect(code).to.equal(0, 'exits 0 when all scenarios succeed'); - }; - var scenarios = []; var mockRunCommand = function() { var currentScenario = process.env.EMBER_TRY_CURRENT_SCENARIO; @@ -792,12 +738,12 @@ describe('tryEach', function() { config: config, dependencyManagerAdapters: [new StubDependencyAdapter()], _on: function() {}, - _exit: mockedExit, _runCommand: mockRunCommand }); writeJSONFile('bower.json', fixtureBower); - return tryEachTask.run(config.scenarios, {}).then(function() { + return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { + expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); expect(scenarios).to.eql(['first']); var currentScenarioIsUndefined = process.env.EMBER_TRY_CURRENT_SCENARIO === undefined; expect(currentScenarioIsUndefined).to.equal(true); @@ -807,5 +753,4 @@ describe('tryEach', function() { }); }); }); - });