Skip to content

Commit 86da9a4

Browse files
committed
js: Properly reject start-up promise if child-process dies.
Previous attempt relied on a cancellation error, which would not propogate through the control flow if not handled.
1 parent dc18923 commit 86da9a4

File tree

2 files changed

+46
-24
lines changed

2 files changed

+46
-24
lines changed

Diff for: javascript/node/selenium-webdriver/remote/index.js

+8-7
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,16 @@ DriverService.prototype.start = function(opt_timeoutMs) {
193193
pathname: self.path_
194194
});
195195

196-
var ready = httpUtil.waitForServer(serverUrl, timeout).then(function() {
196+
return new promise.Promise(function(fulfill, reject) {
197+
var ready = httpUtil.waitForServer(serverUrl, timeout)
198+
.then(fulfill, reject);
199+
earlyTermination.thenCatch(function(e) {
200+
ready.cancel(e);
201+
reject(Error(e.message));
202+
});
203+
}).then(function() {
197204
return serverUrl;
198205
});
199-
200-
earlyTermination.thenCatch(function(e) {
201-
ready.cancel(e.message);
202-
});
203-
204-
return ready;
205206
});
206207
}));
207208

Diff for: javascript/node/selenium-webdriver/test/remote_test.js

+38-17
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,52 @@ var promise = require('../').promise;
1919
var remote = require('../remote');
2020

2121
describe('DriverService', function() {
22+
describe('start()', function() {
23+
var service;
2224

23-
describe('start() fails if child-process dies', function() {
24-
var service = new remote.DriverService(process.execPath, {
25-
port: 1234,
26-
args: ['-e', 'process.exit(1)']
27-
})
25+
beforeEach(function() {
26+
service = new remote.DriverService(process.execPath, {
27+
port: 1234,
28+
args: ['-e', 'process.exit(1)']
29+
});
30+
});
2831

29-
after(function(done) {
32+
afterEach(function(done) {
3033
service.kill().thenFinally(function() {
3134
done();
3235
});
3336
});
3437

35-
it('', function(done) {
38+
it('fails if child-process dies', function(done) {
3639
this.timeout(1000);
37-
service.start(500).then(function() {
38-
done(Error('expected to fail'));
39-
}, function(e) {
40-
try {
41-
assert.equal('Server terminated early with status 1', e.message);
42-
done();
43-
} catch (e) {
44-
done(e);
45-
}
46-
});
40+
service.start(500)
41+
.then(expectFailure.bind(null, done), verifyFailure.bind(null, done));
4742
});
43+
44+
it('failures propagate through control flow if child-process dies',
45+
function(done) {
46+
this.timeout(1000);
47+
48+
promise.controlFlow().execute(function() {
49+
promise.controlFlow().execute(function() {
50+
return service.start(500);
51+
});
52+
})
53+
.then(expectFailure.bind(null, done), verifyFailure.bind(null, done));
54+
});
55+
56+
function verifyFailure(done, e) {
57+
try {
58+
assert.ok(!(e instanceof promise.CancellationError));
59+
assert.equal('Server terminated early with status 1', e.message);
60+
done();
61+
} catch (ex) {
62+
done(ex);
63+
}
64+
}
65+
66+
function expectFailure(done) {
67+
done(Error('expected to fail'));
68+
}
4869
});
4970
});

0 commit comments

Comments
 (0)