Skip to content

Commit

Permalink
fix(browser): don't add already active socket again on reconnect
Browse files Browse the repository at this point in the history
Current behaviour: few 'register' messages from the same socket will lead to crash on disconnect
After fix: next 'register' messages from the same socket will be skipped
  • Loading branch information
j0tunn committed Nov 20, 2014
1 parent 85328f3 commit 37a7958
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 9 deletions.
10 changes: 8 additions & 2 deletions lib/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,14 @@ var Browser = function(id, fullName, /* capturedBrowsers */ collection, emitter,
emitter.emit('browser_register', this);
}

activeSockets.push(newSocket);
events.bindAll(this, newSocket);
var exists = activeSockets.some(function(s) {
return s.id === newSocket.id;
});
if (!exists) {
activeSockets.push(newSocket);
events.bindAll(this, newSocket);
}

if (pendingDisconnect) {
timer.clearTimeout(pendingDisconnect);
}
Expand Down
35 changes: 28 additions & 7 deletions test/unit/browser.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ describe 'Browser', ->
createMockTimer = require './mocks/timer'

browser = collection = emitter = socket = null
socketId = 0

mkSocket = ->
s = new e.EventEmitter
s.id = socketId++
return s

beforeEach ->
socket = new e.EventEmitter
socket = mkSocket()
emitter = new e.EventEmitter
collection = new Collection emitter

Expand Down Expand Up @@ -250,7 +256,7 @@ describe 'Browser', ->
browser.state = Browser.STATE_EXECUTING

browser.onDisconnect 'socket.io-reason', socket
browser.reconnect new e.EventEmitter
browser.reconnect mkSocket()

timer.wind 10
expect(browser.state).to.equal Browser.STATE_EXECUTING
Expand All @@ -263,7 +269,7 @@ describe 'Browser', ->
browser.init()
browser.state = Browser.STATE_EXECUTING

browser.reconnect new e.EventEmitter
browser.reconnect mkSocket()

# still accept results on the old socket
socket.emit 'result', {success: true}
Expand All @@ -281,7 +287,7 @@ describe 'Browser', ->
browser = new Browser 'id', 'Chrome 25.0', collection, emitter, socket, null, 10
browser.state = Browser.STATE_DISCONNECTED

browser.reconnect new e.EventEmitter
browser.reconnect mkSocket()

expect(browser.isReady()).to.equal true

Expand Down Expand Up @@ -388,7 +394,7 @@ describe 'Browser', ->
socket.emit 'disconnect', 'socket.io reason'
expect(browser.isReady()).to.equal false

newSocket = new e.EventEmitter
newSocket = mkSocket()
browser.reconnect newSocket
expect(browser.isReady()).to.equal false

Expand Down Expand Up @@ -429,7 +435,7 @@ describe 'Browser', ->
expect(browser.state).to.equal Browser.STATE_DISCONNECTED
expect(browser.disconnectsCount).to.equal 1

newSocket = new e.EventEmitter
newSocket = mkSocket()
emitter.on 'browser_register', -> browser.execute()

# reconnect on a new socket (which triggers re-execution)
Expand All @@ -453,7 +459,7 @@ describe 'Browser', ->
browser.execute()

# A second connection...
newSocket = new e.EventEmitter
newSocket = mkSocket()
browser.reconnect newSocket

# Disconnect the second connection...
Expand All @@ -464,6 +470,21 @@ describe 'Browser', ->
socket.emit 'result', {success: true, suite: [], log: []}
expect(browser.lastResult.success).to.equal 1

it 'complete only once after reconnect on the same socket', ->
# If there is a new connection on the same socket,
# we should emit complete message only once.
browser = new Browser 'fake-id', 'Chrome 31.0', collection, emitter, socket, null, 10
browser.onComplete = sinon.spy()
browser.init()
browser.execute()

# A second connection...
browser.reconnect socket

socket.emit 'result', {success: true, suite: [], log: []}
socket.emit 'complete'

expect(browser.onComplete.callCount).to.equal 1

it 'disconnect when no message during the run', ->
timer = createMockTimer()
Expand Down

0 comments on commit 37a7958

Please sign in to comment.