From 082c407a257fe0c5bfbbad8c043c94a2f6e0fa86 Mon Sep 17 00:00:00 2001 From: Pierre-Marie Dartus Date: Wed, 3 Feb 2021 11:18:21 +0100 Subject: [PATCH] refactor: Simplify test-runner logic (#196) --- bin/sfdx-lwc-jest | 12 ++++- package.json | 2 +- src/config.js | 9 +--- src/log.js | 3 +- src/resolver.js | 4 +- src/utils/jest.js | 13 ------ src/utils/project.js | 2 - src/utils/shell.js | 31 ------------- src/utils/test-runner.js | 93 ++++++++++++++++++++----------------- src/utils/yargs.js | 2 +- tests/help.test.js | 21 ++------- tests/test-runner.test.js | 97 ++++++++++++++++++++++----------------- tests/utils/shell.test.js | 74 ----------------------------- 13 files changed, 129 insertions(+), 234 deletions(-) delete mode 100644 src/utils/jest.js delete mode 100644 src/utils/shell.js delete mode 100644 tests/utils/shell.test.js diff --git a/bin/sfdx-lwc-jest b/bin/sfdx-lwc-jest index 7deedb92..b658e028 100755 --- a/bin/sfdx-lwc-jest +++ b/bin/sfdx-lwc-jest @@ -13,8 +13,16 @@ const { error } = require('../src/log'); const args = getArgs(); const runJest = require('../src/utils/test-runner'); -runJest(args); -process.on('unhandledRejection', reason => { +runJest(args) + .catch((err) => { + error(err); + return 1; + }) + .then((code) => { + process.exit(code); + }); + +process.on('unhandledRejection', (reason) => { error(reason); }); diff --git a/package.json b/package.json index f4c737ef..d6a3a05f 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ }, "scripts": { "check-license-headers": "node ./scripts/checkLicenseHeaders.js", - "lint": "eslint src/", + "lint": "eslint src/ tests/", "format": "prettier --write '**/*.{js,json,md,html,css}'", "format:check": "prettier --check '**/*.{js,json,md,html,css}'", "release": "npm publish --access public", diff --git a/src/config.js b/src/config.js index 2dd1aa77..568450c5 100644 --- a/src/config.js +++ b/src/config.js @@ -37,11 +37,4 @@ const jestConfig = { const expectedApiVersion = '50.0'; -const jestPath = (() => { - const packageJsonPath = require.resolve('jest/package.json'); - const { bin } = require(packageJsonPath); - - return path.resolve(path.dirname(packageJsonPath), bin); -})(); - -module.exports = { jestConfig, jestPath, expectedApiVersion }; +module.exports = { jestConfig, expectedApiVersion }; diff --git a/src/log.js b/src/log.js index a68204f2..99044caf 100644 --- a/src/log.js +++ b/src/log.js @@ -14,9 +14,8 @@ function info(message) { console.log(`${chalk.blue('info')} ${message}`); } -function error(message, code = 1) { +function error(message) { console.error(`${chalk.red('error')} ${message}`); - process.exit(code); } module.exports = { diff --git a/src/resolver.js b/src/resolver.js index 6e18ed4a..43008852 100644 --- a/src/resolver.js +++ b/src/resolver.js @@ -10,10 +10,12 @@ const fs = require('fs'); const path = require('path'); const lwcResolver = require('@lwc/jest-resolver'); -const { PROJECT_ROOT, getModulePaths, DEFAULT_NAMESPACE } = require('./utils/project.js'); +const { PROJECT_ROOT, getModulePaths } = require('./utils/project.js'); const { getInfoFromId } = require('./utils/module.js'); +const DEFAULT_NAMESPACE = 'c'; + function isFile(file) { let result; diff --git a/src/utils/jest.js b/src/utils/jest.js deleted file mode 100644 index 091beb05..00000000 --- a/src/utils/jest.js +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -'use strict'; - -// For testing purposes -- we can't mock "jest" itself, -// so we injecting it with a different name. -module.exports = { - jestRunner: require('jest'), -}; diff --git a/src/utils/project.js b/src/utils/project.js index cd9dcaa5..5adf11aa 100644 --- a/src/utils/project.js +++ b/src/utils/project.js @@ -11,7 +11,6 @@ const path = require('path'); const fg = require('fast-glob'); const PROJECT_ROOT = fs.realpathSync(process.cwd()); -const DEFAULT_NAMESPACE = 'c'; let PATHS = []; @@ -40,6 +39,5 @@ function getModulePaths() { module.exports = { PROJECT_ROOT, getSfdxProjectJson, - DEFAULT_NAMESPACE, getModulePaths, }; diff --git a/src/utils/shell.js b/src/utils/shell.js deleted file mode 100644 index 8f634b56..00000000 --- a/src/utils/shell.js +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (c) 2018, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -'use strict'; - -const spawn = require('child_process').spawn; -const { error, info } = require('../log'); - -const runCommand = (command, args) => { - const jestProcess = spawn(command, args); - jestProcess.on('error', (err) => { - error('error', err); - }); - - jestProcess.stdout.on('data', function (data) { - info('stdout: ' + String(data)); - }); - - jestProcess.stderr.on('data', function (data) { - info('stderr: ' + String(data)); - }); - - jestProcess.on('exit', function (code) { - info('Exited with code ' + String(code)); - }); -}; - -module.exports = { runCommand }; diff --git a/src/utils/test-runner.js b/src/utils/test-runner.js index 744d5d22..82a456ed 100644 --- a/src/utils/test-runner.js +++ b/src/utils/test-runner.js @@ -8,34 +8,15 @@ const fs = require('fs'); const path = require('path'); -const { jestRunner } = require('./jest'); -const shell = require('./shell'); - -const { error, info } = require('../log'); +const { spawn } = require('child_process'); const { PROJECT_ROOT, getSfdxProjectJson } = require('./project'); -const { jestConfig, expectedApiVersion, jestPath } = require('../config'); - -// CLI options we do not want to pass along to Jest -// prettier-ignore -const OPTIONS_DISALLOW_LIST = [ - '_', - '$0', - 'debug', 'd', - 'skipApiVersionCheck', 'skip-api-version-check' -]; - -function getOptions(argv) { - let options = []; +const { error, info } = require('../log'); +const { jestConfig, expectedApiVersion } = require('../config'); - Object.keys(argv).forEach((arg) => { - if (argv[arg] && !OPTIONS_DISALLOW_LIST.includes(arg)) { - options.push(`--${arg}`); - } - }); - return options.concat(argv._); -} +// List of CLI options that should be passthrough to jest. +const JEST_PASSTHROUGH_OPTIONS = new Set(['coverage', 'updateSnapshot', 'verbose', 'watch']); function validSourceApiVersion() { const sfdxProjectJson = getSfdxProjectJson(); @@ -47,33 +28,61 @@ function validSourceApiVersion() { } } +function getJestPath() { + const packageJsonPath = require.resolve('jest/package.json'); + + const { bin } = require(packageJsonPath); + return path.resolve(path.dirname(packageJsonPath), bin); +} + +function getJestArgs(argv) { + // Extract known options from parsed arguments + const knownOptions = Object.keys(argv) + .filter((key) => argv[key] && JEST_PASSTHROUGH_OPTIONS.has(key)) + .map((key) => `--${key}`); + + // Extract remaining options after `--` + const rest = argv._; + + const jestArgs = [...knownOptions, ...rest]; + + // Force jest to run in band when debugging. + if (argv.debug) { + jestArgs.unshift('--runInBand'); + } + + // Provide default configuration when none is present at the project root. + const hasCustomConfig = fs.existsSync(path.resolve(PROJECT_ROOT, 'jest.config.js')); + if (!hasCustomConfig) { + jestArgs.unshift(`--config=${JSON.stringify(jestConfig)}`); + } + + return jestArgs; +} + async function testRunner(argv) { if (!argv.skipApiVersionCheck) { validSourceApiVersion(); } - const hasCustomConfig = fs.existsSync(path.resolve(PROJECT_ROOT, 'jest.config.js')); - const config = hasCustomConfig ? [] : ['--config', JSON.stringify(jestConfig)]; + const spawnCommand = 'node'; + const spawnArgs = [getJestPath(), ...getJestArgs(argv)]; - const options = getOptions(argv); if (argv.debug) { + spawnArgs.unshift('--inspect-brk'); + info('Running in debug mode...'); - let commandArgs = ['--inspect-brk', jestPath, '--runInBand']; - commandArgs = commandArgs.concat(options); - if (!hasCustomConfig) { - commandArgs.push('--config=' + JSON.stringify(jestConfig)); - } - const command = 'node'; - info(command + ' ' + commandArgs.join(' ')); - - shell.runCommand(command, commandArgs); - } else { - // Jest will not set the env if not run from the bin executable - if (process.env.NODE_ENV == null) { - process.env.NODE_ENV = 'test'; - } - jestRunner.run([...config, ...options]); + info(`${spawnCommand} ${spawnArgs.join(' ')}`); } + + return new Promise((resolve) => { + const jest = spawn(spawnCommand, spawnArgs, { + env: process.env, + stdio: 'inherit', + }); + + jest.on('close', (code) => resolve(code)); + }); } module.exports = testRunner; diff --git a/src/utils/yargs.js b/src/utils/yargs.js index 46922e57..bf30077d 100644 --- a/src/utils/yargs.js +++ b/src/utils/yargs.js @@ -6,10 +6,10 @@ */ 'use strict'; -const options = require('../options/options'); const yargs = require('yargs'); const { error, info } = require('../log'); +const options = require('../options/options'); const argError = (msg, err, yargs) => { if (err) { diff --git a/tests/help.test.js b/tests/help.test.js index 084f3d8a..36d48418 100644 --- a/tests/help.test.js +++ b/tests/help.test.js @@ -7,22 +7,11 @@ 'use strict'; const { exec } = require('child_process'); +const { promisify } = require('util'); -const runInHelpMode = () => { - return new Promise((resolve, reject) => { - exec('node ./bin/sfdx-lwc-jest --help', (error, stdout) => { - if (error) { - reject(error); - } else { - resolve(stdout); - } - }); - }); -}; +const promiseExec = promisify(exec); -test('--help attribute shows help', () => { - expect.assertions(1); - return runInHelpMode().then((stdout) => { - expect(stdout).toMatchSnapshot(); - }); +test('--help attribute shows help', async () => { + const { stdout } = await promiseExec('node ./bin/sfdx-lwc-jest --help'); + expect(stdout).toMatchSnapshot(); }); diff --git a/tests/test-runner.test.js b/tests/test-runner.test.js index 84f09b22..9abe89d1 100644 --- a/tests/test-runner.test.js +++ b/tests/test-runner.test.js @@ -6,56 +6,71 @@ */ 'use strict'; -const fakeJest = require('../src/utils/jest'); -const fakeShell = require('../src/utils/shell'); +const child_process = require('child_process'); +const runJest = require('../src/utils/test-runner'); + +jest.mock('child_process'); +jest.mock('../src/log'); jest.mock('../src/utils/project'); -jest.mock('../src/utils/jest'); -jest.mock('../src/utils/shell'); -const runJest = require('../src/utils/test-runner'); +beforeEach(() => { + child_process.spawn.mockReset(); -const { jestPath } = require('../src/config'); + child_process.spawn.mockImplementation(() => { + return { + on(name, cb) { + if (name === 'close') cb(0); + }, + }; + }); +}); -const generateArgs = ({ args = {}, advanced = [] } = {}) => { - args['_'] = advanced; - return args; -}; +test('invokes node executable with jest path', async () => { + await runJest({ _: [] }); + const [[cmd, args]] = child_process.spawn.mock.calls; -const defaultArgs = generateArgs(); + expect(cmd).toContain('node'); + expect(args[0]).toMatch(/bin\/jest\.js$/); +}); -test('advanced params get passed to Jest', () => { - const advancedParam = '--maxWorkers=4'; - const args = generateArgs({ advanced: [advancedParam] }); - let jestParams = []; - fakeJest.jestRunner.run = jest.fn((array) => { - jestParams = array; - }); - runJest(args); - expect(jestParams).toContain(advancedParam); +test('invokes jest with --runInBand and --inspect-brk in debug mode', async () => { + await runJest({ debug: true, _: [] }); + const [[, args]] = child_process.spawn.mock.calls; + + expect(args).toContain('--inspect-brk'); + expect(args).toContain('--runInBand'); }); -test('config is being passed', () => { - let jestParams = []; - fakeJest.jestRunner.run = jest.fn((array) => { - jestParams = array; - }); - runJest(defaultArgs); - expect(jestParams.length).toBe(2); - expect(jestParams).toContain('--config'); +test.each(['coverage', 'updateSnapshot', 'verbose', 'watch'])( + 'invokes jest with option %s when present', + async (option) => { + await runJest({ [option]: true, _: [] }); + const [[, args]] = child_process.spawn.mock.calls; + + expect(args).toContain(`--${option}`); + }, +); + +test('invokes jest with position arguments when present', async () => { + await runJest({ _: ['--showConfig', 'my/test.js'] }); + const [[, args]] = child_process.spawn.mock.calls; + + expect(args).toContain(`--showConfig`); + expect(args).toContain(`my/test.js`); }); -test('debug flag runs node debugger', () => { - const debugAttr = '--inspect-brk'; - const args = generateArgs({ args: { debug: true } }); - const shallArgs = {}; - fakeJest.jestRunner.run = jest.fn(); - fakeShell.runCommand = jest.fn((command, args) => { - shallArgs.command = command; - shallArgs.args = args; +test('resolves with jest command exit code', async () => { + const EXIT_CODE = 42; + child_process.spawn.mockImplementation(() => { + return { + on(name, cb) { + if (name === 'close') { + cb(EXIT_CODE); + } + }, + }; }); - runJest(args); - expect(fakeJest.jestRunner.run).not.toHaveBeenCalled(); - expect(shallArgs.args).toContain(debugAttr); - expect(shallArgs.args).toContain(jestPath); - expect(shallArgs.command).toBe('node'); + + const res = await runJest({ _: [] }); + expect(res).toBe(EXIT_CODE); }); diff --git a/tests/utils/shell.test.js b/tests/utils/shell.test.js deleted file mode 100644 index 9063a473..00000000 --- a/tests/utils/shell.test.js +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Copyright (c) 2019, salesforce.com, inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -'use strict'; - -const EventEmitter = require('events'); -const child_process = require('child_process'); -const shell = require('../../src/utils/shell'); -const log = require('../../src/log'); -const { runCommand } = shell; - -jest.mock('child_process'); -const processEventEmitter = new EventEmitter(); -const stdoutEmitter = new EventEmitter(); -const stderrEmitter = new EventEmitter(); -child_process.spawn.mockImplementation(() => { - const toReturn = processEventEmitter; - toReturn.stdout = stdoutEmitter; - toReturn.stderr = stderrEmitter; - return toReturn; -}); - -jest.mock('../../src/log', () => { - return { - info: jest.fn(), - error: jest.fn(), - }; -}); - -beforeEach(() => { - jest.clearAllMocks(); -}); - -const mockCommand = 'mockCommand'; -const mockArgs = []; - -test('Should log error with log level error on jestProcess error', () => { - runCommand(mockCommand, mockArgs); - const mockError = 'mock error'; - processEventEmitter.emit('error', mockError); - expect(log.error).toHaveBeenCalledWith('error', mockError); -}); - -test('Should log data with log level info on jestProcess stdout data', () => { - runCommand(mockCommand, mockArgs); - const mockData = 'mock data'; - stdoutEmitter.emit('data', mockData); - expect(log.info).toHaveBeenCalledWith('stdout: mock data'); -}); - -test('Should log data with log level info on jestProcess stderr data', () => { - runCommand(mockCommand, mockArgs); - const mockData = 'mock data'; - stderrEmitter.emit('data', mockData); - expect(log.info).toHaveBeenCalledWith('stderr: mock data'); -}); - -test('Should log exit code with log level info on jestProcess exit', () => { - runCommand(mockCommand, mockArgs); - const mockExitCode = '1'; - processEventEmitter.emit('exit', mockExitCode); - expect(log.info).toHaveBeenCalledWith('Exited with code 1'); -}); - -test('Should not throw error if exit code is null', () => { - runCommand(mockCommand, mockArgs); - expect(() => { - processEventEmitter.emit('exit', null); - }).not.toThrow(); - expect(log.info).toHaveBeenCalledWith('Exited with code null'); -});