Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(browser): ensure browser state is EXECUTING when tests start #3074

Merged
merged 3 commits into from
Jul 23, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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.
Alexei Solodovnicov committed Jul 23, 2018
commit 88b6a74c06c8076447e287f67f0862e3f17a7930
19 changes: 12 additions & 7 deletions lib/browser.js
Original file line number Diff line number Diff line change
@@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a slightly better description would be

// 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 = 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
23 changes: 19 additions & 4 deletions test/unit/browser.spec.js
Original file line number Diff line number Diff line change
@@ -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.