Skip to content

Commit

Permalink
Merge pull request #714 from johanneswuerbach/callback-already-called
Browse files Browse the repository at this point in the history
Fix Error: Callback was already called.
  • Loading branch information
johanneswuerbach committed Jan 2, 2016
2 parents 9f29abd + ec8261d commit bcead48
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 28 deletions.
5 changes: 0 additions & 5 deletions lib/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,6 @@ App.prototype = {
browser.tryAttach(browserName, id, socket);
},

removeRunner: function(runner) {
_.remove(this.runners, runner);
this.emit('runnerRemoved', runner);
},

addRunner: function(runner) {
this.runners.push(runner);
this.emit('runnerAdded', runner);
Expand Down
19 changes: 7 additions & 12 deletions lib/launcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,24 @@ Launcher.prototype = {
}
return;
}
var launcherProcess = this.process;

this.process.removeListener('close', this.onClose.bind(this));
this.process.removeListener('error', this.onError.bind(this));

var self = this;
sig = sig || 'SIGTERM';

var exited = false;
launcherProcess.on('close', function(code, sig) {
if (exited) {
return;
}
exited = true;
this.process.once('close', function(code, sig) {
self.process = null;
if (self._killTimer) {
clearTimeout(self._killTimer);
self._killTimer = null;
}
log.info('Process ' + self.name + ' terminated.', code, sig);
launcherProcess.stdout.removeAllListeners();
launcherProcess.stderr.removeAllListeners();
launcherProcess.removeAllListeners();
if (cb) {
cb(null, code);
}
});
launcherProcess.on('error', function(err) {
this.process.on('error', function(err) {
log.error('Error killing process ' + self.name + '.', err);
});
this._killTimer = setTimeout(this.onKillTimeout.bind(this), this.killTimeout);
Expand Down
2 changes: 1 addition & 1 deletion lib/runners/browser_test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ BrowserTestRunner.prototype = {
return this.quit(this.onFinish);
},
quit: function(cb) {
log.info('Browser ' + this.name() + ' terminated.');
log.info('Closing browser ' + this.name() + '.');
this.launcher.kill(null, cb);
}
};
Expand Down
3 changes: 2 additions & 1 deletion lib/runners/tap_process_test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ TapProcessTestRunner.prototype = {
start: function(onFinish) {
this.finished = false;
this.onFinish = onFinish;
this.launcher.once('processError', this.onProcessError.bind(this));

this.launcher.start();
this.launcher.process.stdout.pipe(this.tapConsumer.stream);
this.launcher.once('processError', this.onProcessError.bind(this));

this.tapConsumer.on('test-result', this.onTestResult.bind(this));
this.tapConsumer.on('all-test-results', this.onAllTestResults.bind(this));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
process.stdin.resume();

process.on('SIGTERM', function() {
// Ignore
console.log('SIGTERM ignored');
});

process.stdin.resume();
9 changes: 4 additions & 5 deletions tests/launcher_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,12 @@ describe('Launcher', function() {
});
});
it('sends SIGKILL when SIGTERM is ignored', function(done) {
settings.command = 'node ' + path.join(__dirname, 'fixtures/ignore_sigterm');
settings.command = 'node ' + path.join(__dirname, 'fixtures/processes/ignore_sigterm.js');
launcher.start();
launcher.killTimeout = 200;
launcher.on('processExit', function() {
done();
});
launcher.kill();
setTimeout(function() {
launcher.kill(null, done);
}, 200);
});
});

Expand Down
4 changes: 2 additions & 2 deletions tests/runners/tap_process_test_runner_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,9 @@ describe('tap process test runner', function() {

it('not errored processes', function(done) {
var settings = {
exe: 'node-not-found'
exe: 'nope-not-existing'
};
var launcher = new Launcher('node-tap-bail-out', settings, config);
var launcher = new Launcher('nope-not-existing', settings, config);
var runner = new TapProcessTestRunner(launcher, reporter);

runner.start(function() {
Expand Down

0 comments on commit bcead48

Please sign in to comment.