From f2ad665965207917ef3c7f98a117520aae3978f2 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Tue, 11 Aug 2020 21:30:07 -0700 Subject: [PATCH 1/5] === BEGIN fix-custom-config-coverage === Add coverage verification after test --- scripts/run-e2e-tests.mjs | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/scripts/run-e2e-tests.mjs b/scripts/run-e2e-tests.mjs index 7bc6456..e35a960 100644 --- a/scripts/run-e2e-tests.mjs +++ b/scripts/run-e2e-tests.mjs @@ -181,17 +181,22 @@ async function npmInstall(cwd, prefix) { */ async function setupTests(projectPath, prefix) { const name = path.basename(projectPath); - const log = (...msgs) => console.log(`${info(prefix)}`, ...msgs); + const log = (...msgs) => console.log(info(prefix), ...msgs); + const throwError = (...msgs) => { + process.exitCode = 1; + console.error(error(prefix), ...msgs.map((msg) => error(msg))); + throw new Error(msgs[0]); + }; log(`Beginning E2E test at`, projectPath); const pkgJsonPath = path.join(projectPath, 'package.json'); if (!(await fileExists(pkgJsonPath))) { prefix = error(prefix); - console.error( - `${prefix} Could not locate package.json for "${name}". Ensure every e2e test has a package.json defined.` + throwError( + `Could not locate package.json for "${name}".`, + `Ensure every e2e test has a package.json.\n`, + `${prefix} Expected to find one at "${pkgJsonPath}".` ); - console.error(`${prefix} Expected to find one at "${pkgJsonPath}".`); - throw new Error(`Could not locate package.json for "${name}".`); } const pkg = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')); @@ -228,10 +233,22 @@ async function setupTests(projectPath, prefix) { await onExit(cp); } catch (e) { process.exitCode = 1; - console.error(error(prefix) + ` Test run failed: ${e.message}`); + throwError(`Test run failed: ${e.message}`); } - // TODO: validate coverage/lcov.info is not empty + const coveragePath = path.join(projectPath, 'coverage', 'lcov.info'); + if (!(await fileExists)) { + throwError( + `Code coverage failed to generate results: Results file does not exist at ${coveragePath}` + ); + } + + const coverageInfo = await fs.readFile(coveragePath, { encoding: 'utf8' }); + if (!coverageInfo) { + throwError( + `Code coverage failed to generate results: Results file is empty` + ); + } }; } From 7d78ef43ec03311cd442a07d18119b2b1af209c3 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Tue, 11 Aug 2020 22:09:43 -0700 Subject: [PATCH 2/5] Add coverage as a seperate babel loader if config already contains a babel loader --- src/lib/babel-loader.js | 13 ++++++++++++- src/webpack.js | 35 ++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/lib/babel-loader.js b/src/lib/babel-loader.js index 028d696..fcc24f1 100644 --- a/src/lib/babel-loader.js +++ b/src/lib/babel-loader.js @@ -1,4 +1,15 @@ -export default function babelLoader(options) { +export function getCoverageBabelLoader() { + return { + test: /\.jsx?$/, + exclude: /node_modules/, + loader: require.resolve('babel-loader'), + query: { + plugins: [require.resolve('babel-plugin-istanbul')], + }, + }; +} + +export function getDefaultBabelLoader(options) { return { test: /\.jsx?$/, exclude: /node_modules/, diff --git a/src/webpack.js b/src/webpack.js index dc887fb..d44a0f4 100644 --- a/src/webpack.js +++ b/src/webpack.js @@ -1,7 +1,10 @@ import path from 'path'; import delve from 'dlv'; import { tryRequire, dedupe } from './lib/util'; -import babelLoader from './lib/babel-loader'; +import { + getDefaultBabelLoader, + getCoverageBabelLoader, +} from './lib/babel-loader'; import cssLoader from './lib/css-loader'; /** @@ -104,6 +107,21 @@ export function addWebpackConfig(karmaConfig, pkg, options) { return Object.assign({}, configured || {}, value); } + let existingBabelLoader = getLoader((rule) => + `${rule.use},${rule.loader}`.match(/\bbabel-loader\b/) + ); + if (existingBabelLoader) { + if (options.coverage) { + loaders.push(getCoverageBabelLoader()); + } + } else { + loaders.push(getDefaultBabelLoader(options)); + } + + if (!getLoader('foo.css')) { + loaders.push(cssLoader(options)); + } + for (let prop of Object.keys(karmaConfig.preprocessors)) { karmaConfig.preprocessors[prop].unshift('webpack'); } @@ -116,20 +134,7 @@ export function addWebpackConfig(karmaConfig, pkg, options) { mode: webpackConfig.mode || 'development', module: { // @TODO check webpack version and use loaders VS rules as the key here appropriately: - // - // TODO: Consider adding coverage as a separate babel-loader so that - // regardless if the user provides their own babel plugins, coverage still - // works - rules: loaders - .concat( - !getLoader((rule) => - `${rule.use},${rule.loader}`.match(/\bbabel-loader\b/) - ) - ? babelLoader(options) - : false, - !getLoader('foo.css') && cssLoader(options) - ) - .filter(Boolean), + rules: loaders, }, resolve: webpackProp('resolve', { modules: webpackProp('resolve.modules', [ From c0ad01f509f2764d9f6f504ce1b022535dab5f51 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Thu, 13 Aug 2020 22:19:39 -0700 Subject: [PATCH 3/5] Remove old TODO --- src/webpack.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/webpack.js b/src/webpack.js index d44a0f4..25eaf11 100644 --- a/src/webpack.js +++ b/src/webpack.js @@ -133,7 +133,6 @@ export function addWebpackConfig(karmaConfig, pkg, options) { // devtool: 'module-source-map', mode: webpackConfig.mode || 'development', module: { - // @TODO check webpack version and use loaders VS rules as the key here appropriately: rules: loaders, }, resolve: webpackProp('resolve', { From ff51d948a6ea5f01d175fddb5ad3ff04775cc2d6 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 14 Aug 2020 10:29:59 -0700 Subject: [PATCH 4/5] Run tests on all PRs and pushes to master --- .github/workflows/main.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 92b3ef3..1fef16d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,12 @@ name: CI -on: [push, pull_request] +on: + pull_request: + branches: + - '**' + push: + branches: + - master jobs: build: From e660b32a10ea68d862f64910ee4c4530e275b5a9 Mon Sep 17 00:00:00 2001 From: Andre Wiggins Date: Fri, 14 Aug 2020 16:42:29 -0700 Subject: [PATCH 5/5] Add code coverage babel plugin to existing babel loader --- e2e-test/webpack-custom/babel.config.js | 9 +++++++++ e2e-test/webpack-custom/package.json | 1 + e2e-test/webpack-custom/src/index.js | 9 +++++++-- e2e-test/webpack-custom/test/index.test.js | 22 ++++++++++++++++------ e2e-test/webpack-custom/webpack.config.js | 2 +- src/lib/babel-loader.js | 11 ----------- src/webpack.js | 22 +++++++++++++++------- 7 files changed, 49 insertions(+), 27 deletions(-) create mode 100644 e2e-test/webpack-custom/babel.config.js diff --git a/e2e-test/webpack-custom/babel.config.js b/e2e-test/webpack-custom/babel.config.js new file mode 100644 index 0000000..0d4806e --- /dev/null +++ b/e2e-test/webpack-custom/babel.config.js @@ -0,0 +1,9 @@ +module.exports = (api) => { + api.cache(true); + + return { + plugins: [ + ['@babel/plugin-transform-react-jsx', { pragma: 'createElement' }], + ], + }; +}; diff --git a/e2e-test/webpack-custom/package.json b/e2e-test/webpack-custom/package.json index b181fa6..4116f8c 100644 --- a/e2e-test/webpack-custom/package.json +++ b/e2e-test/webpack-custom/package.json @@ -4,6 +4,7 @@ "private": true, "dependencies": { "@babel/core": "^7.10.3", + "@babel/plugin-transform-react-jsx": "^7.10.3", "babel-loader": "8.1.0", "babel-plugin-transform-rename-properties": "^0.1.0", "karmatic": "file:../..", diff --git a/e2e-test/webpack-custom/src/index.js b/e2e-test/webpack-custom/src/index.js index 0dc0ae1..2e191f2 100644 --- a/e2e-test/webpack-custom/src/index.js +++ b/e2e-test/webpack-custom/src/index.js @@ -1,3 +1,8 @@ -export function box(value) { - return { _value: value }; +// eslint-disable-next-line no-unused-vars +function createElement(tag, props) { + return { tag, props }; +} + +export function render(value1, value2) { + return
; } diff --git a/e2e-test/webpack-custom/test/index.test.js b/e2e-test/webpack-custom/test/index.test.js index e55386a..025e0b6 100644 --- a/e2e-test/webpack-custom/test/index.test.js +++ b/e2e-test/webpack-custom/test/index.test.js @@ -1,9 +1,19 @@ -import { box } from '../src/index'; +import { render } from '../src/index'; -describe('Box', () => { - it('should have a __v property', () => { - const boxed = box(1); - expect('_value' in boxed).toBe(false); - expect(boxed.__v).toBe(1); +describe('Render result', () => { + it('should have a _value1 property', () => { + const result = render(1, 2); + expect('_value1' in result.props).toBe(true); + expect(result.props._value1).toBe(1); + }); + it('should have a __v2 property', () => { + const result = render(1, 2); + expect('_value2' in result.props).toBe(false); + expect(result.props.__v2).toBe(2); + }); + it('should have a __t property', () => { + const result = render(1, 2); + expect('tag' in result).toBe(false); + expect(result.__t).toBe('div'); }); }); diff --git a/e2e-test/webpack-custom/webpack.config.js b/e2e-test/webpack-custom/webpack.config.js index af1fa3a..bfc93f2 100644 --- a/e2e-test/webpack-custom/webpack.config.js +++ b/e2e-test/webpack-custom/webpack.config.js @@ -9,7 +9,7 @@ module.exports = { plugins: [ [ 'babel-plugin-transform-rename-properties', - { rename: { _value: '__v' } }, + { rename: { _value2: '__v2', tag: '__t' } }, ], ], }, diff --git a/src/lib/babel-loader.js b/src/lib/babel-loader.js index fcc24f1..e406413 100644 --- a/src/lib/babel-loader.js +++ b/src/lib/babel-loader.js @@ -1,14 +1,3 @@ -export function getCoverageBabelLoader() { - return { - test: /\.jsx?$/, - exclude: /node_modules/, - loader: require.resolve('babel-loader'), - query: { - plugins: [require.resolve('babel-plugin-istanbul')], - }, - }; -} - export function getDefaultBabelLoader(options) { return { test: /\.jsx?$/, diff --git a/src/webpack.js b/src/webpack.js index 25eaf11..df9de68 100644 --- a/src/webpack.js +++ b/src/webpack.js @@ -1,10 +1,7 @@ import path from 'path'; import delve from 'dlv'; import { tryRequire, dedupe } from './lib/util'; -import { - getDefaultBabelLoader, - getCoverageBabelLoader, -} from './lib/babel-loader'; +import { getDefaultBabelLoader } from './lib/babel-loader'; import cssLoader from './lib/css-loader'; /** @@ -107,12 +104,23 @@ export function addWebpackConfig(karmaConfig, pkg, options) { return Object.assign({}, configured || {}, value); } - let existingBabelLoader = getLoader((rule) => + let babelLoader = getLoader((rule) => `${rule.use},${rule.loader}`.match(/\bbabel-loader\b/) ); - if (existingBabelLoader) { + if (babelLoader) { if (options.coverage) { - loaders.push(getCoverageBabelLoader()); + if (babelLoader.loader.query) { + babelLoader.loader.query.plugins = [ + ...(babelLoader.loader.query?.plugins ?? []), + require.resolve('babel-plugin-istanbul'), + ]; + } else { + babelLoader.loader.options = babelLoader.loader.options || {}; + babelLoader.loader.options.plugins = [ + ...(babelLoader.loader.options?.plugins ?? []), + require.resolve('babel-plugin-istanbul'), + ]; + } } } else { loaders.push(getDefaultBabelLoader(options));