diff --git a/lib/server.js b/lib/server.js index cbad577ed..1b440231c 100644 --- a/lib/server.js +++ b/lib/server.js @@ -116,9 +116,10 @@ class Server extends KarmaEventEmitter { BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'), BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js') ]) - .then(() => NetUtils.getAvailablePort(config.port, config.listenAddress)) - .then((port) => { - config.port = port + .then(() => NetUtils.bindAvailablePort(config.port, config.listenAddress)) + .then((boundServer) => { + config.port = boundServer.address().port + this._boundServer = boundServer this._injector.invoke(this._start, this) }) .catch(this.dieOnError.bind(this)) @@ -156,7 +157,7 @@ class Server extends KarmaEventEmitter { this._injector.invoke(watcher.watch) } - webServer.listen(config.port, config.listenAddress, () => { + webServer.listen(this._boundServer, () => { this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`) this.emit('listening', config.port) diff --git a/lib/utils/net-utils.js b/lib/utils/net-utils.js index 7262ba40b..28531d441 100644 --- a/lib/utils/net-utils.js +++ b/lib/utils/net-utils.js @@ -4,29 +4,24 @@ const Promise = require('bluebird') const net = require('net') const NetUtils = { - isPortAvailable (port, listenAddress) { + bindAvailablePort (port, listenAddress) { return new Promise((resolve, reject) => { const server = net.createServer() - server.unref() - server.on('error', (err) => { - server.close() - if (err.code === 'EADDRINUSE' || err.code === 'EACCES') { - resolve(false) - } else { - reject(err) - } - }) - - server.listen(port, listenAddress, () => { - server.close(() => resolve(true)) - }) + server + .on('error', (err) => { + server.close() + if (err.code === 'EADDRINUSE' || err.code === 'EACCES') { + server.listen(++port, listenAddress) + } else { + reject(err) + } + }) + .on('listening', () => { + resolve(server) + }) + .listen(port, listenAddress) }) - }, - - getAvailablePort (port, listenAddress) { - return NetUtils.isPortAvailable(port, listenAddress) - .then((available) => available ? port : NetUtils.getAvailablePort(port + 1, listenAddress)) } } diff --git a/package.json b/package.json index e111f79b3..6bac5fdf5 100644 --- a/package.json +++ b/package.json @@ -195,6 +195,7 @@ "Samuel Marks ", "Saugat Acharya ", "Schmulik Raskin ", + "Sergei Startsev ", "Sergey Kruk ", "Sergey Simonchik ", "Seth Rhodes ", diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js index a13104739..fcd2f9b61 100644 --- a/test/unit/server.spec.js +++ b/test/unit/server.spec.js @@ -11,6 +11,7 @@ describe('server', () => { var mockLauncher var mockWebServer var mockSocketServer + var mockBoundServer var mockExecutor var doneSpy var server = mockConfig = browserCollection = webServerOnError = null @@ -45,8 +46,7 @@ describe('server', () => { sinon.stub(server._injector, 'invoke').returns([]) - mockExecutor = - {schedule: () => {}} + mockExecutor = { schedule: () => {} } mockFileList = { refresh: sinon.spy(() => { @@ -80,6 +80,13 @@ describe('server', () => { } } + mockBoundServer = { + on: sinon.spy((event, callback) => callback && callback()), + listen: sinon.spy((port, listenAddress, callback) => callback && callback()), + close: sinon.spy((callback) => callback && callback()), + address: () => { return { port: 9876 } } + } + mockWebServer = { on (name, handler) { if (name === 'error') { @@ -99,7 +106,8 @@ describe('server', () => { close: sinon.spy((callback) => callback && callback()) } - sinon.stub(server._injector, 'get') + sinon + .stub(server._injector, 'get') .withArgs('webServer').returns(mockWebServer) .withArgs('socketServer').returns(mockSocketServer) @@ -107,10 +115,15 @@ describe('server', () => { }) describe('start', () => { + var config beforeEach(() => { + config = { port: 9876, listenAddress: '127.0.0.1' } sinon.spy(BundleUtils, 'bundleResourceIfNotExist') - sinon.stub(NetUtils, 'getAvailablePort').resolves(9876) - sinon.stub(server, 'get').withArgs('config').returns({ port: 9876, listenAddress: '127.0.0.1' }) + sinon.stub(NetUtils, 'bindAvailablePort').resolves(mockBoundServer) + sinon.stub(mockBoundServer, 'address').returns({ port: 9877 }) + sinon + .stub(server, 'get') + .withArgs('config').returns(config) }) it('should compile static resources', (done) => { @@ -123,7 +136,17 @@ describe('server', () => { it('should search for available port', (done) => { server.start().then(() => { - expect(NetUtils.getAvailablePort).to.have.been.calledWith(9876, '127.0.0.1') + expect(NetUtils.bindAvailablePort).to.have.been.calledWith(9876, '127.0.0.1') + expect(mockBoundServer.address).to.have.been.called + done() + }) + }) + + it('should change config.port to available', (done) => { + expect(config.port).to.be.equal(9876) + server.start().then(() => { + expect(config.port).to.be.equal(9877) + expect(server._boundServer).to.be.equal(mockBoundServer) done() }) }) @@ -133,6 +156,10 @@ describe('server', () => { // server._start() // ============================================================================ describe('_start', () => { + beforeEach(() => { + server._boundServer = mockBoundServer + }) + it('should start the web server after all files have been preprocessed successfully', () => { server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) @@ -142,7 +169,8 @@ describe('server', () => { expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher) fileListOnResolve() - expect(mockWebServer.listen).to.have.been.called + expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func) + expect(webServerOnError).not.to.be.null expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher) }) @@ -155,7 +183,8 @@ describe('server', () => { expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher) fileListOnReject() - expect(mockWebServer.listen).to.have.been.called + expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func) + expect(webServerOnError).not.to.be.null expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher) }) @@ -163,26 +192,14 @@ describe('server', () => { server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) expect(mockWebServer.listen).not.to.have.been.called + expect(webServerOnError).not.to.be.null expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher) fileListOnResolve() - expect(mockWebServer.listen).to.have.been.called + expect(mockWebServer.listen).to.have.been.calledWith(mockBoundServer, sinon.match.func) expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher) }) - it('should listen on the listenAddress in the config', () => { - server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) - - expect(mockWebServer.listen).not.to.have.been.called - expect(webServerOnError).not.to.be.null - - expect(mockConfig.listenAddress).to.be.equal('127.0.0.1') - - fileListOnResolve() - expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1') - expect(mockConfig.listenAddress).to.be.equal('127.0.0.1') - }) - it('should emit a listening event once server begin accepting connections', () => { server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy) diff --git a/test/unit/utils/net-utils.spec.js b/test/unit/utils/net-utils.spec.js index 23d32e4cf..e7051b970 100644 --- a/test/unit/utils/net-utils.spec.js +++ b/test/unit/utils/net-utils.spec.js @@ -4,46 +4,33 @@ const NetUtils = require('../../../lib/utils/net-utils') const connect = require('connect') const net = require('net') -describe('NetUtils.isPortAvailable', () => { - it('it is possible to run server on available port', (done) => { - NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => { - expect(available).to.be.true - const server = net - .createServer(connect()) - .listen(9876, '127.0.0.1', () => { - server.close(done) - }) +describe('NetUtils.bindAvailablePort', () => { + it('resolves server with bound port when it is available', (done) => { + NetUtils.bindAvailablePort(9876, '127.0.0.1').then((boundServer) => { + const port = boundServer.address().port + expect(port).to.be.equal(9876) + expect(boundServer).not.to.be.null + const server = net.createServer(connect()).listen(boundServer, () => { + server.close(done) + }) }) }) - it('resolves with false when port is used', (done) => { - const server = net - .createServer(connect()) - .listen(9876, '127.0.0.1', () => { - NetUtils.isPortAvailable(9876, '127.0.0.1').then((available) => { - expect(available).to.be.false - server.close(done) - }) + it('resolves with next available port', (done) => { + const server = net.createServer(connect()).listen(9876, '127.0.0.1', () => { + NetUtils.bindAvailablePort(9876, '127.0.0.1').then((boundServer) => { + const port = boundServer.address().port + expect(port).to.be.equal(9877) + expect(boundServer).not.to.be.null + server.close(done) }) - }) -}) - -describe('NetUtils.getAvailablePort', () => { - it('resolves with port when is available', (done) => { - NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => { - expect(port).to.equal(9876) - done() }) }) - it('resolves with next available port', (done) => { - const stub = sinon.stub(NetUtils, 'isPortAvailable') - stub.withArgs(9876).resolves(false) - stub.withArgs(9877).resolves(false) - stub.withArgs(9878).resolves(true) - - NetUtils.getAvailablePort(9876, '127.0.0.1').then((port) => { - expect(port).to.equal(9878) + it('rejects if a critical error occurs', (done) => { + const incorrectAddress = '123.321' + NetUtils.bindAvailablePort(9876, incorrectAddress).catch((err) => { + expect(err).not.to.be.null done() }) })