Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(@angular/cli): improve unit test performance #6160

Merged
merged 1 commit into from
May 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
"isbinaryfile": "^3.0.0",
"istanbul-instrumenter-loader": "^2.0.0",
"json-loader": "^0.5.4",
"karma-sourcemap-loader": "^0.3.7",
"karma-webpack": "^2.0.0",
"less": "^2.7.2",
"less-loader": "^4.0.2",
"loader-utils": "^1.0.2",
Expand Down Expand Up @@ -96,6 +94,7 @@
"url-loader": "^0.5.7",
"walk-sync": "^0.3.1",
"webpack": "~2.4.0",
"webpack-dev-middleware": "^1.10.2",
"webpack-dev-server": "~2.4.2",
"webpack-merge": "^2.4.0",
"zone.js": "^0.8.4"
Expand Down
13 changes: 1 addition & 12 deletions packages/@angular/cli/blueprints/ng/files/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,14 @@ module.exports = function (config) {
client:{
clearContext: false // leave Jasmine Spec Runner output visible in browser
},
files: [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For projects using those, is this PR still working (even if it has the pre-PR performance hit)? Do we need to regenerate projects?

Copy link
Contributor Author

@filipesilva filipesilva May 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to regenerate projects. tests/e2e/tests/test/test-backwards-compat.ts tests that the old config works.

The new plugin has some logic that removes these entries if they are present in the karma config, that ensures backwards compatibility.

{ pattern: './<%= sourceDir %>/test.ts', watched: false }
],
preprocessors: {
'./<%= sourceDir %>/test.ts': ['@angular/cli']
},
mime: {
'text/x-typescript': ['ts','tsx']
},
coverageIstanbulReporter: {
reports: [ 'html', 'lcovonly' ],
fixWebpackSourcePaths: true
},
angularCli: {
environment: 'dev'
},
reporters: config.angularCli && config.angularCli.codeCoverage
? ['progress', 'coverage-istanbul']
: ['progress', 'kjhtml'],
reporters: ['progress', 'kjhtml'],
port: 9876,
colors: true,
logLevel: config.LOG_INFO,
Expand Down
21 changes: 13 additions & 8 deletions packages/@angular/cli/models/webpack-configs/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as webpack from 'webpack';

import { CliConfig } from '../config';
import { WebpackTestOptions } from '../webpack-test-config';
import { KarmaWebpackEmitlessError } from '../../plugins/karma-webpack-emitless-error';


/**
* Enumerate loaders and their dependencies from this file to let the dependency validator
Expand All @@ -20,7 +20,9 @@ export function getTestConfig(testConfig: WebpackTestOptions) {
const configPath = CliConfig.configFilePath();
const projectRoot = path.dirname(configPath);
const appConfig = CliConfig.fromProject().config.apps[0];
const nodeModules = path.resolve(projectRoot, 'node_modules');
const extraRules: any[] = [];
const extraPlugins: any[] = [];

if (testConfig.codeCoverage && CliConfig.fromProject()) {
const codeCoverageExclude = CliConfig.fromProject().get('test.codeCoverage.exclude');
Expand All @@ -38,7 +40,6 @@ export function getTestConfig(testConfig: WebpackTestOptions) {
});
}


extraRules.push({
test: /\.(js|ts)$/, loader: 'istanbul-instrumenter-loader',
enforce: 'post',
Expand All @@ -49,17 +50,21 @@ export function getTestConfig(testConfig: WebpackTestOptions) {
return {
devtool: testConfig.sourcemaps ? 'inline-source-map' : 'eval',
entry: {
test: path.resolve(projectRoot, appConfig.root, appConfig.test)
main: path.resolve(projectRoot, appConfig.root, appConfig.test)
},
module: {
rules: [].concat(extraRules)
},
plugins: [
new webpack.SourceMapDevToolPlugin({
filename: null, // if no value is provided the sourcemap is inlined
test: /\.(ts|js)($|\?)/i // process .js and .ts files only
new webpack.optimize.CommonsChunkPlugin({
minChunks: Infinity,
name: 'inline'
}),
new KarmaWebpackEmitlessError()
]
new webpack.optimize.CommonsChunkPlugin({
name: 'vendor',
chunks: ['main'],
minChunks: (module: any) => module.resource && module.resource.startsWith(nodeModules)
})
].concat(extraPlugins)
};
}
6 changes: 0 additions & 6 deletions packages/@angular/cli/models/webpack-test-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as webpack from 'webpack';
const webpackMerge = require('webpack-merge');

import { BuildOptions } from './build-options';
Expand Down Expand Up @@ -28,11 +27,6 @@ export class WebpackTestConfig extends NgCliWebpackConfig {
];

this.config = webpackMerge(webpackConfigs);
delete this.config.entry;

// Remove any instance of CommonsChunkPlugin, not needed with karma-webpack.
this.config.plugins = this.config.plugins.filter((plugin: any) =>
!(plugin instanceof webpack.optimize.CommonsChunkPlugin));

return this.config;
}
Expand Down
3 changes: 1 addition & 2 deletions packages/@angular/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@
"inquirer": "^3.0.0",
"isbinaryfile": "^3.0.0",
"json-loader": "^0.5.4",
"karma-sourcemap-loader": "^0.3.7",
"karma-webpack": "^2.0.0",
"less": "^2.7.2",
"less-loader": "^4.0.2",
"lodash": "^4.11.1",
Expand Down Expand Up @@ -81,6 +79,7 @@
"url-loader": "^0.5.7",
"walk-sync": "^0.3.1",
"webpack": "~2.4.0",
"webpack-dev-middleware": "^1.10.2",
"webpack-dev-server": "~2.4.2",
"webpack-merge": "^2.4.0",
"zone.js": "^0.8.4"
Expand Down
41 changes: 41 additions & 0 deletions packages/@angular/cli/plugins/karma-context.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!DOCTYPE html>
<!--
This is the execution context.
Loaded within the iframe.
Reloaded before every execution run.
-->
<html>

<head>
<title></title>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no" />
</head>

<body>
<!-- The scripts need to be in the body DOM element, as some test running frameworks need the body
to have already been created so they can insert their magic into it. For example, if loaded
before body, Angular Scenario test framework fails to find the body and crashes and burns in
an epic manner. -->
<script src="context.js"></script>
<script type="text/javascript">
// Configure our Karma and set up bindings
%CLIENT_CONFIG%
window.__karma__.setupContext(window);

// All served files with the latest timestamps
%MAPPINGS%
</script>
<script type="text/javascript" src="_karma_webpack_/inline.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/polyfills.bundle.js" crossorigin="anonymous"></script>
<!-- Dynamically replaced with <script> tags -->
%SCRIPTS%
<script type="text/javascript" src="_karma_webpack_/scripts.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/vendor.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/main.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript">
window.__karma__.loaded();
</script>
</body>

</html>
43 changes: 43 additions & 0 deletions packages/@angular/cli/plugins/karma-debug.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!doctype html>
<!--
This file is almost the same as context.html - loads all source files,
but its purpose is to be loaded in the main frame (not within an iframe),
just for immediate execution, without reporting to Karma server.
-->
<html>

<head>
%X_UA_COMPATIBLE%
<title>Karma DEBUG RUNNER</title>
<link href="favicon.ico" rel="icon" type="image/x-icon" />
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no" />
</head>

<body>
<!-- The scripts need to be at the end of body, so that some test running frameworks
(Angular Scenario, for example) need the body to be loaded so that it can insert its magic
into it. If it is before body, then it fails to find the body and crashes and burns in an epic
manner. -->
<script src="context.js"></script>
<script src="debug.js"></script>
<script type="text/javascript">
// Configure our Karma
%CLIENT_CONFIG%

// All served files with the latest timestamps
%MAPPINGS%
</script>
<script type="text/javascript" src="_karma_webpack_/inline.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/polyfills.bundle.js" crossorigin="anonymous"></script>
<!-- Dynamically replaced with <script> tags -->
%SCRIPTS%
<script type="text/javascript" src="_karma_webpack_/scripts.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/vendor.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript" src="_karma_webpack_/main.bundle.js" crossorigin="anonymous"></script>
<script type="text/javascript">
window.__karma__.loaded();
</script>
</body>

</html>
20 changes: 0 additions & 20 deletions packages/@angular/cli/plugins/karma-webpack-emitless-error.ts

This file was deleted.

Loading