fix(test): clear up clearContext (#3597)

johnjbarton · web-flow · commit 8997b7465de3 · 2020-12-23T08:46:44.000-08:00
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'.
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=https://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 === 'https://return.com')
         done()
       }, 5)
+
       clock.tick(10)
     })
 
