fix(test): clear up clearContext

emit the 'complete' event after the navigation event, if any. The 'complete' event on the client triggers the server to begin shutdown. The shutdown can race with the navigate context. Simplify return_url implementation, assuming that we don't need any additional execution in the client after we send 'complete'.
karma-runner · Dec 22, 2020 · 59bc7b0 · 59bc7b0
1 parent 1a65bf1
commit 59bc7b0
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 70 deletions.
diff --git a/client/karma.js b/client/karma.js
@@ -137,10 +137,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
     }
   }
 
-  function clearContext () {
-    navigateContextTo('about:blank')
-  }
-
   this.log = function (type, args) {
     var values = []
 
@@ -234,15 +230,15 @@ function Karma (socket, iframe, opener, navigator, location, document) {
       socket.emit('result', resultsBuffer)
       resultsBuffer = []
     }
+    // A test could have incorrectly issued a navigate. Wait one turn
+    // to ensure the error from an incorrect navigate is processed.
+    setTimeout(() => {
+      if (this.config.clearContext) {
+        navigateContextTo('about:blank')
+      }
 
-    if (self.config.clearContext) {
-      // A test could have incorrectly issued a navigate. To clear the context
-      // we will navigate the iframe. Delay ours to ensure the error from an
-      // incorrect navigate is processed.
-      setTimeout(clearContext)
-    }
+      socket.emit('complete', result || {})
 
-    socket.emit('complete', result || {}, function () {
       if (returnUrl) {
         location.href = returnUrl
       }
@@ -260,26 +256,23 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   }
 
   socket.on('execute', function (cfg) {
-    // Delay our navigation to the next event in case the clearContext has not completed.
-    setTimeout(function allowClearContextToComplete () {
-      // reset startEmitted and reload the iframe
-      startEmitted = false
-      self.config = cfg
-
-      navigateContextTo(constant.CONTEXT_URL)
-
-      if (self.config.clientDisplayNone) {
-        [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
-          el.style.display = 'none'
-        })
-      }
+    // reset startEmitted and reload the iframe
+    startEmitted = false
+    self.config = cfg
 
-      // clear the console before run
-      // works only on FF (Safari, Chrome do not allow to clear console from js source)
-      if (window.console && window.console.clear) {
-        window.console.clear()
-      }
-    })
+    navigateContextTo(constant.CONTEXT_URL)
+
+    if (self.config.clientDisplayNone) {
+      [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
+        el.style.display = 'none'
+      })
+    }
+
+    // clear the console before run
+    // works only on FF (Safari, Chrome do not allow to clear console from js source)
+    if (window.console && window.console.clear) {
+      window.console.clear()
+    }
   })
   socket.on('stop', function () {
     this.complete()

diff --git a/lib/server.js b/lib/server.js
@@ -256,8 +256,6 @@ class Server extends KarmaEventEmitter {
 
       const replySocketEvents = events.bufferEvents(socket, ['start', 'info', 'karma_error', 'result', 'complete'])
 
-      socket.on('complete', (data, ack) => ack())
-
       socket.on('error', (err) => {
         this.log.debug('karma server socket error: ' + err)
       })

diff --git a/static/karma.js b/static/karma.js
@@ -147,10 +147,6 @@ function Karma (socket, iframe, opener, navigator, location, document) {
     }
   }
 
-  function clearContext () {
-    navigateContextTo('about:blank')
-  }
-
   this.log = function (type, args) {
     var values = []
 
@@ -244,15 +240,15 @@ function Karma (socket, iframe, opener, navigator, location, document) {
       socket.emit('result', resultsBuffer)
       resultsBuffer = []
     }
+    // A test could have incorrectly issued a navigate. Wait one turn
+    // to ensure the error from an incorrect navigate is processed.
+    setTimeout(() => {
+      if (this.config.clearContext) {
+        navigateContextTo('about:blank')
+      }
 
-    if (self.config.clearContext) {
-      // A test could have incorrectly issued a navigate. To clear the context
-      // we will navigate the iframe. Delay ours to ensure the error from an
-      // incorrect navigate is processed.
-      setTimeout(clearContext)
-    }
+      socket.emit('complete', result || {})
 
-    socket.emit('complete', result || {}, function () {
       if (returnUrl) {
         location.href = returnUrl
       }
@@ -270,26 +266,23 @@ function Karma (socket, iframe, opener, navigator, location, document) {
   }
 
   socket.on('execute', function (cfg) {
-    // Delay our navigation to the next event in case the clearContext has not completed.
-    setTimeout(function allowClearContextToComplete () {
-      // reset startEmitted and reload the iframe
-      startEmitted = false
-      self.config = cfg
-
-      navigateContextTo(constant.CONTEXT_URL)
-
-      if (self.config.clientDisplayNone) {
-        [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
-          el.style.display = 'none'
-        })
-      }
+    // reset startEmitted and reload the iframe
+    startEmitted = false
+    self.config = cfg
 
-      // clear the console before run
-      // works only on FF (Safari, Chrome do not allow to clear console from js source)
-      if (window.console && window.console.clear) {
-        window.console.clear()
-      }
-    })
+    navigateContextTo(constant.CONTEXT_URL)
+
+    if (self.config.clientDisplayNone) {
+      [].forEach.call(document.querySelectorAll('#banner, #browsers'), function (el) {
+        el.style.display = 'none'
+      })
+    }
+
+    // clear the console before run
+    // works only on FF (Safari, Chrome do not allow to clear console from js source)
+    if (window.console && window.console.clear) {
+      window.console.clear()
+    }
   })
   socket.on('stop', function () {
     this.complete()

diff --git a/test/client/karma.spec.js b/test/client/karma.spec.js
@@ -437,24 +437,20 @@ describe('Karma', function () {
     it('should navigate the client to return_url if specified', function (done) {
       windowLocation.search = '?id=567&return_url=http://return.com'
       socket = new MockSocket()
-      k = new ClientKarma(socket, {}, windowStub, windowNavigator, windowLocation)
+      k = new ClientKarma(socket, iframe, windowStub, windowNavigator, windowLocation)
       clientWindow = { karma: k }
       ck = new ContextKarma(ContextKarma.getDirectCallParentKarmaMethod(clientWindow))
       ck.config = {}
 
       sinon.spy(socket, 'disconnect')
-
-      socket.on('complete', function (data, ack) {
-        ack()
-      })
+      clock.tick(500)
 
       ck.complete()
-
-      clock.tick(500)
-      setTimeout(function () {
+      setTimeout(() => {
         assert(windowLocation.href === 'http://return.com')
         done()
       }, 5)
+
       clock.tick(10)
     })