From f1e43fdc6026c8e9a262fc7a1608c28876b5c608 Mon Sep 17 00:00:00 2001 From: Blackbaud-SteveBrush Date: Thu, 18 May 2017 17:10:34 -0400 Subject: [PATCH 1/5] Added no-coverage flag to watch --- cli/test.js | 11 ++++- config/karma/dev.karma.conf.js | 2 +- config/karma/shared.karma.conf.js | 2 +- config/webpack/test.webpack.config.js | 58 +++++++++++++++------------ index.js | 2 +- test/cli-test.spec.js | 25 ++++++++++-- test/config-webpack-test.spec.js | 34 +++++++++++++--- 7 files changed, 93 insertions(+), 41 deletions(-) diff --git a/cli/test.js b/cli/test.js index 516e8ba1..83c3453d 100644 --- a/cli/test.js +++ b/cli/test.js @@ -5,11 +5,16 @@ * Spawns the karam start command. * @name test */ -function test(command) { +function test(command, argv) { const path = require('path'); const spawn = require('cross-spawn'); + let runCoverage = 'true'; + if (argv && argv.coverage === false) { + runCoverage = 'false'; + } + const karmaConfigPath = path.resolve( __dirname, '..', @@ -22,7 +27,9 @@ function test(command) { 'start', karmaConfigPath, '--command', - command + command, + '--coverage', + runCoverage ]; const options = { diff --git a/config/karma/dev.karma.conf.js b/config/karma/dev.karma.conf.js index cb0c76a2..5d46a80d 100644 --- a/config/karma/dev.karma.conf.js +++ b/config/karma/dev.karma.conf.js @@ -17,7 +17,7 @@ function getConfig(config) { const runtimePath = path.resolve(process.cwd(), 'runtime'); const skyPagesConfig = skyPagesConfigUtil.getSkyPagesConfig('test'); - let webpackConfig = testWebpackConfig.getWebpackConfig(skyPagesConfig); + let webpackConfig = testWebpackConfig.getWebpackConfig({}, skyPagesConfig); // Import shared karma config testKarmaConf(config); diff --git a/config/karma/shared.karma.conf.js b/config/karma/shared.karma.conf.js index 844cfd7b..950b3053 100644 --- a/config/karma/shared.karma.conf.js +++ b/config/karma/shared.karma.conf.js @@ -34,7 +34,7 @@ function getConfig(config) { '../../utils/spec-styles.js': ['webpack'], '../../utils/spec-bundle.js': ['coverage', 'webpack', 'sourcemap'] }, - webpack: testWebpackConfig.getWebpackConfig(skyPagesConfig), + webpack: testWebpackConfig.getWebpackConfig(argv, skyPagesConfig), coverageReporter: { dir: path.join(process.cwd(), 'coverage'), reporters: [ diff --git a/config/webpack/test.webpack.config.js b/config/webpack/test.webpack.config.js index 871fdce7..26880801 100644 --- a/config/webpack/test.webpack.config.js +++ b/config/webpack/test.webpack.config.js @@ -1,7 +1,8 @@ /*jslint node: true */ 'use strict'; -function getWebpackConfig(skyPagesConfig) { +function getWebpackConfig(argv, skyPagesConfig) { + function spaPath() { return skyPagesConfigUtil.spaPath.apply(skyPagesConfigUtil, arguments); } @@ -38,7 +39,7 @@ function getWebpackConfig(skyPagesConfig) { let alias = aliasBuilder.buildAliasList(skyPagesConfig); - return { + let config = { devtool: 'inline-source-map', resolveLoader: { @@ -119,30 +120,6 @@ function getWebpackConfig(skyPagesConfig) { 'raw-loader', 'sass-loader' ] - }, - { - enforce: 'post', - test: /\.(js|ts)$/, - use: [ - { - loader: 'istanbul-instrumenter-loader', - options: { - esModules: true - } - }, - { - loader: 'source-map-inline-loader' - } - ], - include: srcPath, - exclude: [ - /\.(e2e|spec)\.ts$/, - /node_modules/, - /index\.ts/, - /fixtures/, - /testing/, - /src(\\|\/)app(\\|\/)lib/ - ] } ] }, @@ -185,6 +162,35 @@ function getWebpackConfig(skyPagesConfig) { processExitCode ] }; + + if (argv.coverage === 'true') { + config.module.rules.push({ + enforce: 'post', + test: /\.(js|ts)$/, + use: [ + { + loader: 'istanbul-instrumenter-loader', + options: { + esModules: true + } + }, + { + loader: 'source-map-inline-loader' + } + ], + include: srcPath, + exclude: [ + /\.(e2e|spec)\.ts$/, + /node_modules/, + /index\.ts/, + /fixtures/, + /testing/, + /src(\\|\/)app(\\|\/)lib/ + ] + }); + } + + return config; } module.exports = { diff --git a/index.js b/index.js index 976efda4..990f90f2 100644 --- a/index.js +++ b/index.js @@ -21,7 +21,7 @@ module.exports = { break; case 'test': case 'watch': - require('./cli/test')(command); + require('./cli/test')(command, argv); break; case 'version': require('./cli/version')(); diff --git a/test/cli-test.spec.js b/test/cli-test.spec.js index 4ea4c1c4..9620f13d 100644 --- a/test/cli-test.spec.js +++ b/test/cli-test.spec.js @@ -22,7 +22,7 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd); + require('../cli/test')(cmd, {}); expect(found).toEqual(true); mock.stop('cross-spawn'); @@ -45,7 +45,7 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd); + require('../cli/test')(cmd, {}); expect(found).toEqual(true); mock.stop('cross-spawn'); @@ -65,12 +65,29 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd); + require('../cli/test')(cmd, {}); expect(argv.command).toEqual(cmd); mock.stop('cross-spawn'); }); + it('should pass the --no-coverage flag to karma', () => { + const cmd = 'CUSTOM_CMD'; + let argv; + + const minimist = require('minimist'); + mock('cross-spawn', (node, flags) => { + argv = minimist(flags); + return { + on: () => {} + }; + }); + + require('../cli/test')(cmd, { coverage: false }); + expect(argv.coverage).toEqual('false'); + mock.stop('cross-spawn'); + }); + it('should pass the exitCode', (done) => { const EXIT_CODE = 1337; @@ -87,7 +104,7 @@ describe('cli test', () => { } })); - require('../cli/test')('test'); + require('../cli/test')('test', {}); mock.stop('cross-spawn'); }); diff --git a/test/config-webpack-test.spec.js b/test/config-webpack-test.spec.js index 24459cc0..93879f70 100644 --- a/test/config-webpack-test.spec.js +++ b/test/config-webpack-test.spec.js @@ -4,6 +4,21 @@ const runtimeUtils = require('../utils/runtime-test-utils'); describe('config webpack common', () => { + let argv; + let skyPagesConfig; + + beforeEach(() => { + argv = { + coverage: 'true' + }; + skyPagesConfig = { + skyux: { + mode: 'advanced' + }, + runtime: runtimeUtils.getDefaultRuntime() + }; + }); + it('should expose a getWebpackConfig method', () => { const lib = require('../config/webpack/test.webpack.config'); expect(typeof lib.getWebpackConfig).toEqual('function'); @@ -11,12 +26,19 @@ describe('config webpack common', () => { it('should return a config object', () => { const lib = require('../config/webpack/test.webpack.config'); - const config = lib.getWebpackConfig({ - skyux: { - mode: 'advanced' - }, - runtime: runtimeUtils.getDefaultRuntime() - }); + const config = lib.getWebpackConfig(argv, skyPagesConfig); expect(config).toEqual(jasmine.any(Object)); }); + + it('should not run coverage if argv.coverage is false', () => { + argv.coverage = 'false'; + const lib = require('../config/webpack/test.webpack.config'); + const config = lib.getWebpackConfig(argv, skyPagesConfig); + + let instrumentLoader = config.module.rules.filter(rule => { + return (rule.use && rule.use[0].loader === 'istanbul-instrumenter-loader'); + })[0]; + + expect(instrumentLoader).not.toBeDefined(); + }); }); From 6c3ae89c3e493e112dd60404eeaf492957bd08e4 Mon Sep 17 00:00:00 2001 From: Blackbaud-SteveBrush Date: Mon, 22 May 2017 16:10:46 -0400 Subject: [PATCH 2/5] Updated tests and logic --- cli/test.js | 18 ++++++++---------- config/karma/dev.karma.conf.js | 2 +- config/karma/shared.karma.conf.js | 2 +- config/webpack/test.webpack.config.js | 4 ++-- test/cli-test.spec.js | 23 +++++++++++++++++++---- test/config-webpack-test.spec.js | 21 +++++++++++++++------ 6 files changed, 46 insertions(+), 24 deletions(-) diff --git a/cli/test.js b/cli/test.js index 83c3453d..da758179 100644 --- a/cli/test.js +++ b/cli/test.js @@ -5,16 +5,10 @@ * Spawns the karam start command. * @name test */ -function test(command, argv) { - +function test(command, argv = {}) { const path = require('path'); const spawn = require('cross-spawn'); - let runCoverage = 'true'; - if (argv && argv.coverage === false) { - runCoverage = 'false'; - } - const karmaConfigPath = path.resolve( __dirname, '..', @@ -27,11 +21,15 @@ function test(command, argv) { 'start', karmaConfigPath, '--command', - command, - '--coverage', - runCoverage + command ]; + if (argv.coverage === false) { + flags.push('--no-coverage'); + } else { + flags.push('--coverage'); + } + const options = { stdio: 'inherit' }; diff --git a/config/karma/dev.karma.conf.js b/config/karma/dev.karma.conf.js index 5d46a80d..cb0c76a2 100644 --- a/config/karma/dev.karma.conf.js +++ b/config/karma/dev.karma.conf.js @@ -17,7 +17,7 @@ function getConfig(config) { const runtimePath = path.resolve(process.cwd(), 'runtime'); const skyPagesConfig = skyPagesConfigUtil.getSkyPagesConfig('test'); - let webpackConfig = testWebpackConfig.getWebpackConfig({}, skyPagesConfig); + let webpackConfig = testWebpackConfig.getWebpackConfig(skyPagesConfig); // Import shared karma config testKarmaConf(config); diff --git a/config/karma/shared.karma.conf.js b/config/karma/shared.karma.conf.js index 950b3053..ed4ecd63 100644 --- a/config/karma/shared.karma.conf.js +++ b/config/karma/shared.karma.conf.js @@ -34,7 +34,7 @@ function getConfig(config) { '../../utils/spec-styles.js': ['webpack'], '../../utils/spec-bundle.js': ['coverage', 'webpack', 'sourcemap'] }, - webpack: testWebpackConfig.getWebpackConfig(argv, skyPagesConfig), + webpack: testWebpackConfig.getWebpackConfig(skyPagesConfig, argv), coverageReporter: { dir: path.join(process.cwd(), 'coverage'), reporters: [ diff --git a/config/webpack/test.webpack.config.js b/config/webpack/test.webpack.config.js index 26880801..cf18738c 100644 --- a/config/webpack/test.webpack.config.js +++ b/config/webpack/test.webpack.config.js @@ -1,7 +1,7 @@ /*jslint node: true */ 'use strict'; -function getWebpackConfig(argv, skyPagesConfig) { +function getWebpackConfig(skyPagesConfig, argv = {}) { function spaPath() { return skyPagesConfigUtil.spaPath.apply(skyPagesConfigUtil, arguments); @@ -163,7 +163,7 @@ function getWebpackConfig(argv, skyPagesConfig) { ] }; - if (argv.coverage === 'true') { + if (argv.coverage !== false) { config.module.rules.push({ enforce: 'post', test: /\.(js|ts)$/, diff --git a/test/cli-test.spec.js b/test/cli-test.spec.js index 9620f13d..fbaed5f5 100644 --- a/test/cli-test.spec.js +++ b/test/cli-test.spec.js @@ -71,20 +71,35 @@ describe('cli test', () => { }); + it('should pass the --coverage flag to karma by default', () => { + const cmd = 'CUSTOM_CMD'; + let found = false; + + mock('cross-spawn', (node, flags) => { + found = flags.includes('--coverage'); + return { + on: () => {} + }; + }); + + require('../cli/test')(cmd); + expect(found).toEqual(true); + mock.stop('cross-spawn'); + }); + it('should pass the --no-coverage flag to karma', () => { const cmd = 'CUSTOM_CMD'; - let argv; + let found = false; - const minimist = require('minimist'); mock('cross-spawn', (node, flags) => { - argv = minimist(flags); + found = flags.includes('--no-coverage'); return { on: () => {} }; }); require('../cli/test')(cmd, { coverage: false }); - expect(argv.coverage).toEqual('false'); + expect(found).toEqual(true); mock.stop('cross-spawn'); }); diff --git a/test/config-webpack-test.spec.js b/test/config-webpack-test.spec.js index 93879f70..f4ae25fe 100644 --- a/test/config-webpack-test.spec.js +++ b/test/config-webpack-test.spec.js @@ -8,9 +8,7 @@ describe('config webpack common', () => { let skyPagesConfig; beforeEach(() => { - argv = { - coverage: 'true' - }; + argv = {}; skyPagesConfig = { skyux: { mode: 'advanced' @@ -26,14 +24,25 @@ describe('config webpack common', () => { it('should return a config object', () => { const lib = require('../config/webpack/test.webpack.config'); - const config = lib.getWebpackConfig(argv, skyPagesConfig); + const config = lib.getWebpackConfig(skyPagesConfig); expect(config).toEqual(jasmine.any(Object)); }); + it('should run coverage by default', () => { + const lib = require('../config/webpack/test.webpack.config'); + const config = lib.getWebpackConfig(skyPagesConfig); + + let instrumentLoader = config.module.rules.filter(rule => { + return (rule.use && rule.use[0].loader === 'istanbul-instrumenter-loader'); + })[0]; + + expect(instrumentLoader).toBeDefined(); + }); + it('should not run coverage if argv.coverage is false', () => { - argv.coverage = 'false'; + argv.coverage = false; const lib = require('../config/webpack/test.webpack.config'); - const config = lib.getWebpackConfig(argv, skyPagesConfig); + const config = lib.getWebpackConfig(skyPagesConfig, argv); let instrumentLoader = config.module.rules.filter(rule => { return (rule.use && rule.use[0].loader === 'istanbul-instrumenter-loader'); From 0dcfc48fcf459c3f9d1b01fb9e19cd687e18b92e Mon Sep 17 00:00:00 2001 From: Blackbaud-SteveBrush Date: Mon, 22 May 2017 16:31:04 -0400 Subject: [PATCH 3/5] Minor adjustments --- test/cli-test.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/cli-test.spec.js b/test/cli-test.spec.js index fbaed5f5..a867c283 100644 --- a/test/cli-test.spec.js +++ b/test/cli-test.spec.js @@ -22,7 +22,7 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd, {}); + require('../cli/test')(cmd); expect(found).toEqual(true); mock.stop('cross-spawn'); @@ -45,7 +45,7 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd, {}); + require('../cli/test')(cmd); expect(found).toEqual(true); mock.stop('cross-spawn'); @@ -65,7 +65,7 @@ describe('cli test', () => { }; }); - require('../cli/test')(cmd, {}); + require('../cli/test')(cmd); expect(argv.command).toEqual(cmd); mock.stop('cross-spawn'); From 418a96855870ee3901437282889c7dd13df522ec Mon Sep 17 00:00:00 2001 From: Blackbaud-SteveBrush Date: Mon, 22 May 2017 16:33:15 -0400 Subject: [PATCH 4/5] Further cleanup --- test/cli-test.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cli-test.spec.js b/test/cli-test.spec.js index a867c283..6c00cac3 100644 --- a/test/cli-test.spec.js +++ b/test/cli-test.spec.js @@ -119,7 +119,7 @@ describe('cli test', () => { } })); - require('../cli/test')('test', {}); + require('../cli/test')('test'); mock.stop('cross-spawn'); }); From 4f0d828bb5afdefd0b6f2c48f29190534ba8c5a3 Mon Sep 17 00:00:00 2001 From: Blackbaud-SteveBrush Date: Tue, 23 May 2017 10:15:57 -0400 Subject: [PATCH 5/5] Removed parameter defaults --- cli/test.js | 4 ++-- config/webpack/test.webpack.config.js | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cli/test.js b/cli/test.js index da758179..c2cd294c 100644 --- a/cli/test.js +++ b/cli/test.js @@ -5,7 +5,7 @@ * Spawns the karam start command. * @name test */ -function test(command, argv = {}) { +function test(command, argv) { const path = require('path'); const spawn = require('cross-spawn'); @@ -24,7 +24,7 @@ function test(command, argv = {}) { command ]; - if (argv.coverage === false) { + if (argv && argv.coverage === false) { flags.push('--no-coverage'); } else { flags.push('--coverage'); diff --git a/config/webpack/test.webpack.config.js b/config/webpack/test.webpack.config.js index cf18738c..af5ba379 100644 --- a/config/webpack/test.webpack.config.js +++ b/config/webpack/test.webpack.config.js @@ -1,7 +1,7 @@ /*jslint node: true */ 'use strict'; -function getWebpackConfig(skyPagesConfig, argv = {}) { +function getWebpackConfig(skyPagesConfig, argv) { function spaPath() { return skyPagesConfigUtil.spaPath.apply(skyPagesConfigUtil, arguments); @@ -20,6 +20,7 @@ function getWebpackConfig(skyPagesConfig, argv = {}) { const skyPagesConfigUtil = require('../sky-pages/sky-pages.config'); const aliasBuilder = require('./alias-builder'); + const runCoverage = (!argv || argv.coverage !== false); skyPagesConfig.runtime.includeRouteModule = false; const ENV = process.env.ENV = process.env.NODE_ENV = 'test'; @@ -163,7 +164,7 @@ function getWebpackConfig(skyPagesConfig, argv = {}) { ] }; - if (argv.coverage !== false) { + if (runCoverage) { config.module.rules.push({ enforce: 'post', test: /\.(js|ts)$/,