From 69ab0d785f954bc025affffb471f38c5fdc48b19 Mon Sep 17 00:00:00 2001 From: Alexei Solodovnicov Date: Fri, 6 Jul 2018 00:24:10 +0200 Subject: [PATCH 1/3] fix(browser): ensure browser state is EXECUTING when tests start Browser state is EXECUTING when it actually started to execute tests. This state change is triggered by client on actual tests execution start. --- lib/browser.js | 2 ++ lib/browser_collection.js | 9 --------- lib/executor.js | 1 - test/unit/browser.spec.js | 8 ++++++-- test/unit/browser_collection.spec.js | 26 -------------------------- 5 files changed, 8 insertions(+), 38 deletions(-) diff --git a/lib/browser.js b/lib/browser.js index ba870157e..8d42af288 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -101,6 +101,8 @@ class Browser { this.lastResult = new Result() this.lastResult.total = info.total + this.state = EXECUTING + if (info.total === null) { this.log.warn('Adapter did not report total number of specs.') } diff --git a/lib/browser_collection.js b/lib/browser_collection.js index fd77fd815..e42376353 100644 --- a/lib/browser_collection.js +++ b/lib/browser_collection.js @@ -1,6 +1,5 @@ 'use strict' -const EXECUTING = require('./browser').STATE_EXECUTING const Result = require('./browser_result') class BrowserCollection { @@ -31,14 +30,6 @@ class BrowserCollection { return this.browsers.find((browser) => browser.id === browserId) || null } - setAllToExecuting () { - this.browsers.forEach((browser) => { - browser.state = EXECUTING - }) - - this.emitter.emit('browsers_change', this) - } - areAllReady (nonReadyList) { nonReadyList = nonReadyList || [] diff --git a/lib/executor.js b/lib/executor.js index 55694c4b8..189c575fc 100644 --- a/lib/executor.js +++ b/lib/executor.js @@ -31,7 +31,6 @@ class Executor { log.debug('Captured %s browsers', this.capturedBrowsers.length) this.executionScheduled = false this.capturedBrowsers.clearResults() - this.capturedBrowsers.setAllToExecuting() this.pendingCount = this.capturedBrowsers.length this.runningBrowsers = this.capturedBrowsers.clone() this.emitter.emit('run_start', this.runningBrowsers) diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index 3d857773e..398b85b99 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -136,8 +136,13 @@ describe('Browser', () => { browser = new Browser('fake-id', 'full name', collection, emitter, socket) }) + it('should change state to EXECUTING', () => { + browser.state = Browser.STATE_READY + browser.onStart({total: 20}) + expect(browser.state).to.equal(Browser.STATE_EXECUTING) + }) + it('should set total count of specs', () => { - browser.state = Browser.STATE_EXECUTING browser.onStart({total: 20}) expect(browser.lastResult.total).to.equal(20) }) @@ -146,7 +151,6 @@ describe('Browser', () => { const spy = sinon.spy() emitter.on('browser_start', spy) - browser.state = Browser.STATE_EXECUTING browser.onStart({total: 20}) expect(spy).to.have.been.calledWith(browser, {total: 20}) diff --git a/test/unit/browser_collection.spec.js b/test/unit/browser_collection.spec.js index 7fb36a45f..20023eb93 100644 --- a/test/unit/browser_collection.spec.js +++ b/test/unit/browser_collection.spec.js @@ -71,32 +71,6 @@ describe('BrowserCollection', () => { }) }) - describe('setAllToExecuting', () => { - let browsers = null - - beforeEach(() => { - browsers = [new Browser(), new Browser(), new Browser()] - browsers.forEach((browser) => { - collection.add(browser) - }) - }) - - it('should set all browsers state to executing', () => { - collection.setAllToExecuting() - browsers.forEach((browser) => { - expect(browser.isReady()).to.equal(false) - expect(browser.state).to.equal(Browser.STATE_EXECUTING) - }) - }) - - it('should fire "browsers_change" event', () => { - const spy = sinon.spy() - emitter.on('browsers_change', spy) - collection.setAllToExecuting() - expect(spy).to.have.been.called - }) - }) - describe('areAllReady', () => { let browsers = null From 88b6a74c06c8076447e287f67f0862e3f17a7930 Mon Sep 17 00:00:00 2001 From: Alexei Solodovnicov Date: Mon, 23 Jul 2018 16:31:08 +0200 Subject: [PATCH 2/3] fix(browser): introduced an additional browser state CONFIGURING The CONFIGURING state means that the browser is already not READY for tests, as someone has requested tests execution (and provided a config file). But the provided config file is not yet processed, configuration is not applied and the tests execution is not yet started, so the browser is not yet at EXECUTING state. --- lib/browser.js | 19 ++++++++++++------- test/unit/browser.spec.js | 23 +++++++++++++++++++---- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/lib/browser.js b/lib/browser.js index 8d42af288..f95f8120c 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -7,17 +7,20 @@ const logger = require('./logger') // The browser is ready to execute tests. const READY = 1 +// The browser is configuring before tests execution. +const CONFIGURING = 2 + // The browser is executing the tests. -const EXECUTING = 2 +const EXECUTING = 3 // The browser is not executing, but temporarily disconnected (waiting for reconnecting). -const READY_DISCONNECTED = 3 +const READY_DISCONNECTED = 4 // The browser is executing the tests, but temporarily disconnect (waiting for reconnecting). -const EXECUTING_DISCONNECTED = 4 +const EXECUTING_DISCONNECTED = 5 // The browser got permanently disconnected (being removed from the collection and destroyed). -const DISCONNECTED = 5 +const DISCONNECTED = 6 class Browser { constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) { @@ -139,7 +142,7 @@ class Browser { if (this.state === READY) { this.disconnect() - } else if (this.state === EXECUTING) { + } else if (this.state === CONFIGURING || this.state === EXECUTING) { this.log.debug('Disconnected during run, waiting %sms for reconnecting.', this.disconnectDelay) this.state = EXECUTING_DISCONNECTED @@ -158,7 +161,7 @@ class Browser { if (this.state === EXECUTING_DISCONNECTED) { this.state = EXECUTING this.log.debug('Reconnected on %s.', newSocket.id) - } else if (this.state === EXECUTING || this.state === READY) { + } else if (this.state === READY || this.state === CONFIGURING || this.state === EXECUTING) { this.log.debug('New connection %s (already have %s)', newSocket.id, this.getActiveSocketsIds()) } else if (this.state === DISCONNECTED) { this.state = READY @@ -211,7 +214,8 @@ class Browser { execute (config) { this.activeSockets.forEach((socket) => socket.emit('execute', config)) - this.state = EXECUTING + this.state = CONFIGURING + this.refreshNoActivityTimeout() } @@ -269,6 +273,7 @@ Browser.factory = function ( } Browser.STATE_READY = READY +Browser.STATE_CONFIGURING = CONFIGURING Browser.STATE_EXECUTING = EXECUTING Browser.STATE_READY_DISCONNECTED = READY_DISCONNECTED Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index 398b85b99..c15feecb2 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -366,17 +366,26 @@ describe('Browser', () => { }) }) - describe('execute', () => { - it('should emit execute and change state to EXECUTING', () => { + describe('execute and start', () => { + it('should emit execute and change state to CONFIGURING', () => { const spyExecute = sinon.spy() const config = {} browser = new Browser('fake-id', 'full name', collection, emitter, socket) socket.on('execute', spyExecute) browser.execute(config) - expect(browser.isReady()).to.equal(false) + expect(browser.state).to.equal(Browser.STATE_CONFIGURING) expect(spyExecute).to.have.been.calledWith(config) }) + + it('should emit start and change state to EXECUTING', () => { + browser = new Browser('fake-id', 'full name', collection, emitter, socket) + browser.init() // init socket listeners + + expect(browser.state).to.equal(Browser.STATE_READY) + socket.emit('start', {total: 1}) + expect(browser.state).to.equal(Browser.STATE_EXECUTING) + }) }) describe('scenario:', () => { @@ -423,6 +432,7 @@ describe('Browser', () => { const timer = createMockTimer() browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, timer, 10) browser.init() + expect(browser.state).to.equal(Browser.STATE_READY) browser.execute() socket.emit('start', {total: 10}) @@ -439,8 +449,9 @@ describe('Browser', () => { // reconnect on a new socket (which triggers re-execution) browser.reconnect(newSocket) - expect(browser.state).to.equal(Browser.STATE_EXECUTING) + expect(browser.state).to.equal(Browser.STATE_CONFIGURING) newSocket.emit('start', {total: 11}) + expect(browser.state).to.equal(Browser.STATE_EXECUTING) socket.emit('result', {success: true, suite: [], log: []}) // expected cleared last result (should not include the results from previous run) @@ -455,6 +466,8 @@ describe('Browser', () => { // we need to keep the old socket, in the case that the new socket will disconnect. browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10) browser.init() + expect(browser.state).to.equal(Browser.STATE_READY) + browser.execute() // A second connection... @@ -463,6 +476,8 @@ describe('Browser', () => { // Disconnect the second connection... browser.onDisconnect('socket.io-reason', newSocket) + expect(browser.state).to.equal(Browser.STATE_CONFIGURING) + socket.emit('start', {total: 1}) expect(browser.state).to.equal(Browser.STATE_EXECUTING) // It should still be listening on the old socket. From 65799d3f030332f591c8e744436c0531be9061ce Mon Sep 17 00:00:00 2001 From: Alexei Solodovnicov Date: Mon, 23 Jul 2018 20:14:32 +0200 Subject: [PATCH 3/3] fix(browser): refactored browser state names --- client/updater.js | 2 +- lib/browser.js | 40 +++++++++++++--------------- lib/browser_collection.js | 2 +- lib/reporters/base.js | 2 +- test/unit/browser.spec.js | 36 ++++++++++++------------- test/unit/browser_collection.spec.js | 6 ++--- 6 files changed, 42 insertions(+), 46 deletions(-) diff --git a/client/updater.js b/client/updater.js index b1562e1cf..6c6be933a 100644 --- a/client/updater.js +++ b/client/updater.js @@ -8,7 +8,7 @@ function StatusUpdater (socket, titleElement, bannerElement, browsersElement) { var items = [] var status for (var i = 0; i < browsers.length; i++) { - status = browsers[i].isReady ? 'idle' : 'executing' + status = browsers[i].isConnected ? 'idle' : 'executing' items.push('
  • ' + browsers[i].name + ' is ' + status + '
    • ') } browsersElement.innerHTML = items.join('\n') diff --git a/lib/browser.js b/lib/browser.js index f95f8120c..6f6995ac1 100644 --- a/lib/browser.js +++ b/lib/browser.js @@ -4,30 +4,27 @@ const Result = require('./browser_result') const helper = require('./helper') const logger = require('./logger') -// The browser is ready to execute tests. -const READY = 1 +// The browser is connected but not yet been commanded to execute tests. +const CONNECTED = 1 -// The browser is configuring before tests execution. +// The browser has been told to execute tests; it is configuring before tests execution. const CONFIGURING = 2 // The browser is executing the tests. const EXECUTING = 3 -// The browser is not executing, but temporarily disconnected (waiting for reconnecting). -const READY_DISCONNECTED = 4 - // The browser is executing the tests, but temporarily disconnect (waiting for reconnecting). -const EXECUTING_DISCONNECTED = 5 +const EXECUTING_DISCONNECTED = 4 // The browser got permanently disconnected (being removed from the collection and destroyed). -const DISCONNECTED = 6 +const DISCONNECTED = 5 class Browser { constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) { this.id = id this.fullName = fullName this.name = helper.browserFullNameToShort(fullName) - this.state = READY + this.state = CONNECTED this.lastResult = new Result() this.disconnectsCount = 0 this.activeSockets = [socket] @@ -57,8 +54,8 @@ class Browser { this.emitter.emit('browser_register', this) } - isReady () { - return this.state === READY + isConnected () { + return this.state === CONNECTED } toString () { @@ -66,7 +63,7 @@ class Browser { } onKarmaError (error) { - if (this.isReady()) { + if (this.isConnected()) { return } @@ -77,7 +74,7 @@ class Browser { } onInfo (info) { - if (this.isReady()) { + if (this.isConnected()) { return } @@ -115,11 +112,11 @@ class Browser { } onComplete (result) { - if (this.isReady()) { + if (this.isConnected()) { return } - this.state = READY + this.state = CONNECTED this.lastResult.totalTimeEnd() if (!this.lastResult.success) { @@ -140,7 +137,7 @@ class Browser { return } - if (this.state === READY) { + if (this.state === CONNECTED) { this.disconnect() } else if (this.state === CONFIGURING || this.state === EXECUTING) { this.log.debug('Disconnected during run, waiting %sms for reconnecting.', this.disconnectDelay) @@ -161,10 +158,10 @@ class Browser { if (this.state === EXECUTING_DISCONNECTED) { this.state = EXECUTING this.log.debug('Reconnected on %s.', newSocket.id) - } else if (this.state === READY || this.state === CONFIGURING || this.state === EXECUTING) { + } else if (this.state === CONNECTED || this.state === CONFIGURING || this.state === EXECUTING) { this.log.debug('New connection %s (already have %s)', newSocket.id, this.getActiveSocketsIds()) } else if (this.state === DISCONNECTED) { - this.state = READY + this.state = CONNECTED this.log.info('Connected on socket %s with id %s', newSocket.id, this.id) this.collection.add(this) @@ -193,7 +190,7 @@ class Browser { } // ignore - probably results from last run (after server disconnecting) - if (this.isReady()) { + if (this.isConnected()) { return } @@ -207,7 +204,7 @@ class Browser { return { id: this.id, name: this.name, - isReady: this.state === READY + isConnected: this.state === CONNECTED } } @@ -272,10 +269,9 @@ Browser.factory = function ( return new Browser(id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) } -Browser.STATE_READY = READY +Browser.STATE_CONNECTED = CONNECTED Browser.STATE_CONFIGURING = CONFIGURING Browser.STATE_EXECUTING = EXECUTING -Browser.STATE_READY_DISCONNECTED = READY_DISCONNECTED Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED Browser.STATE_DISCONNECTED = DISCONNECTED diff --git a/lib/browser_collection.js b/lib/browser_collection.js index e42376353..0c624d762 100644 --- a/lib/browser_collection.js +++ b/lib/browser_collection.js @@ -34,7 +34,7 @@ class BrowserCollection { nonReadyList = nonReadyList || [] this.browsers.forEach((browser) => { - if (!browser.isReady()) { + if (!browser.isConnected()) { nonReadyList.push(browser) } }) diff --git a/lib/reporters/base.js b/lib/reporters/base.js index d5227ef4a..e55a8819d 100644 --- a/lib/reporters/base.js +++ b/lib/reporters/base.js @@ -47,7 +47,7 @@ const BaseReporter = function (formatError, reportSlow, useColors, browserConsol msg += util.format(' (skipped %d)', results.skipped) } - if (browser.isReady) { + if (browser.isConnected) { if (results.disconnected) { msg += this.FINISHED_DISCONNECTED } else if (results.error) { diff --git a/test/unit/browser.spec.js b/test/unit/browser.spec.js index c15feecb2..a8c17d2ef 100644 --- a/test/unit/browser.spec.js +++ b/test/unit/browser.spec.js @@ -86,7 +86,7 @@ describe('Browser', () => { it('should ignore if browser not executing', () => { const spy = sinon.spy() emitter.on('browser_error', spy) - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onKarmaError() expect(browser.lastResult.error).to.equal(false) @@ -122,7 +122,7 @@ describe('Browser', () => { const spy = sinon.spy() emitter.on('browser_dump', spy) - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onInfo({dump: 'something'}) browser.onInfo({total: 20}) @@ -137,7 +137,7 @@ describe('Browser', () => { }) it('should change state to EXECUTING', () => { - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onStart({total: 20}) expect(browser.state).to.equal(Browser.STATE_EXECUTING) }) @@ -168,10 +168,10 @@ describe('Browser', () => { Date.now.restore() }) - it('should set isReady to true', () => { + it('should set isConnected to true', () => { browser.state = Browser.STATE_EXECUTING browser.onComplete() - expect(browser.isReady()).to.equal(true) + expect(browser.isConnected()).to.equal(true) }) it('should fire "browsers_change" event', () => { @@ -188,7 +188,7 @@ describe('Browser', () => { emitter.on('browsers_change', spy) emitter.on('browser_complete', spy) - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onComplete() expect(spy).not.to.have.been.called }) @@ -241,7 +241,7 @@ describe('Browser', () => { it('should not complete if browser not executing', () => { const spy = sinon.spy() emitter.on('browser_complete', spy) - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onDisconnect('socket.io-reason', socket) expect(spy).not.to.have.been.called @@ -290,7 +290,7 @@ describe('Browser', () => { browser.reconnect(mkSocket()) - expect(browser.isReady()).to.equal(true) + expect(browser.isConnected()).to.equal(true) }) }) @@ -324,7 +324,7 @@ describe('Browser', () => { }) it('should ignore if not running', () => { - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.onResult(createSuccessResult()) browser.onResult(createSuccessResult()) browser.onResult(createFailedResult()) @@ -356,13 +356,13 @@ describe('Browser', () => { }) describe('serialize', () => { - it('should return plain object with only name, id, isReady properties', () => { + it('should return plain object with only name, id, isConnected properties', () => { browser = new Browser('fake-id', 'full name', collection, emitter, socket) - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED browser.name = 'Browser 1.0' browser.id = '12345' - expect(browser.serialize()).to.deep.equal({id: '12345', name: 'Browser 1.0', isReady: true}) + expect(browser.serialize()).to.deep.equal({id: '12345', name: 'Browser 1.0', isConnected: true}) }) }) @@ -382,7 +382,7 @@ describe('Browser', () => { browser = new Browser('fake-id', 'full name', collection, emitter, socket) browser.init() // init socket listeners - expect(browser.state).to.equal(Browser.STATE_READY) + expect(browser.state).to.equal(Browser.STATE_CONNECTED) socket.emit('start', {total: 1}) expect(browser.state).to.equal(Browser.STATE_EXECUTING) }) @@ -396,15 +396,15 @@ describe('Browser', () => { browser.state = Browser.STATE_EXECUTING socket.emit('result', {success: true, suite: [], log: []}) socket.emit('disconnect', 'socket.io reason') - expect(browser.isReady()).to.equal(false) + expect(browser.isConnected()).to.equal(false) const newSocket = mkSocket() browser.reconnect(newSocket) - expect(browser.isReady()).to.equal(false) + expect(browser.isConnected()).to.equal(false) newSocket.emit('result', {success: false, suite: [], log: []}) newSocket.emit('complete') - expect(browser.isReady()).to.equal(true) + expect(browser.isConnected()).to.equal(true) expect(browser.lastResult.success).to.equal(1) expect(browser.lastResult.failed).to.equal(1) }) @@ -432,7 +432,7 @@ describe('Browser', () => { const timer = createMockTimer() browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, timer, 10) browser.init() - expect(browser.state).to.equal(Browser.STATE_READY) + expect(browser.state).to.equal(Browser.STATE_CONNECTED) browser.execute() socket.emit('start', {total: 10}) @@ -466,7 +466,7 @@ describe('Browser', () => { // we need to keep the old socket, in the case that the new socket will disconnect. browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10) browser.init() - expect(browser.state).to.equal(Browser.STATE_READY) + expect(browser.state).to.equal(Browser.STATE_CONNECTED) browser.execute() diff --git a/test/unit/browser_collection.spec.js b/test/unit/browser_collection.spec.js index 20023eb93..417d2ad79 100644 --- a/test/unit/browser_collection.spec.js +++ b/test/unit/browser_collection.spec.js @@ -77,7 +77,7 @@ describe('BrowserCollection', () => { beforeEach(() => { browsers = [new Browser(), new Browser(), new Browser()] browsers.forEach((browser) => { - browser.state = Browser.STATE_READY + browser.state = Browser.STATE_CONNECTED collection.add(browser) }) }) @@ -109,8 +109,8 @@ describe('BrowserCollection', () => { collection.add(browsers[1]) expect(collection.serialize()).to.deep.equal([ - {id: '1', name: 'B 1.0', isReady: true}, - {id: '2', name: 'B 2.0', isReady: true} + {id: '1', name: 'B 1.0', isConnected: true}, + {id: '2', name: 'B 2.0', isConnected: true} ]) }) })