Skip to content

Commit 59873ed

Browse files
MasterOdinMasterOdin
committed
Fix returning promise with passing options
Signed-off-by: Matthew Peveler <matt.peveler@gmail.com>
1 parent 724a0e1 commit 59873ed

File tree

3 files changed

+245
-116
lines changed

3 files changed

+245
-116
lines changed

‎lib/portfinder.js

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,6 @@ exports.setBasePath = function (path) {
124124
};
125125

126126
internals.getPort = function (options, callback) {
127-
if (!callback) {
128-
callback = options;
129-
options = {};
130-
}
131-
132127
options.port = Number(options.port) || Number(exports.basePort);
133128
options.host = options.host || null;
134129
options.stopPort = Number(options.stopPort) || Number(exports.highestPort);
@@ -139,7 +134,7 @@ internals.getPort = function (options, callback) {
139134
throw Error('Provided options.startPort(' + options.startPort + ') is less than 0, which are cannot be bound.');
140135
}
141136
if(options.stopPort < options.startPort) {
142-
throw Error('Provided options.stopPort(' + options.stopPort + 'is less than options.startPort (' + options.startPort + ')');
137+
throw Error('Provided options.stopPort(' + options.stopPort + ') is less than options.startPort (' + options.startPort + ')');
143138
}
144139
}
145140

@@ -182,7 +177,7 @@ internals.getPort = function (options, callback) {
182177
} else {
183178
const idx = exports._defaultHosts.indexOf(currentHost);
184179
exports._defaultHosts.splice(idx, 1);
185-
return exports.getPort(options, callback);
180+
return internals.getPort(options, callback);
186181
}
187182
} else {
188183
// error is not accounted for, file ticket, handle special case
@@ -208,7 +203,7 @@ internals.getPort = function (options, callback) {
208203
}
209204
} else {
210205
// otherwise, try again, using sorted port, aka, highest open for >= 1 host
211-
return exports.getPort({ port: openPorts.pop(), host: options.host, startPort: options.startPort, stopPort: options.stopPort }, callback);
206+
return internals.getPort({ port: openPorts.pop(), host: options.host, startPort: options.startPort, stopPort: options.stopPort }, callback);
212207
}
213208

