Skip to content

Commit dc7265b

Browse files
Alexeijohnjbarton
Alexei
authored and
johnjbarton
committed
fix(browser): ensure browser state is EXECUTING when tests start (#3074)
* 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. Introduced an additional browser state CONFIGURING The CONFIGURING state means that the browser is not just CONNECTED for tests, but someone has requested tests execution (and provided a config file). But the provided config file is not yet processed, configuration is not applied or the tests execution is not yet started and we have not received the first event from the remote browser, so the browser object is not yet at EXECUTING state. Refactored browser state names: renamed READY -> CONNECTED Fixes #1640
1 parent 7617279 commit dc7265b

7 files changed

Lines changed: 70 additions & 84 deletions

File tree

‎client/updater.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ function StatusUpdater (socket, titleElement, bannerElement, browsersElement) {
88
var items = []
99
var status
1010
for (var i = 0; i < browsers.length; i++) {
11-
status = browsers[i].isReady ? 'idle' : 'executing'
11+
status = browsers[i].isConnected ? 'idle' : 'executing'
1212
items.push('<li class="' + status + '">' + browsers[i].name + ' is ' + status + '</li>')
1313
}
1414
browsersElement.innerHTML = items.join('\n')

‎lib/browser.js‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ const Result = require('./browser_result')
44
const helper = require('./helper')
55
const logger = require('./logger')
66

7-
// The browser is ready to execute tests.
8-
const READY = 1
7+
// The browser is connected but not yet been commanded to execute tests.
8+
const CONNECTED = 1
99

10-
// The browser is executing the tests.
11-
const EXECUTING = 2
10+
// The browser has been told to execute tests; it is configuring before tests execution.
11+
const CONFIGURING = 2
1212

13-
// The browser is not executing, but temporarily disconnected (waiting for reconnecting).
14-
const READY_DISCONNECTED = 3
13+
// The browser is executing the tests.
14+
const EXECUTING = 3
1515

1616
// The browser is executing the tests, but temporarily disconnect (waiting for reconnecting).
1717
const EXECUTING_DISCONNECTED = 4
@@ -24,7 +24,7 @@ class Browser {
2424
this.id = id
2525
this.fullName = fullName
2626
this.name = helper.browserFullNameToShort(fullName)
27-
this.state = READY
27+
this.state = CONNECTED
2828
this.lastResult = new Result()
2929
this.disconnectsCount = 0
3030
this.activeSockets = [socket]
@@ -54,8 +54,8 @@ class Browser {
5454
this.emitter.emit('browser_register', this)
5555
}
5656

57-
isReady () {
58-
return this.state === READY
57+
isConnected () {
58+
return this.state === CONNECTED
5959
}
6060

6161
toString () {
@@ -76,7 +76,7 @@ class Browser {
7676
}
7777

7878
onKarmaError (error) {
79-
if (this.isReady()) {
79+
if (this.isConnected()) {
8080
return
8181
}
8282

@@ -87,7 +87,7 @@ class Browser {
8787
}
8888

8989
onInfo (info) {
90-
if (this.isReady()) {
90+
if (this.isConnected()) {
9191
return
9292
}
9393

@@ -114,6 +114,8 @@ class Browser {
114114
this.lastResult = new Result()
115115
this.lastResult.total = info.total
116116

117+
this.state = EXECUTING
118+
117119
if (info.total === null) {
118120
this.log.warn('Adapter did not report total number of specs.')
119121
}
@@ -123,11 +125,11 @@ class Browser {
123125
}
124126

125127
onComplete (result) {
126-
if (this.isReady()) {
128+
if (this.isConnected()) {
127129
return
128130
}
129131

130-
this.state = READY
132+
this.state = CONNECTED
131133
this.lastResult.totalTimeEnd()
132134

133135
if (!this.lastResult.success) {
@@ -148,9 +150,9 @@ class Browser {
148150
return
149151
}
150152

151-
if (this.state === READY) {
153+
if (this.state === CONNECTED) {
152154
this.disconnect()
153-
} else if (this.state === EXECUTING) {
155+
} else if (this.state === CONFIGURING || this.state === EXECUTING) {
154156
this.log.debug('Disconnected during run, waiting %sms for reconnecting.', this.disconnectDelay)
155157
this.state = EXECUTING_DISCONNECTED
156158

@@ -169,10 +171,10 @@ class Browser {
169171
if (this.state === EXECUTING_DISCONNECTED) {
170172
this.state = EXECUTING
171173
this.log.debug('Reconnected on %s.', newSocket.id)
172-
} else if (this.state === EXECUTING || this.state === READY) {
174+
} else if (this.state === CONNECTED || this.state === CONFIGURING || this.state === EXECUTING) {
173175
this.log.debug('New connection %s (already have %s)', newSocket.id, this.getActiveSocketsIds())
174176
} else if (this.state === DISCONNECTED) {
175-
this.state = READY
177+
this.state = CONNECTED
176178
this.log.info('Connected on socket %s with id %s', newSocket.id, this.id)
177179
this.collection.add(this)
178180

@@ -201,7 +203,7 @@ class Browser {
201203
}
202204

203205
// ignore - probably results from last run (after server disconnecting)
204-
if (this.isReady()) {
206+
if (this.isConnected()) {
205207
return
206208
}
207209

@@ -215,14 +217,15 @@ class Browser {
215217
return {
216218
id: this.id,
217219
name: this.name,
218-
isReady: this.state === READY
220+
isConnected: this.state === CONNECTED
219221
}
220222
}
221223

222224
execute (config) {
223225
this.activeSockets.forEach((socket) => socket.emit('execute', config))
224226

225-
this.state = EXECUTING
227+
this.state = CONFIGURING
228+
226229
this.refreshNoActivityTimeout()
227230
}
228231

@@ -279,9 +282,9 @@ Browser.factory = function (
279282
return new Browser(id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout)
280283
}
281284

282-
Browser.STATE_READY = READY
285+
Browser.STATE_CONNECTED = CONNECTED
286+
Browser.STATE_CONFIGURING = CONFIGURING
283287
Browser.STATE_EXECUTING = EXECUTING
284-
Browser.STATE_READY_DISCONNECTED = READY_DISCONNECTED
285288
Browser.STATE_EXECUTING_DISCONNECTED = EXECUTING_DISCONNECTED
286289
Browser.STATE_DISCONNECTED = DISCONNECTED
287290

‎lib/browser_collection.js‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use strict'
22

3-
const EXECUTING = require('./browser').STATE_EXECUTING
43
const Result = require('./browser_result')
54

65
class BrowserCollection {
@@ -31,19 +30,11 @@ class BrowserCollection {
3130
return this.browsers.find((browser) => browser.id === browserId) || null
3231
}
3332

34-
setAllToExecuting () {
35-
this.browsers.forEach((browser) => {
36-
browser.state = EXECUTING
37-
})
38-
39-
this.emitter.emit('browsers_change', this)
40-
}
41-
4233
areAllReady (nonReadyList) {
4334
nonReadyList = nonReadyList || []
4435

4536
this.browsers.forEach((browser) => {
46-
if (!browser.isReady()) {
37+
if (!browser.isConnected()) {
4738
nonReadyList.push(browser)
4839
}
4940
})

‎lib/executor.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ class Executor {
3131
log.debug('Captured %s browsers', this.capturedBrowsers.length)
3232
this.executionScheduled = false
3333
this.capturedBrowsers.clearResults()
34-
this.capturedBrowsers.setAllToExecuting()
3534
this.pendingCount = this.capturedBrowsers.length
3635
this.runningBrowsers = this.capturedBrowsers.clone()
3736
this.emitter.emit('run_start', this.runningBrowsers)

‎lib/reporters/base.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const BaseReporter = function (formatError, reportSlow, useColors, browserConsol
4747
msg += util.format(' (skipped %d)', results.skipped)
4848
}
4949

50-
if (browser.isReady) {
50+
if (browser.isConnected) {
5151
if (results.disconnected) {
5252
msg += this.FINISHED_DISCONNECTED
5353
} else if (results.error) {

‎test/unit/browser.spec.js‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ describe('Browser', () => {
9494
it('should ignore if browser not executing', () => {
9595
const spy = sinon.spy()
9696
emitter.on('browser_error', spy)
97-
browser.state = Browser.STATE_READY
97+
browser.state = Browser.STATE_CONNECTED
9898

9999
browser.onKarmaError()
100100
expect(browser.lastResult.error).to.equal(false)
@@ -130,7 +130,7 @@ describe('Browser', () => {
130130
const spy = sinon.spy()
131131
emitter.on('browser_dump', spy)
132132

133-
browser.state = Browser.STATE_READY
133+
browser.state = Browser.STATE_CONNECTED
134134
browser.onInfo({dump: 'something'})
135135
browser.onInfo({total: 20})
136136

@@ -144,8 +144,13 @@ describe('Browser', () => {
144144
browser = new Browser('fake-id', 'full name', collection, emitter, socket)
145145
})
146146

147+
it('should change state to EXECUTING', () => {
148+
browser.state = Browser.STATE_CONNECTED
149+
browser.onStart({total: 20})
150+
expect(browser.state).to.equal(Browser.STATE_EXECUTING)
151+
})
152+
147153
it('should set total count of specs', () => {
148-
browser.state = Browser.STATE_EXECUTING
149154
browser.onStart({total: 20})
150155
expect(browser.lastResult.total).to.equal(20)
151156
})
@@ -154,7 +159,6 @@ describe('Browser', () => {
154159
const spy = sinon.spy()
155160
emitter.on('browser_start', spy)
156161

157-
browser.state = Browser.STATE_EXECUTING
158162
browser.onStart({total: 20})
159163

160164
expect(spy).to.have.been.calledWith(browser, {total: 20})
@@ -172,10 +176,10 @@ describe('Browser', () => {
172176
Date.now.restore()
173177
})
174178

175-
it('should set isReady to true', () => {
179+
it('should set isConnected to true', () => {
176180
browser.state = Browser.STATE_EXECUTING
177181
browser.onComplete()
178-
expect(browser.isReady()).to.equal(true)
182+
expect(browser.isConnected()).to.equal(true)
179183
})
180184

181185
it('should fire "browsers_change" event', () => {
@@ -192,7 +196,7 @@ describe('Browser', () => {
192196
emitter.on('browsers_change', spy)
193197
emitter.on('browser_complete', spy)
194198

195-
browser.state = Browser.STATE_READY
199+
browser.state = Browser.STATE_CONNECTED
196200
browser.onComplete()
197201
expect(spy).not.to.have.been.called
198202
})
@@ -245,7 +249,7 @@ describe('Browser', () => {
245249
it('should not complete if browser not executing', () => {
246250
const spy = sinon.spy()
247251
emitter.on('browser_complete', spy)
248-
browser.state = Browser.STATE_READY
252+
browser.state = Browser.STATE_CONNECTED
249253

250254
browser.onDisconnect('socket.io-reason', socket)
251255
expect(spy).not.to.have.been.called
@@ -294,7 +298,7 @@ describe('Browser', () => {
294298

295299
browser.reconnect(mkSocket())
296300

297-
expect(browser.isReady()).to.equal(true)
301+
expect(browser.isConnected()).to.equal(true)
298302
})
299303
})
300304

@@ -328,7 +332,7 @@ describe('Browser', () => {
328332
})
329333

330334
it('should ignore if not running', () => {
331-
browser.state = Browser.STATE_READY
335+
browser.state = Browser.STATE_CONNECTED
332336
browser.onResult(createSuccessResult())
333337
browser.onResult(createSuccessResult())
334338
browser.onResult(createFailedResult())
@@ -360,27 +364,36 @@ describe('Browser', () => {
360364
})
361365

362366
describe('serialize', () => {
363-
it('should return plain object with only name, id, isReady properties', () => {
367+
it('should return plain object with only name, id, isConnected properties', () => {
364368
browser = new Browser('fake-id', 'full name', collection, emitter, socket)
365-
browser.state = Browser.STATE_READY
369+
browser.state = Browser.STATE_CONNECTED
366370
browser.name = 'Browser 1.0'
367371
browser.id = '12345'
368372

369-
expect(browser.serialize()).to.deep.equal({id: '12345', name: 'Browser 1.0', isReady: true})
373+
expect(browser.serialize()).to.deep.equal({id: '12345', name: 'Browser 1.0', isConnected: true})
370374
})
371375
})
372376

373-
describe('execute', () => {
374-
it('should emit execute and change state to EXECUTING', () => {
377+
describe('execute and start', () => {
378+
it('should emit execute and change state to CONFIGURING', () => {
375379
const spyExecute = sinon.spy()
376380
const config = {}
377381
browser = new Browser('fake-id', 'full name', collection, emitter, socket)
378382
socket.on('execute', spyExecute)
379383
browser.execute(config)
380384

381-
expect(browser.isReady()).to.equal(false)
385+
expect(browser.state).to.equal(Browser.STATE_CONFIGURING)
382386
expect(spyExecute).to.have.been.calledWith(config)
383387
})
388+
389+
it('should emit start and change state to EXECUTING', () => {
390+
browser = new Browser('fake-id', 'full name', collection, emitter, socket)
391+
browser.init() // init socket listeners
392+
393+
expect(browser.state).to.equal(Browser.STATE_CONNECTED)
394+
socket.emit('start', {total: 1})
395+
expect(browser.state).to.equal(Browser.STATE_EXECUTING)
396+
})
384397
})
385398

386399
describe('scenario:', () => {
@@ -391,15 +404,15 @@ describe('Browser', () => {
391404
browser.state = Browser.STATE_EXECUTING
392405
socket.emit('result', {success: true, suite: [], log: []})
393406
socket.emit('disconnect', 'socket.io reason')
394-
expect(browser.isReady()).to.equal(false)
407+
expect(browser.isConnected()).to.equal(false)
395408

396409
const newSocket = mkSocket()
397410
browser.reconnect(newSocket)
398-
expect(browser.isReady()).to.equal(false)
411+
expect(browser.isConnected()).to.equal(false)
399412

400413
newSocket.emit('result', {success: false, suite: [], log: []})
401414
newSocket.emit('complete')
402-
expect(browser.isReady()).to.equal(true)
415+
expect(browser.isConnected()).to.equal(true)
403416
expect(browser.lastResult.success).to.equal(1)
404417
expect(browser.lastResult.failed).to.equal(1)
405418
})
@@ -427,6 +440,7 @@ describe('Browser', () => {
427440
const timer = createMockTimer()
428441
browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, timer, 10)
429442
browser.init()
443+
expect(browser.state).to.equal(Browser.STATE_CONNECTED)
430444

431445
browser.execute()
432446
socket.emit('start', {total: 10})
@@ -443,8 +457,9 @@ describe('Browser', () => {
443457

444458
// reconnect on a new socket (which triggers re-execution)
445459
browser.reconnect(newSocket)
446-
expect(browser.state).to.equal(Browser.STATE_EXECUTING)
460+
expect(browser.state).to.equal(Browser.STATE_CONFIGURING)
447461
newSocket.emit('start', {total: 11})
462+
expect(browser.state).to.equal(Browser.STATE_EXECUTING)
448463
socket.emit('result', {success: true, suite: [], log: []})
449464

450465
// expected cleared last result (should not include the results from previous run)
@@ -459,6 +474,8 @@ describe('Browser', () => {
459474
// we need to keep the old socket, in the case that the new socket will disconnect.
460475
browser = new Browser('fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10)
461476
browser.init()
477+
expect(browser.state).to.equal(Browser.STATE_CONNECTED)
478+
462479
browser.execute()
463480

464481
// A second connection...
@@ -467,6 +484,8 @@ describe('Browser', () => {
467484

468485
// Disconnect the second connection...
469486
browser.onDisconnect('socket.io-reason', newSocket)
487+
expect(browser.state).to.equal(Browser.STATE_CONFIGURING)
488+
socket.emit('start', {total: 1})
470489
expect(browser.state).to.equal(Browser.STATE_EXECUTING)
471490

472491
// It should still be listening on the old socket.

0 commit comments

Comments
 (0)