From a3d1f11542eab55928bbb0cf4f21cfe34b603006 Mon Sep 17 00:00:00 2001 From: Yaroslav Admin Date: Tue, 5 May 2020 23:40:46 +0200 Subject: [PATCH] refactor(test): add common method to start server in background (#3495) The existing code had pretty confusing logic (old lines 33-36)`: when background server start fails it is actually considered a success, but child process is not started. Because of this `lastRun` contains result output of the `karma start`. This is no longer the case, hence two test cases had to be updated as they never executed `karma run` despite the statement and were asserting against `karma start` output. This should be more clear now. As background server process now has its own variable to store output, there is no need for a dedicated runOut command and it can be removed. --- test/e2e/browser_console.feature | 2 +- test/e2e/error.feature | 2 +- test/e2e/launcher-error.feature | 2 +- test/e2e/pass-opts.feature | 2 +- test/e2e/step_definitions/core_steps.js | 78 +++++++------------------ test/e2e/step_definitions/hooks.js | 7 +-- test/e2e/support/world.js | 60 +++++++++++++++++++ test/e2e/timeout.feature | 2 +- 8 files changed, 88 insertions(+), 67 deletions(-) diff --git a/test/e2e/browser_console.feature b/test/e2e/browser_console.feature index df2517110..24dc977ed 100644 --- a/test/e2e/browser_console.feature +++ b/test/e2e/browser_console.feature @@ -183,7 +183,7 @@ Feature: Browser Console Configuration ]; singleRun = false; """ - When I runOut Karma + When I run Karma Then it passes with like: """ LOG: 'foo' diff --git a/test/e2e/error.feature b/test/e2e/error.feature index 1426266d2..3fd66e173 100644 --- a/test/e2e/error.feature +++ b/test/e2e/error.feature @@ -29,7 +29,7 @@ Feature: Error Display ]; singleRun = false; """ - When I runOut Karma + When I run Karma Then it fails with like: """ SyntaxError: Unexpected token '}' diff --git a/test/e2e/launcher-error.feature b/test/e2e/launcher-error.feature index 7d6e0cd55..2fa15e554 100644 --- a/test/e2e/launcher-error.feature +++ b/test/e2e/launcher-error.feature @@ -13,7 +13,7 @@ Feature: Launcher error 'karma-script-launcher' ]; """ - When I run Karma + When I start Karma Then it fails with like: """ Missing fake dependency diff --git a/test/e2e/pass-opts.feature b/test/e2e/pass-opts.feature index 7723839c8..f8055bb26 100644 --- a/test/e2e/pass-opts.feature +++ b/test/e2e/pass-opts.feature @@ -15,7 +15,7 @@ Feature: Passing Options singleRun = false; """ And command line arguments of: "-- arg1 arg2" - When I runOut Karma + When I run Karma Then it passes with no debug: """ . diff --git a/test/e2e/step_definitions/core_steps.js b/test/e2e/step_definitions/core_steps.js index f27a4b812..c9c2e5f7b 100644 --- a/test/e2e/step_definitions/core_steps.js +++ b/test/e2e/step_definitions/core_steps.js @@ -1,7 +1,7 @@ const { defineParameterType, Given, Then, When } = require('cucumber') const fs = require('fs') const path = require('path') -const { exec, spawn } = require('child_process') +const { exec } = require('child_process') const stopper = require('../../../lib/stopper') let additionalArgs = [] @@ -21,29 +21,10 @@ function execKarma (command, level, callback) { }, done) } - const runOut = command === 'runOut' - if (command === 'run' || command === 'runOut') { - let isRun = false - this.child = spawn('' + runtimePath, ['start', '--log-level', 'warn', configFile]) - const done = () => { - this.child && this.child.kill() - callback() - } - - this.child.on('error', (error) => { - this.lastRun.error = error - done() - }) - - this.child.stderr.on('data', (chunk) => { - this.lastRun.stderr += chunk.toString() - }) - - this.child.stdout.on('data', (chunk) => { - this.lastRun.stdout += chunk.toString() - const cmd = runtimePath + ' run ' + configFile + ' ' + additionalArgs - if (!isRun) { - isRun = true + if (command === 'run') { + this.runBackgroundProcess(['start', '--log-level', 'warn', configFile]) + .then(() => { + const cmd = runtimePath + ' run ' + configFile + ' ' + additionalArgs setTimeout(() => { exec(cmd, { @@ -52,15 +33,13 @@ function execKarma (command, level, callback) { if (error) { this.lastRun.error = error } - if (runOut) { - this.lastRun.stdout = stdout - this.lastRun.stderr = stderr - } - done() + this.lastRun.stdout = stdout + this.lastRun.stderr = stderr + callback() }) }, 1000) - } - }) + }) + .catch((error) => callback(error)) } else { executor((error, stdout, stderr) => { if (error) { @@ -100,24 +79,13 @@ When('I stop a server programmatically', function (callback) { }, 1000) }) -When('I start a server in background', function (callback) { - this.writeConfigFile() - - const configFile = this.configFile - const runtimePath = this.karmaExecutable - this.child = spawn(runtimePath, ['start', '--log-level', 'debug', configFile]) - this.child.stdout.on('data', () => { - callback() - callback = () => null - }) - this.child.on('exit', (exitCode) => { - this.childExitCode = exitCode - }) +When('I start a server in background', async function () { + await this.runBackgroundProcess(['start', '--log-level', 'debug', this.configFile]) }) defineParameterType({ name: 'command', - regexp: /run|runOut|start|init|stop/ + regexp: /run|start|init|stop/ }) defineParameterType({ @@ -190,17 +158,15 @@ Then('it fails with like:', function (expectedOutput, callback) { } }) -Then(/^The (server|stopper) is dead(:? with exit code (\d+))?$/, - function (stopperOrServer, code, callback) { - const server = stopperOrServer === 'server' - const _this = this - setTimeout(function () { - const actualExitCode = server ? _this.childExitCode : _this.stopperExitCode - if (actualExitCode === undefined) return callback(new Error('Server has not exited.')) - if (code === undefined || parseInt(code, 10) === actualExitCode) return callback() - callback(new Error('Exit-code mismatch')) - }, 4000) - }) +Then(/^The (server|stopper) is dead(:? with exit code (\d+))?$/, function (stopperOrServer, code, callback) { + const server = stopperOrServer === 'server' + setTimeout(() => { + const actualExitCode = server ? this.backgroundProcess.handle.exitCode : this.stopperExitCode + if (actualExitCode === undefined) return callback(new Error('Server has not exited.')) + if (code === undefined || parseInt(code, 10) === actualExitCode) return callback() + callback(new Error('Exit-code mismatch')) + }, 4000) +}) Then(/^the file at ([a-zA-Z0-9/\\_.]+) contains:$/, function (filePath, expectedOutput) { const data = fs.readFileSync(path.join(this.workDir, filePath), 'utf8') diff --git a/test/e2e/step_definitions/hooks.js b/test/e2e/step_definitions/hooks.js index 8b0f99d2f..9b1b49bf6 100644 --- a/test/e2e/step_definitions/hooks.js +++ b/test/e2e/step_definitions/hooks.js @@ -6,10 +6,5 @@ Before(function () { After(async function () { await this.proxy.stopIfRunning() - - const running = this.child != null && typeof this.child.kill === 'function' - if (running) { - this.child.kill() - this.child = null - } + await this.stopBackgroundProcessIfRunning() }) diff --git a/test/e2e/support/world.js b/test/e2e/support/world.js index dd7edcb64..ecae024e9 100644 --- a/test/e2e/support/world.js +++ b/test/e2e/support/world.js @@ -1,3 +1,4 @@ +const { spawn } = require('child_process') const fs = require('fs') const vm = require('vm') const path = require('path') @@ -56,6 +57,12 @@ class World { stdout: '', stderr: '' } + + this.backgroundProcess = { + handle: null, + stdout: '', + stderr: '' + } } updateConfig (configOverrides) { @@ -85,6 +92,59 @@ module.exports = (config) => { rimraf.sync(this.sandboxDir) mkdirp.sync(this.sandboxDir) } + + async runBackgroundProcess (args, readyOutput = null) { + return new Promise((resolve, reject) => { + const handle = this.backgroundProcess.handle = spawn(this.karmaExecutable, args, { cwd: this.workDir }) + + let isSettled = false + + // The errorHandler only handles spawn errors (e.g. invalid executable + // path). It is removed once process has spawned. Kill errors + // handling is done in {@link stopBackgroundProcessIfRunning}. + // See https://nodejs.org/api/child_process.html#child_process_event_error + // + // This is because the Cucumber step to start the background process is + // considered successful once first output from the spawned process has + // been received. Then Karma server is running in the background (while + // subsequent Cucumber steps are executed) and there is no way to mark + // Cucumber step that started a server as failed since it has already + // completed. + const errorHandler = (error) => { + isSettled = true + this.backgroundProcess.handle = null + reject(error) + } + handle.once('error', errorHandler) + + handle.stderr.on('data', (chunk) => { + this.backgroundProcess.stderr += chunk.toString() + }) + + handle.stdout.on('data', (chunk) => { + this.backgroundProcess.stdout += chunk.toString() + + if (!isSettled) { + if (readyOutput == null || this.backgroundProcess.stdout.includes(readyOutput)) { + isSettled = true + handle.off('error', errorHandler) + resolve() + } + } + }) + }) + } + + async stopBackgroundProcessIfRunning () { + if (this.backgroundProcess.handle != null && this.backgroundProcess.handle.exitCode == null) { + return new Promise((resolve, reject) => { + this.backgroundProcess.handle + .once('exit', () => resolve()) + .once('error', (error) => reject(error)) + .kill() + }) + } + } } setWorldConstructor(World) diff --git a/test/e2e/timeout.feature b/test/e2e/timeout.feature index 6b5aa5543..fc1eb84eb 100644 --- a/test/e2e/timeout.feature +++ b/test/e2e/timeout.feature @@ -14,7 +14,7 @@ Feature: Timeout ]; captureTimeout = 100 """ - When I run Karma + When I start Karma Then it fails with like: """ have not captured in 100 ms