214209
});
@@ -220,11 +215,13 @@ internals.getPort = function (options, callback) {
220215
// Responds with a unbound port on the current machine.
221216
//
222217
exports.getPort = function (options, callback) {
223-
if (!callback) {
218+
if (typeof options === 'function') {
224219
callback = options;
225220
options = {};
226221
}
227222

223+
options = options || {};
224+
228225
if (!callback) {
229226
return new Promise(function (resolve, reject) {
230227
internals.getPort(options, function (err, port) {
@@ -247,18 +244,13 @@ exports.getPort = function (options, callback) {
247244
exports.getPortPromise = exports.getPort;
248245

249246
internals.getPorts = function (count, options, callback) {
250-
if (!callback) {
251-
callback = options;
252-
options = {};
253-
}
254-
255247
let lastPort = null;
256248
_async.timesSeries(count, function(index, asyncCallback) {
257249
if (lastPort) {
258250
options.port = exports.nextPort(lastPort);
259251
}
260252

261-
exports.getPort(options, function (err, port) {
253+
internals.getPort(options, function (err, port) {
262254
if (err) {
263255
asyncCallback(err);
264256
} else {
@@ -277,11 +269,13 @@ internals.getPorts = function (count, options, callback) {
277269
// Responds with an array of unbound ports on the current machine.
278270
//
279271
exports.getPorts = function (count, options, callback) {
280-
if (!callback) {
272+
if (typeof options === 'function') {
281273
callback = options;
282274
options = {};
283275
}
284276

277+
options = options || {};
278+
285279
if (!callback) {
286280
return new Promise(function(resolve, reject) {
287281
internals.getPorts(count, options, function(err, ports) {

‎test/port-finder-multiple.test.js

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,35 +22,84 @@ describe('with 5 existing servers', function () {
2222
helper.stopServers(servers, done);
2323
});
2424

25-
test('the getPorts() method with an argument of 3 should respond with the first three available ports (32773, 32774, 32775)', function (done) {
26-
portfinder.getPorts(3, function (err, ports) {
27-
expect(err).toBeNull();
28-
expect(ports).toEqual([32773, 32774, 32775]);
29-
done();
25+
describe.each([
26+
['getPorts()', false, portfinder.getPorts],
27+
['getPorts()', true, portfinder.getPorts],
28+
['getPortsPromise()', true, portfinder.getPortsPromise],
29+
])(`the %s method (promise: %p)`, function (name, isPromise, method) {
30+
test('with an argument of 3 should respond with the first three available ports (32773, 32774, 32775)', function (done) {
31+
if (isPromise) {
32+
method(3)
33+
.then(function (ports) {
34+
expect(ports).toEqual([32773, 32774, 32775]);
35+
done();
36+
})
37+
.catch(function (err) {
38+
done(err);
39+
});
40+
} else {
41+
method(3, function (err, ports) {
42+
if (err) {
43+
done(err);
44+
return;
45+
}
46+
expect(err).toBeNull();
47+
expect(ports).toEqual([32773, 32774, 32775]);
48+
done();
49+
});
50+
}
51+
});
52+
53+
test('with stopPort smaller than 3 available ports', function (done) {
54+
if (isPromise) {
55+
method(3, { stopPort: 32774 })
56+
.then(function () {
57+
done('Expected error to be thrown');
58+
})
59+
.catch(function (err) {
60+
expect(err).not.toBeNull();
61+
expect(err.message).toEqual('No open ports found in between 32768 and 32774');
62+
done();
63+
});
64+
} else {
65+
method(3, { stopPort: 32774 }, function (err, ports) {
66+
expect(err).not.toBeNull();
67+
expect(err.message).toEqual('No open ports found in between 32768 and 32774');
68+
expect(ports).toEqual([32773, 32774, undefined]);
69+
done();
70+
});
71+
}
3072
});
3173
});
3274
});
3375

3476
describe('with no existing servers', function () {
35-
test('the getPorts() method with an argument of 3 should respond with the first three available ports (32768, 32769, 32770)', function (done) {
36-
portfinder.getPorts(3, function (err, ports) {
37-
expect(err).toBeNull();
38-
expect(ports).toEqual([32768, 32769, 32770]);
39-
done();
77+
describe.each([
78+
['getPorts()', false, portfinder.getPorts],
79+
['getPorts()', true, portfinder.getPorts],
80+
['getPortsPromise()', true, portfinder.getPortsPromise],
81+
])(`the %s method (promise: %p)`, function (name, isPromise, method) {
82+
test('with an argument of 3 should respond with the first three available ports (32768, 32769, 32770)', function (done) {
83+
if (isPromise) {
84+
method(3)
85+
.then(function (ports) {
86+
expect(ports).toEqual([32768, 32769, 32770]);
87+
done();
88+
})
89+
.catch(function (err) {
90+
done(err);
91+
});
92+
} else {
93+
method(3, function (err, ports) {
94+
if (err) {
95+
done(err);
96+
return;
97+
}
98+
expect(err).toBeNull();
99+
expect(ports).toEqual([32768, 32769, 32770]);
100+
done();
101+
});
102+
}
40103
});
41104
});
42-
43-
test.each([
44-
['getPorts()', portfinder.getPorts],
45-
['getPortsPromise()', portfinder.getPortsPromise],
46-
])('the %s promise method with an argument of 3 should respond with the first three available ports (32768, 32769, 32770)', function (name, method, done) {
47-
method(3)
48-
.then(function (ports) {
49-
expect(ports).toEqual([32768, 32769, 32770]);
50-
done();
51-
})
52-
.catch(function (err) {
53-
done(err);
54-
});
55-
});
56105
});

0 commit comments

Comments
 (0)