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.

Sign up for GitHub

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

Jump to bottom

chore(test): mark all second connections reconnects #3598

Merged
merged 1 commit into from
Dec 23, 2020
Merged

chore(test): mark all second connections reconnects #3598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 5 additions & 6 deletions client/karma.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@ function Karma (socket, iframe, opener, navigator, location, document) {
}
}

// This variable will be set to "true" whenever the socket lost connection and was able to
// reconnect to the Karma server. This will be passed to the Karma server then, so that
// Karma can differentiate between a socket client reconnect and a full browser reconnect.
// To start we will signal the server that we are not reconnecting. If the socket loses
// connection and was able to reconnect to the Karma server we will get a
// second 'connect' event. There we will pass 'true' and that will be passed to the
// Karma server then, so that Karma can differentiate between a socket client
// econnect and a full browser reconnect.
var socketReconnect = false

this.VERSION = constant.VERSION
Expand Down Expand Up @@ -306,9 +308,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
info.displayName = displayName
}
socket.emit('register', info)
})

socket.on('reconnect', function () {
socketReconnect = true
})
}
Expand Down
59 changes: 35 additions & 24 deletions lib/browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ const EXECUTING_DISCONNECTED = 'EXECUTING_DISCONNECTED' // The browser is execut
const DISCONNECTED = 'DISCONNECTED' // The browser got completely disconnected (e.g. browser crash) and can be only restored with a restart of execution.

