Repository: ember-cli/ember-try
Pull request #114 (2017-02-19 21:40:56)

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).

Addresses the issue identified in https://github.com/ember-cli/ember-cli/issues/6779.

rwjblue: Before this PR (when using ember-cli#master) a failing test suite with `ember try:one` will report `0` exit code. After this PR (when using ember-cli#master) a failing test suite with `ember try:one` will report `1` exit code.

You can test using the demo repo I created for https://github.com/ember-cli/ember-cli/issues/6779 (https://github.com/rwjblue/exit-code-incorrect-demo).

Patch:
diff --git a/lib/tasks/try-each.js b/lib/tasks/try-each.js
index 8ad9bc1..8c8cafb 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) {


Test patch:
diff --git a/test/tasks/try-each-test.js b/test/tasks/try-each-test.js
index 2db2508..64a399c 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() {
       });
     });
   });
-
 });