Merge pull request #1205 from testem/fix-leaky-runners

[BUGFIX] Prevent leaked runners/sub processes.

bin/run-tests.js

@@ -1,39 +1,21 @@
 #!/usr/bin/env node
 'use strict';
 
-var spawn = require('child_process').spawn;
+var execa = require('execa');
+var command = 'npm';
 
-function run(command, args, cb) {
-
-  console.log('Running: ' + command + ' ' + args.join(' '));
-
-  var child = spawn(command, args, { stdio: 'inherit' });
-
-  child.on('error', function(err) {
-    cb(err);
-  });
-
-  child.on('exit', function(code) {
-    if (code === 0) {
-      return cb();
-    }
-
-    cb(code);
-  });
-}
-
-var testArgs;
+var args;
 if (process.env.BROWSER_TESTS) {
-  testArgs = ['run', 'browser-tests'];
+  args = ['run', 'browser-tests'];
 } else {
-  testArgs = ['run', 'testem-tests'];
+  args = ['run', 'testem-tests'];
 }
 
-run('npm', testArgs, function(err) {
-  if (err) {
-    console.error(err);
-    process.exit(1);
-  }
+console.log('Running: ' + command + ' ' + args.join(' '));
 
-  process.exit(0);
+execa(command, args, { stdio: 'inherit' }).then(function(result) {
+  process.exit(result.code);
+}).catch(function(err) {
+  console.error(err);
+  process.exit(1);
 });

examples/coverage_istanbul/testem.js

@@ -1,7 +1,6 @@
 var path  = require('path');
 var http  = require('http');
 var fs    = require('fs');
-var cp    = require('child_process');
 var shell = require('shelljs');
 var port  = 7358;
 var server;

lib/fileutils.js

@@ -1,33 +1,31 @@
 'use strict';
 
 var fs = require('fs');
-var childProcess = require('child_process');
+var execa = require('execa');
 var Bluebird = require('bluebird');
 var log = require('npmlog');
 
 var fsStatAsync = Bluebird.promisify(fs.stat);
 
 var isWin = require('./utils/is-win')();
 
-var fileExists = function(path) {
+exports.fileExists = function fileExists(path) {
   return fsStatAsync(path).then(function(stats) {
     return stats.isFile();
   }).catchReturn(false);
 };
-exports.fileExists = fileExists;
 
-var executableExists = function(exe, options) {
+exports.executableExists = function executableExists(exe, options) {
   var cmd = isWin ? 'where' : 'which';
+  var test = execa(cmd, [exe], options);
 
-  return new Bluebird.Promise(function(resolve) {
-    var test = childProcess.spawn(cmd, [exe], options);
-    test.on('error', function(error) {
-      log.error('Error spawning "' + cmd + exe + '"', error);
-    });
-    test.on('close', function(exitCode) {
-      return resolve(exitCode === 0);
-    });
+  test.on('error', function(error) {
+    log.error('Error spawning "' + cmd + exe + '"', error);
   });
-};
 
-exports.executableExists = executableExists;
+  return test.then(function(result) {
+    return result.code === 0;
+  }, function() {
+    return false;
+  });
+};

lib/process-ctl.js

@@ -1,7 +1,7 @@
 'use strict';
 
 var log = require('npmlog');
-var crossSpawn = require('cross-spawn');
+var execa = require('execa');
 var Bluebird = require('bluebird');
 var util = require('util');
 var EventEmitter = require('events').EventEmitter;
@@ -33,19 +33,16 @@ ProcessCtl.prototype.prepareOptions = function(options) {
   return extend({}, defaults, options);
 };
 
+ProcessCtl.prototype._spawn = function(exe, args, options) {
+  log.info('spawning: ' + exe + ' - ' + util.inspect(args));
+  var p  = new Process(this.name, { killTimeout: this.killTimeout }, execa(exe, args, options));
+  this.emit('processStarted', p);
+  return Bluebird.resolve(p);
+};
+
 ProcessCtl.prototype.spawn = function(exe, args, options) {
   var _options = this.prepareOptions(options);
-
   var self = this;
-  var spawn = function spawn(exe) {
-    log.info('spawning: ' + exe + ' - ' + util.inspect(args));
-
-    var p = new Process(this.name, { killTimeout: this.killTimeout }, crossSpawn(exe, args, _options));
-
-    this.emit('processStarted', p);
-
-    return Bluebird.resolve(p);
-  };
 
   if (Array.isArray(exe)) {
     return Bluebird.reduce(exe, function(found, exe) {
@@ -60,14 +57,14 @@ ProcessCtl.prototype.spawn = function(exe, args, options) {
       });
     }, false).then(function(found) {
       if (!found) {
-        return Bluebird.reject(new Error('No executable found in: ' + util.inspect(exe)));
+        throw new Error('No executable found in: ' + util.inspect(exe));
       }
 
-      return spawn.call(self, found);
+      return self._spawn.call(self, found, args, _options);
     });
   }
 
-  return spawn.call(this, exe);
+  return this._spawn.call(this, exe, args, _options);
 };
 
 ProcessCtl.prototype.exec = function(cmd, options) {

lib/utils/process.js

@@ -1,6 +1,6 @@
 'use strict';
 
-var childProcess = require('child_process');
+var execa = require('execa');
 var log = require('npmlog');
 var Bluebird = require('bluebird');
 var EventEmitter = require('events').EventEmitter;
@@ -121,7 +121,7 @@ function kill(p, sig) {
       args.push('/f');
     }
 
-    spawn(command, args).then(function(result) {
+    execa(command, args).then(function(result) {
       // Processes without windows can't be killed without /F, detect and force
       // kill them directly
       if (result.stderr.indexOf('can only be terminated forcefully') !== -1) {
@@ -135,22 +135,4 @@ function kill(p, sig) {
   }
 }
 
-function spawn(command, args, options) {
-  return new Bluebird.Promise(function(resolve, reject) {
-    var p = childProcess.spawn(command, args, options);
-    var stdout = '';
-    var stderr = '';
-    p.stdout.on('data', function(chunk) {
-      stdout += chunk;
-    });
-    p.stderr.on('data', function(chunk) {
-      stderr += chunk;
-    });
-    p.on('error', reject);
-    p.on('close', function(code) {
-      resolve({ code: code, stdout: stdout, stderr: stderr });
-    });
-  });
-}
-
 module.exports = Process;

package.json

@@ -26,7 +26,7 @@
     "charm": "^1.0.0",
     "commander": "^2.6.0",
     "consolidate": "^0.14.0",
-    "cross-spawn": "^5.1.0",
+    "execa": "^0.9.0",
     "express": "^4.10.7",
     "fireworm": "^0.7.0",
     "glob": "^7.0.4",

tests/ci/ci_tests.js

@@ -9,7 +9,7 @@ var assert = require('chai').assert;
 var expect = require('chai').expect;
 var path = require('path');
 var http = require('http');
-var childProcess = require('child_process');
+var execa = require('execa');
 var Bluebird = require('bluebird');
 
 var FakeReporter = require('../support/fake_reporter');
@@ -147,7 +147,7 @@ describe('ci mode app', function() {
           launcher.on('processStarted', function(process) {
             setTimeout(function() {
               if (isWin) {
-                childProcess.exec('taskkill /pid ' + process.pid + ' /T');
+                execa('taskkill /pid ' + process.pid + ' /T');
               } else {
                 process.kill();
               }

tests/fileutils_tests.js

@@ -1,8 +1,6 @@
 'use strict';
 
 var path = require('path');
-var childProcess = require('child_process');
-var EventEmitter = require('events').EventEmitter;
 
 var expect = require('chai').expect;
 var sinon = require('sinon');
@@ -63,20 +61,5 @@ describe('fileutils', function() {
         expect(result).to.be.false();
       });
     });
-
-    it('returns false for not existing executables with a custom which', function() {
-      var process = new EventEmitter();
-
-      sandbox.stub(childProcess, 'spawn').callsFake(function() {
-        setTimeout(function() {
-          process.emit('close', 127);
-        });
-        return process;
-      });
-
-      return fileutils.executableExists('not-found').then(function(result) {
-        expect(result).to.be.false();
-      });
-    });
   });
 });