class Browser {
constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout) {
constructor (id, fullName, collection, emitter, socket, timer, disconnectDelay,
noActivityTimeout, singleRun, clientConfig) {
this.id = id
this.fullName = fullName
this.name = helper.browserFullNameToShort(fullName)
this.lastResult = new BrowserResult()
this.disconnectsCount = 0
this.activeSockets = [socket]
this.noActivityTimeout = noActivityTimeout
this.singleRun = singleRun
this.clientConfig = clientConfig
this.collection = collection
this.emitter = emitter
this.socket = socket
Expand Down Expand Up @@ -94,9 +97,8 @@ class Browser {
}
}

onDisconnect (reason, disconnectedSocket) {
onSocketDisconnect (reason, disconnectedSocket) {
helper.arrayRemove(this.activeSockets, disconnectedSocket)

if (this.activeSockets.length) {
this.log.debug(`Disconnected ${disconnectedSocket.id}, still have ${this.getActiveSocketsIds()}`)
return
Expand All @@ -119,24 +121,27 @@ class Browser {
}
}

reconnect (newSocket) {
if (this.state === EXECUTING_DISCONNECTED) {
reconnect (newSocket, clientSaysReconnect) {
if (!clientSaysReconnect || this.state === DISCONNECTED) {
this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`)
this.setState(CONNECTED)

// The disconnected browser is already part of the collection.
// Update the collection view in the UI (header on client.html)
this.emitter.emit('browsers_change', this.collection)
// Notify the launcher
this.emitter.emit('browser_register', this)
// Execute tests if configured to do so.
if (this.singleRun) {
this.execute()
}
} else if (this.state === EXECUTING_DISCONNECTED) {
this.log.debug('Lost socket connection, but browser continued to execute. Reconnected ' +
`on socket ${newSocket.id}.`)
this.setState(EXECUTING)
} else if ([CONNECTED, CONFIGURING, EXECUTING].includes(this.state)) {
this.log.debug(`Rebinding to new socket ${newSocket.id} (already have ` +
`${this.getActiveSocketsIds()})`)
} else if (this.state === DISCONNECTED) {
this.log.info(`Disconnected browser returned on socket ${newSocket.id} with id ${this.id}.`)
this.setState(CONNECTED)

// Since the disconnected browser is already part of the collection and we want to
// make sure that the server can properly handle the browser like it's the first time
// connecting this browser (as we want a complete new execution), we need to emit the
// following events:
this.emitter.emit('browsers_change', this.collection)
this.emitter.emit('browser_register', this)
}

if (!this.activeSockets.some((s) => s.id === newSocket.id)) {
Expand All @@ -161,8 +166,8 @@ class Browser {
this.refreshNoActivityTimeout()
}

execute (config) {
this.activeSockets.forEach((socket) => socket.emit('execute', config))
execute () {
this.activeSockets.forEach((socket) => socket.emit('execute', this.clientConfig))
this.setState(CONFIGURING)
this.refreshNoActivityTimeout()
}
Expand All @@ -172,10 +177,14 @@ class Browser {
}

disconnect (reason) {
this.log.warn(`Disconnected (${this.disconnectsCount} times)${reason || ''}`)
this.setState(DISCONNECTED)
this.log.warn(`Disconnected (${this.disconnectsCount} times) ${reason || ''}`)
johnjbarton marked this conversation as resolved.
Show resolved Hide resolved
this.disconnectsCount++
this.emitter.emit('browser_error', this, `Disconnected${reason || ''}`)
this.emitter.emit('browser_error', this, `Disconnected ${reason || ''}`)
johnjbarton marked this conversation as resolved.
Show resolved Hide resolved
this.remove()
}

remove () {
this.setState(DISCONNECTED)
this.collection.remove(this)
}

Expand All @@ -201,7 +210,7 @@ class Browser {

bindSocketEvents (socket) {
// TODO: check which of these events are actually emitted by socket
socket.on('disconnect', (reason) => this.onDisconnect(reason, socket))
socket.on('disconnect', (reason) => this.onSocketDisconnect(reason, socket))
socket.on('start', (info) => this.onStart(info))
socket.on('karma_error', (error) => this.onKarmaError(error))
socket.on('complete', (result) => this.onComplete(result))
Expand Down Expand Up @@ -246,9 +255,11 @@ class Browser {
Browser.factory = function (
id, fullName, /* capturedBrowsers */ collection, emitter, socket, timer,
/* config.browserDisconnectTimeout */ disconnectDelay,
/* config.browserNoActivityTimeout */ noActivityTimeout
) {
return new Browser(id, fullName, collection, emitter, socket, timer, disconnectDelay, noActivityTimeout)
/* config.browserNoActivityTimeout */ noActivityTimeout,
/* config.singleRun */ singleRun,
/* config.client */ clientConfig) {
return new Browser(id, fullName, collection, emitter, socket, timer,
disconnectDelay, noActivityTimeout, singleRun, clientConfig)
}

Browser.STATE_CONNECTED = CONNECTED
Expand Down
28 changes: 6 additions & 22 deletions lib/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,27 +263,12 @@ class Server extends KarmaEventEmitter {
})

socket.on('register', (info) => {
let newBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null

if (newBrowser) {
// By default if a browser disconnects while still executing, we assume that the test
// execution still continues because just the socket connection has been terminated. Now
// since we know whether this is just a socket reconnect or full client reconnect, we
// need to update the browser state accordingly. This is necessary because in case a
// browser crashed and has been restarted, we need to start with a fresh execution.
if (!info.isSocketReconnect) {
newBrowser.setState(Browser.STATE_DISCONNECTED)
}

newBrowser.reconnect(socket)
const knownBrowser = info.id ? (capturedBrowsers.getById(info.id) || singleRunBrowsers.getById(info.id)) : null

// Since not every reconnected browser is able to continue with its previous execution,
// we need to start a new execution in case a browser has restarted and is now idling.
if (newBrowser.state === Browser.STATE_CONNECTED && config.singleRun) {
newBrowser.execute(config.client)
}
if (knownBrowser) {
knownBrowser.reconnect(socket, info.isSocketReconnect)
} else {
newBrowser = this._injector.createChild([{
const newBrowser = this._injector.createChild([{
johnjbarton marked this conversation as resolved.
Show resolved Hide resolved
id: ['value', info.id || null],
fullName: ['value', (helper.isDefined(info.displayName) ? info.displayName : info.name)],
socket: ['value', socket]
Expand All @@ -292,7 +277,7 @@ class Server extends KarmaEventEmitter {
newBrowser.init()

if (config.singleRun) {
newBrowser.execute(config.client)
newBrowser.execute()
singleRunBrowsers.add(newBrowser)
}
}
Expand Down Expand Up @@ -336,8 +321,7 @@ class Server extends KarmaEventEmitter {
singleRunDoneBrowsers[completedBrowser.id] = true

if (launcher.kill(completedBrowser.id)) {
// workaround to supress "disconnect" warning
completedBrowser.state = Browser.STATE_DISCONNECTED
completedBrowser.remove()
}

emitRunCompleteIfAllBrowsersDone()
Expand Down
11 changes: 5 additions & 6 deletions static/karma.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ function Karma (socket, iframe, opener, navigator, location, document) {
}
}

// This variable will be set to "true" whenever the socket lost connection and was able to
// reconnect to the Karma server. This will be passed to the Karma server then, so that
// Karma can differentiate between a socket client reconnect and a full browser reconnect.
// To start we will signal the server that we are not reconnecting. If the socket loses
// connection and was able to reconnect to the Karma server we will get a
// second 'connect' event. There we will pass 'true' and that will be passed to the
// Karma server then, so that Karma can differentiate between a socket client
// econnect and a full browser reconnect.
var socketReconnect = false

this.VERSION = constant.VERSION
Expand Down Expand Up @@ -316,9 +318,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
info.displayName = displayName
}
socket.emit('register', info)
})

socket.on('reconnect', function () {
socketReconnect = true
})
}
Expand Down
6 changes: 4 additions & 2 deletions test/client/karma.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,13 @@ describe('Karma', function () {
})

it('should mark "register" event for reconnected socket', function () {
// First connect.
socket.emit('connect')

socket.on('register', sinon.spy(function (info) {
assert(info.isSocketReconnect === true)
}))

socket.emit('reconnect')
// Reconnect
socket.emit('connect')
})

Expand Down
1 change: 1 addition & 0 deletions test/e2e/reconnecting.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Feature: Passing Options
When I start Karma
Then it passes with:
"""
LOG: '============== START TEST =============='
.....
Chrome Headless
"""
15 changes: 7 additions & 8 deletions test/e2e/support/reconnecting/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,26 @@ describe('plus', function () {
}

it('should pass', function () {
// In flaky fails we probably get two starts.
console.log('============== START TEST ==============')
expect(1).toBe(1)
})

it('should disconnect', function (done) {
expect(2).toBe(2)
socket().disconnect()

done()
setTimeout(() => {
socket().disconnect()
done()
}, 500)
})

it('should work', function () {
expect(plus(1, 2)).toBe(3)
})

it('should re-connect', function (done) {
it('should re-connect', function () {
expect(4).toBe(4)
// Emit reconnect, so Karma will not start new test run after reconnecting.
socket().emit('reconnect')
socket().connect()

done()
})

it('should work', function () {
Expand Down
Loading