From 074969bf812c0a087dcebab59872011fbbc577c6 Mon Sep 17 00:00:00 2001 From: Yaroslav Admin Date: Sun, 19 Apr 2020 15:56:39 +0200 Subject: [PATCH] refactor(test): adjust sandbox folder location and simplify config logic This is a bit bigger change, but modifications are closely related and I think it makes sense to land them all together. The main change is in test/e2e/support/world.js file, where all important paths are defined: - `workDir` is the current working directory for the tested Karma process - `test/e2e/support`. Same as before. - `sandboxDir` moved from `tmp/sandbox` in the repository root to `test/e2e/support/sandbox`. Sandbox directory is reset before every scenario (see `test/e2e/step_definitions/hooks.js`) and this is where all generated files should be (hence change of paths in `browser_console.feature`). This also makes things cleaner as now all paths in the .feature files are relative to `workDir`. - `configFile` is path where Karma config file is generated. It no longer contains hash as it was not really needed. It may be re-introduced later if tests are run in parallel, but then it makes more sense to have it as a part of `sandboxDir` instead. - `karmaExecutable` is an absolute path to the Karma executable. The change from `__dirname + '/` to `_resolve('` in two .feature files is to avoid having two different hacks to resolve absolute paths. The last change in this file is to simplify config generation methods. Remaining changes in `test/e2e/step_definitions/core_steps.js` are adapting steps to the above changes by removing bunch of boilerplate, which is no longer necessary. --- .eslintignore | 2 +- .gitignore | 3 +- test/e2e/browser_console.feature | 18 +-- test/e2e/launcher-error.feature | 2 +- test/e2e/step_definitions/core_steps.js | 193 ++++++++++-------------- test/e2e/step_definitions/hooks.js | 6 +- test/e2e/support/world.js | 90 +++++++---- test/e2e/timeout.feature | 2 +- 8 files changed, 155 insertions(+), 161 deletions(-) diff --git a/.eslintignore b/.eslintignore index 93626b9ee..94013a079 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,5 +1,5 @@ +test/e2e/support/sandbox test/e2e/support/error/under-test.js test/unit/fixtures/bundled.js static/karma.js static/context.js -tmp/* diff --git a/.gitignore b/.gitignore index 52a47db45..616e0d81c 100644 --- a/.gitignore +++ b/.gitignore @@ -4,11 +4,10 @@ static/context.js static/karma.js .idea/* *.iml -tmp/* docs/_build *.swp *.swo -test/e2e/console.log +test/e2e/support/sandbox test/e2e/coverage/coverage test/e2e/coverageQunit/coverage test/e2e/coverageRequirejs/coverage diff --git a/test/e2e/browser_console.feature b/test/e2e/browser_console.feature index 409313b31..df2517110 100644 --- a/test/e2e/browser_console.feature +++ b/test/e2e/browser_console.feature @@ -50,12 +50,12 @@ Feature: Browser Console Configuration 'karma-chrome-launcher' ]; browserConsoleLogOptions = { - path: 'test/e2e/console.log', + path: 'sandbox/console.log', format: '%t:%m' }; """ When I start Karma - Then the file at test/e2e/console.log contains: + Then the file at sandbox/console.log contains: """ log:'foo' debug:'bar' @@ -74,12 +74,12 @@ Feature: Browser Console Configuration 'karma-chrome-launcher' ]; browserConsoleLogOptions = { - path: 'test/e2e/console.log', + path: 'sandbox/console.log', format: '%t:%T:%m' }; """ When I start Karma - Then the file at test/e2e/console.log contains: + Then the file at sandbox/console.log contains: """ log:LOG:'foo' debug:DEBUG:'bar' @@ -98,13 +98,13 @@ Feature: Browser Console Configuration 'karma-chrome-launcher' ]; browserConsoleLogOptions = { - path: 'test/e2e/console.log', + path: 'sandbox/console.log', format: '%t:%T:%m', level: 'warn' }; """ When I start Karma - Then the file at test/e2e/console.log contains: + Then the file at sandbox/console.log contains: """ log:LOG:'foo' warn:WARN:'foobar' @@ -121,12 +121,12 @@ Feature: Browser Console Configuration 'karma-chrome-launcher' ]; browserConsoleLogOptions = { - path: 'test/e2e/console.log', + path: 'sandbox/console.log', format: '%b' }; """ When I start Karma - Then the file at test/e2e/console.log contains: + Then the file at sandbox/console.log contains: """ Chrome Headless """ @@ -141,7 +141,7 @@ Feature: Browser Console Configuration 'karma-chrome-launcher' ]; browserConsoleLogOptions = { - path: 'test/e2e/console.log', + path: 'sandbox/console.log', format: '%b', terminal: false }; diff --git a/test/e2e/launcher-error.feature b/test/e2e/launcher-error.feature index 35ffa2a11..7d6e0cd55 100644 --- a/test/e2e/launcher-error.feature +++ b/test/e2e/launcher-error.feature @@ -7,7 +7,7 @@ Feature: Launcher error Given a configuration with: """ files = ['launcher-error/specs.js']; - browsers = [__dirname + '/launcher-error/fake-browser.sh']; + browsers = [_resolve('launcher-error/fake-browser.sh')]; plugins = [ 'karma-jasmine', 'karma-script-launcher' diff --git a/test/e2e/step_definitions/core_steps.js b/test/e2e/step_definitions/core_steps.js index 5f8d4bbab..16f0456a1 100644 --- a/test/e2e/step_definitions/core_steps.js +++ b/test/e2e/step_definitions/core_steps.js @@ -2,105 +2,81 @@ const { defineParameterType, Given, Then, When } = require('cucumber') const fs = require('fs') const path = require('path') const { exec, spawn } = require('child_process') -const rimraf = require('rimraf') const stopper = require('../../../lib/stopper') -const baseDir = fs.realpathSync(path.join(__dirname, '/../../..')) -const tmpDir = path.join(baseDir, 'tmp', 'sandbox') -const tmpConfigFile = 'karma.conf.js' -let cleansingNeeded = true let additionalArgs = [] -function cleanseIfNeeded () { - if (cleansingNeeded) { - try { - rimraf.sync(tmpDir) - } catch (e) { - } +function execKarma (command, level, callback) { + level = level || 'warn' - cleansingNeeded = false + this.writeConfigFile() - return cleansingNeeded - } -} + const configFile = this.configFile + const runtimePath = this.karmaExecutable + const baseDir = this.workDir -function execKarma (command, level, callback) { - level = level || 'warn' + const executor = (done) => { + const cmd = runtimePath + ' ' + command + ' --log-level ' + level + ' ' + configFile + ' ' + additionalArgs - this.writeConfigFile(tmpDir, tmpConfigFile, (err, hash) => { - if (err) { - return callback.fail(new Error(err)) + return exec(cmd, { + cwd: baseDir + }, 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() } - const configFile = path.join(tmpDir, hash + '.' + tmpConfigFile) - const runtimePath = path.join(baseDir, 'bin', 'karma') - const executor = (done) => { - const cmd = runtimePath + ' ' + command + ' --log-level ' + level + ' ' + configFile + ' ' + additionalArgs + this.child.on('error', (error) => { + this.lastRun.error = error + done() + }) - return exec(cmd, { - cwd: baseDir - }, done) - } + this.child.stderr.on('data', (chunk) => { + this.lastRun.stderr += chunk.toString() + }) - const runOut = command === 'runOut' - if (command === 'run' || command === 'runOut') { - let isRun = false - this.child = spawn('' + runtimePath, ['start', '--log-level', 'warn', configFile]) - const done = () => { - cleansingNeeded = true - this.child && this.child.kill() - callback() + this.child.stdout.on('data', (chunk) => { + this.lastRun.stdout += chunk.toString() + const cmd = runtimePath + ' run ' + configFile + ' ' + additionalArgs + if (!isRun) { + isRun = true + + setTimeout(() => { + exec(cmd, { + cwd: baseDir + }, (error, stdout, stderr) => { + if (error) { + this.lastRun.error = error + } + if (runOut) { + this.lastRun.stdout = stdout + this.lastRun.stderr = stderr + } + done() + }) + }, 1000) } - - this.child.on('error', (error) => { + }) + } else { + executor((error, stdout, stderr) => { + if (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 - - setTimeout(() => { - exec(cmd, { - cwd: baseDir - }, (error, stdout, stderr) => { - if (error) { - this.lastRun.error = error - } - if (runOut) { - this.lastRun.stdout = stdout - this.lastRun.stderr = stderr - } - done() - }) - }, 1000) - } - }) - } else { - executor((error, stdout, stderr) => { - if (error) { - this.lastRun.error = error - } - this.lastRun.stdout = stdout - this.lastRun.stderr = stderr - cleansingNeeded = true - callback() - }) - } - }) + } + this.lastRun.stdout = stdout + this.lastRun.stderr = stderr + callback() + }) + } } -Given('a configuration with:', function (fileContent, callback) { - cleanseIfNeeded() - this.addConfigContent(fileContent) - return callback() +Given('a configuration with:', function (fileContent) { + this.updateConfig(fileContent) }) Given('command line arguments of: {string}', function (args, callback) { @@ -113,34 +89,27 @@ Given('a proxy on port {int} that prepends {string} to the base path', async fun }) When('I stop a server programmatically', function (callback) { - const _this = this - setTimeout(function () { - stopper.stop(_this.configFile, function (exitCode) { - _this.stopperExitCode = exitCode + setTimeout(() => { + stopper.stop(this.config, (exitCode) => { + this.stopperExitCode = exitCode callback() }) }, 1000) }) When('I start a server in background', function (callback) { - this.writeConfigFile(tmpDir, tmpConfigFile, (function (_this) { - return function (err, hash) { - if (err) { - return callback.fail(new Error(err)) - } - const configFile = path.join(tmpDir, hash + '.' + tmpConfigFile) - const runtimePath = path.join(baseDir, 'bin', 'karma') - _this.child = spawn('' + runtimePath, ['start', '--log-level', 'debug', configFile]) - _this.child.stdout.on('data', function () { - callback() - callback = function () { - } - }) - _this.child.on('exit', function (exitCode) { - _this.childExitCode = exitCode - }) - } - })(this)) + 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 + }) }) defineParameterType({ @@ -230,11 +199,9 @@ Then(/^The (server|stopper) is dead(:? with exit code (\d+))?$/, }, 4000) }) -Then(/^the file at ([a-zA-Z0-9/\\_.]+) contains:$/, - function (filePath, expectedOutput, callback) { - const data = fs.readFileSync(filePath, { encoding: 'UTF-8' }) - if (data.match(expectedOutput)) { - return callback() - } - callback(new Error('Expected output to match the following:\n ' + expectedOutput + '\nGot:\n ' + data)) - }) +Then(/^the file at ([a-zA-Z0-9/\\_.]+) contains:$/, function (filePath, expectedOutput) { + const data = fs.readFileSync(path.join(this.workDir, filePath), 'utf8') + if (!data.match(expectedOutput)) { + throw new Error('Expected output to match the following:\n ' + expectedOutput + '\nGot:\n ' + data) + } +}) diff --git a/test/e2e/step_definitions/hooks.js b/test/e2e/step_definitions/hooks.js index 12ef627a2..8b0f99d2f 100644 --- a/test/e2e/step_definitions/hooks.js +++ b/test/e2e/step_definitions/hooks.js @@ -1,4 +1,8 @@ -const { After } = require('cucumber') +const { After, Before } = require('cucumber') + +Before(function () { + this.ensureSandbox() +}) After(async function () { await this.proxy.stopIfRunning() diff --git a/test/e2e/support/world.js b/test/e2e/support/world.js index ef1e4c339..dd7edcb64 100644 --- a/test/e2e/support/world.js +++ b/test/e2e/support/world.js @@ -1,25 +1,54 @@ const fs = require('fs') const vm = require('vm') const path = require('path') -const hasher = require('crypto').createHash const mkdirp = require('mkdirp') -const _ = require('lodash') +const rimraf = require('rimraf') const { setWorldConstructor } = require('cucumber') const Proxy = require('./proxy') class World { constructor () { this.proxy = new Proxy() - this.template = _.template(`process.env.CHROME_BIN = require('puppeteer').executablePath(); module.exports = function (config) {\n config.set(\n <%= content %>\n );\n};`) - this.configFile = { + /** + * The current working directory path for all Karma commands. + * @type {string} + */ + this.workDir = fs.realpathSync(__dirname) + + /** + * The directory where all files generated during tests are stored. + * It is removed after each scenario. + * @type {string} + */ + this.sandboxDir = path.join(this.workDir, 'sandbox') + + /** + * Path to the final Karma config file. + * @type {string} + */ + this.configFile = path.join(this.sandboxDir, 'karma.conf.js') + + /** + * Absolute path to the Karma executable. + * @type {string} + */ + this.karmaExecutable = fs.realpathSync(path.join(__dirname, '../../../bin/karma')) + + this.config = { singleRun: true, reporters: ['dots'], frameworks: ['jasmine'], - basePath: __dirname, + basePath: this.workDir, colors: false, - __dirname: __dirname, - _resolve: (name) => path.resolve(__dirname, '..', 'support', name) + // Current approach uses vm.runInNewContext() method to apply + // configuration overrides. With this approach config object is used as an + // evaluation context and as result none of the regular node module + // variables (e.g. require, __dirname) are accessible. + // This requires hacks as below to support path resolution. It should be + // better to expose regular node module variables to the evaluation + // scope without polluting the config object. + _resolve: (name) => path.resolve(this.workDir, name) } this.lastRun = { @@ -29,37 +58,32 @@ class World { } } - addConfigContent (content) { - if (content == null) { - content = '' - } - return vm.runInNewContext(content, this.configFile) + updateConfig (configOverrides) { + vm.runInNewContext(configOverrides, this.config) } - writeConfigFile (dir, file, done) { - return mkdirp(dir, 0x1ed, (err) => { - let content, hash - if (err) { - return done(err) - } + writeConfigFile () { + delete this.config._resolve + + const config = JSON.stringify(Object.assign({}, this.config, { + customLaunchers: Object.assign({ + ChromeHeadlessNoSandbox: { base: 'ChromeHeadless', flags: ['--no-sandbox'] } + }, this.config.customLaunchers) + })) + + const content = `process.env.CHROME_BIN = require('puppeteer').executablePath(); + +module.exports = (config) => { + config.set(${config}); +}; +` - delete this.configFile.__dirname - content = this.generateJS(this.configFile) - hash = hasher('md5').update(content + Math.random()).digest('hex') - fs.writeFile(path.join(dir, hash + '.' + file), content, (err) => { - done(err, hash) - }) - }) + fs.writeFileSync(this.configFile, content) } - generateJS (config) { - return this.template({ - content: JSON.stringify(Object.assign({}, config, { - customLaunchers: Object.assign({ - ChromeHeadlessNoSandbox: { base: 'ChromeHeadless', flags: ['--no-sandbox'] } - }, config.customLaunchers) - })) - }) + ensureSandbox () { + rimraf.sync(this.sandboxDir) + mkdirp.sync(this.sandboxDir) } } diff --git a/test/e2e/timeout.feature b/test/e2e/timeout.feature index 43d292c6f..6b5aa5543 100644 --- a/test/e2e/timeout.feature +++ b/test/e2e/timeout.feature @@ -7,7 +7,7 @@ Feature: Timeout Given a configuration with: """ files = ['timeout/specs.js']; - browsers = [__dirname + '/timeout/fake-browser.sh']; + browsers = [_resolve('timeout/fake-browser.sh')]; plugins = [ 'karma-jasmine', 'karma-script-launcher'