From fee477117d0c2ecf4f47961338929b54f57c8c47 Mon Sep 17 00:00:00 2001
From: step2yeung <syeung@linkedin.com>
Date: Thu, 9 May 2019 14:27:28 -0700
Subject: [PATCH] core fix: if the socket disconnects, reconnect with
 browser-login event with the new socket

---
 docs/config_file.md                        |  1 +
 lib/config.js                              |  1 +
 lib/runners/browser_test_runner.js         | 11 +++++--
 public/testem/testem_connection.js         |  3 ++
 tests/runners/browser_test_runner_tests.js | 35 +++++++++++++++++++++-
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/docs/config_file.md b/docs/config_file.md
index bb870d84e..bef69df0f 100644
--- a/docs/config_file.md
+++ b/docs/config_file.md
@@ -68,6 +68,7 @@ Chrome, Chrome Canary, Chromium, Firefox, IE, Opera, PhantomJS, Safari, Safari T
 
     custom_browser_socket_events    [Object]  an object containing keys corresponding to event names that point to handler functions, which are to be added to the browser socket
     browser_disconnect_timeout   [Number]  timeout to error after disconnect in seconds (10s)
+    browser_reconnect_limit      [Number]  number of browser reconnects to allow (3)
     browser_start_timeout        [Number]  timeout to error after browser start in seconds (30s)
     browser_paths:               [Object]  hash of browsers (keys) to an string of their binary paths (values)
     browser_exes:                [Object]  hash of browsers (keys) to an string of their binary names (values)
diff --git a/lib/config.js b/lib/config.js
index 005d5538d..24e1cc234 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -502,6 +502,7 @@ Config.prototype.defaults = {
   bail_on_uncaught_error: true,
   browser_start_timeout: 30,
   browser_disconnect_timeout: 10,
+  browser_reconnect_limit: 3,
   client_decycle_depth: 5,
   socket_heartbeat_timeout() {
     let browserDisconnectTimeout = this.get('browser_disconnect_timeout');
diff --git a/lib/runners/browser_test_runner.js b/lib/runners/browser_test_runner.js
index 68967498d..59680d96e 100644
--- a/lib/runners/browser_test_runner.js
+++ b/lib/runners/browser_test_runner.js
@@ -18,6 +18,7 @@ module.exports = class BrowserTestRunner {
     this.singleRun = singleRun;
     this.logs = [];
     this.currentTestContext = {};
+    this.disconnectCount = 0;
 
     this.pendingTimer = undefined;
     this.onProcessExitTimer = undefined;
@@ -88,7 +89,10 @@ module.exports = class BrowserTestRunner {
 
   tryAttach(browser, id, socket) {
     if (id !== this.launcherId) {
-      return;
+      return false;
+    } else if (this.disconnectCount > this.launcher.config.get('browser_reconnect_limit')) {
+      log.info(`socket reconnection limit has been exceeded, not attaching new socket for browser ${browser} and id ${id}`);
+      return false;
     }
 
     log.info('tryAttach', browser, id);
@@ -140,6 +144,8 @@ module.exports = class BrowserTestRunner {
     tap.on('all-test-results', () => {
       this.socket.emit('tap-all-test-results');
     });
+
+    return true;
   }
 
   name() {
@@ -241,7 +247,8 @@ module.exports = class BrowserTestRunner {
     if (this.finished) { return; }
 
     this.pending = true;
-    let timeout  = this.launcher.config.get('browser_disconnect_timeout');
+    this.disconnectCount += 1;
+    const timeout = this.launcher.config.get('browser_disconnect_timeout');
     this.pendingTimer = setTimeout(() => {
       if (this.finished) {
         return;
diff --git a/public/testem/testem_connection.js b/public/testem/testem_connection.js
index 6c27a0d0c..f3941d03b 100644
--- a/public/testem/testem_connection.js
+++ b/public/testem/testem_connection.js
@@ -181,6 +181,9 @@ function initSocket(id) {
     connectStatus = 'connected';
     syncConnectStatus();
   });
+  socket.on('reconnect', function() {
+    this.emit('browser-login', getBrowserName(navigator.userAgent), id);
+  });
   socket.on('disconnect', function() {
     connectStatus = 'disconnected';
     syncConnectStatus();
diff --git a/tests/runners/browser_test_runner_tests.js b/tests/runners/browser_test_runner_tests.js
index 4b343fab3..850b99a9a 100644
--- a/tests/runners/browser_test_runner_tests.js
+++ b/tests/runners/browser_test_runner_tests.js
@@ -361,12 +361,45 @@ describe('browser test runner', function() {
     });
   });
 
+  describe('tryAttach', function() {
+    let reporter, launcher, runner, socket;
+
+    beforeEach(function() {
+      reporter = new FakeReporter();
+      let config = new Config('ci', {
+        reporter: reporter,
+        browser_disconnect_timeout: 0.1,
+        browser_reconnect_limit: 0
+      });
+      launcher = new Launcher('ci', { protocol: 'browser' }, config);
+      runner = new BrowserTestRunner(launcher, reporter, null, null, config);
+      socket = new FakeSocket();
+    });
+
+    it('exceeding browser reconnect limit should not allow browser reconnect', function(done) {
+      let didReattach = true;
+      launcher.settings.exe = 'node';
+      launcher.settings.args = [path.join(__dirname, '../fixtures/processes/just-running.js')];
+      runner.start(function() {
+        expect(didReattach).to.eq(false);
+        done();
+      });
+
+      runner.tryAttach('browser', launcher.id, socket);
+      runner.onDisconnect();
+      didReattach = runner.tryAttach('browser', launcher.id, socket);
+    });
+  });
+
   describe('onDisconnect', function() {
     let reporter, launcher, runner, socket;
 
     beforeEach(function() {
       reporter = new FakeReporter();
-      let config = new Config('ci', { reporter: reporter, browser_disconnect_timeout: 0.1 });
+      let config = new Config('ci', {
+        reporter: reporter,
+        browser_disconnect_timeout: 0.1
+      });
       launcher = new Launcher('ci', { protocol: 'browser' }, config);
       runner = new BrowserTestRunner(launcher, reporter, null, null, config);
       socket = new FakeSocket();