From 8997b7465de3c5f7e436078b57acae98de1ce39a Mon Sep 17 00:00:00 2001 From: johnjbarton Date: Wed, 23 Dec 2020 08:46:44 -0800 Subject: [PATCH] fix(test): clear up clearContext (#3597) 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'. --- client/karma.js | 53 +++++++++++++++++---------------------- lib/server.js | 2 -- static/karma.js | 53 +++++++++++++++++---------------------- test/client/karma.spec.js | 12 +++------ 4 files changed, 50 insertions(+), 70 deletions(-) diff --git a/client/karma.js b/client/karma.js index 6790a77ac..4442589cd 100644 --- 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 index 4a3eecfca..229c21b19 100644 --- 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 index 0c289e0ca..4b791bc85 100644 --- 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 index a309b174f..567339f48 100644 --- 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) })