diff --git a/package.json b/package.json index 7493cad88c29..fc88b041b1df 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,7 @@ "istanbul": "^0.4.5", "jasmine": "^2.6.0", "jasmine-spec-reporter": "^3.2.0", + "karma": "~3.1.1", "karma-jasmine-html-reporter": "^0.2.2", "license-checker": "^20.1.0", "minimatch": "^3.0.4", diff --git a/packages/angular_devkit/build_angular/package.json b/packages/angular_devkit/build_angular/package.json index 498ac49eedce..e699262781d7 100644 --- a/packages/angular_devkit/build_angular/package.json +++ b/packages/angular_devkit/build_angular/package.json @@ -77,7 +77,7 @@ "jquery": "^3.3.1", "jasmine-core": "~2.8.0", "jasmine-spec-reporter": "~4.2.1", - "karma": "~3.0.0", + "karma": "~3.1.1", "karma-chrome-launcher": "~2.2.0", "karma-coverage-istanbul-reporter": "~2.0.0", "karma-jasmine": "~1.1.0", diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/karma.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/karma.ts index 54de40d22155..bdc4e2a04744 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/karma.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/plugins/karma.ts @@ -9,13 +9,15 @@ // TODO: cleanup this file, it's copied as is from Angular CLI. import * as path from 'path'; -import * as fs from 'fs'; import * as glob from 'glob'; import * as webpack from 'webpack'; const webpackDevMiddleware = require('webpack-dev-middleware'); -import { AssetPattern } from '../../browser/schema'; import { KarmaWebpackFailureCb } from './karma-webpack-failure-cb'; +import { statsErrorsToString } from '../utilities/stats'; +import { getWebpackStatsConfig } from '../models/webpack-configs/stats'; +import { createConsoleLogger } from '@angular-devkit/core/node'; +import { logging } from '@angular-devkit/core'; /** * Enumerate needed (but not require/imported) dependencies from this file @@ -62,7 +64,7 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => { ) } const options = config.buildWebpack.options; - const projectRoot = config.buildWebpack.projectRoot as string; + const logger: logging.Logger = config.buildWebpack.logger || createConsoleLogger(); successCb = config.buildWebpack.successCb; failureCb = config.buildWebpack.failureCb; @@ -93,7 +95,9 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => { // Add webpack config. const webpackConfig = config.buildWebpack.webpackConfig; const webpackMiddlewareConfig = { - logLevel: 'error', // Hide webpack output because its noisy. + // Hide webpack output because its noisy. + logLevel: 'error', + stats: false, watchOptions: { poll: options.poll }, publicPath: '/_karma_webpack_/', }; @@ -147,9 +151,9 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => { try { compiler = webpack(webpackConfig); } catch (e) { - console.error(e.stack || e); + logger.error(e.stack || e) if (e.details) { - console.error(e.details); + logger.error(e.details) } throw e; } @@ -175,9 +179,17 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => { } let lastCompilationHash: string | undefined; + const statsConfig = getWebpackStatsConfig(); compiler.hooks.done.tap('karma', (stats: any) => { - // Refresh karma only when there are no webpack errors, and if the compilation changed. - if (stats.compilation.errors.length === 0 && stats.hash != lastCompilationHash) { + if (stats.compilation.errors.length > 0) { + const json = stats.toJson(config.stats); + // Print compilation errors. + logger.error(statsErrorsToString(json, statsConfig)); + lastCompilationHash = undefined; + // Emit a failure build event if there are compilation errors. + failureCb && failureCb(); + } else if (stats.hash != lastCompilationHash) { + // Refresh karma only when there are no webpack errors, and if the compilation changed. lastCompilationHash = stats.hash; emitter.refreshFiles(); } diff --git a/packages/angular_devkit/build_angular/src/karma/index.ts b/packages/angular_devkit/build_angular/src/karma/index.ts index 3479edf21f68..1bd80e91196e 100644 --- a/packages/angular_devkit/build_angular/src/karma/index.ts +++ b/packages/angular_devkit/build_angular/src/karma/index.ts @@ -97,6 +97,7 @@ export class KarmaBuilder implements Builder { // When this workaround is removed, user projects need to be updated to use a Karma // version that has a fix for this issue. toJSON: () => { }, + logger: this.context.logger, }; // TODO: inside the configs, always use the project root and not the workspace root. @@ -108,15 +109,14 @@ export class KarmaBuilder implements Builder { // Complete the observable once the Karma server returns. const karmaServer = new karma.Server(karmaOptions, () => obs.complete()); - karmaServer.start(); + const karmaStartPromise = karmaServer.start(); // Cleanup, signal Karma to exit. return () => { - // Karma does not seem to have a way to exit the server gracefully. - // See https://github.com/karma-runner/karma/issues/2867#issuecomment-369912167 - // TODO: make a PR for karma to add `karmaServer.close(code)`, that - // calls `disconnectBrowsers(code);` - // karmaServer.close(); + // Karma only has the `stop` method start with 3.1.1, so we must defensively check. + if (karmaServer.stop && typeof karmaServer.stop === 'function') { + return karmaStartPromise.then(() => karmaServer.stop()); + } }; })), ); diff --git a/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts b/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts index 9d9523965cda..778b3b687376 100644 --- a/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts +++ b/packages/angular_devkit/build_angular/test/browser/rebuild_spec_large.ts @@ -233,7 +233,7 @@ describe('Browser Builder rebuilds', () => { // Add a major static analysis error on a non-main file to the initial build. host.replaceInFile('./src/app/app.component.ts', `'app-root'`, `(() => 'app-root')()`); - const overrides = { watch: true, aot: true, forkTypeChecker: false }; + const overrides = { watch: true, aot: true }; const logger = new TestLogger('rebuild-aot-errors'); const staticAnalysisError = 'Function expressions are not supported in decorators'; const syntaxError = 'Declaration or statement expected.'; @@ -298,7 +298,7 @@ describe('Browser Builder rebuilds', () => { 'src/app/imported-styles.css': 'p {color: #f00;}', }); - const overrides = { watch: true, aot: true, forkTypeChecker: false }; + const overrides = { watch: true, aot: true }; let buildNumber = 0; runTargetSpec(host, browserTargetSpec, overrides, DefaultTimeout * 3).pipe( diff --git a/packages/angular_devkit/build_angular/test/karma/rebuilds_spec_large.ts b/packages/angular_devkit/build_angular/test/karma/rebuilds_spec_large.ts index 494823bf121a..a9613859373e 100644 --- a/packages/angular_devkit/build_angular/test/karma/rebuilds_spec_large.ts +++ b/packages/angular_devkit/build_angular/test/karma/rebuilds_spec_large.ts @@ -6,68 +6,102 @@ * found in the LICENSE file at https://angular.io/license */ -import { runTargetSpec } from '@angular-devkit/architect/testing'; -import { debounceTime, take, tap } from 'rxjs/operators'; +import { DefaultTimeout, runTargetSpec } from '@angular-devkit/architect/testing'; +import { Subject } from 'rxjs'; +import { debounceTime, delay, take, takeUntil, takeWhile, tap } from 'rxjs/operators'; import { host, karmaTargetSpec } from '../utils'; -// Karma watch mode is currently bugged: -// - errors print a huge stack trace -// - karma does not have a way to close the server -// gracefully (https://github.com/karma-runner/karma/issues/3149) -// TODO: fix these before 6.0 final. -xdescribe('Karma Builder watch mode', () => { +describe('Karma Builder watch mode', () => { beforeEach(done => host.initialize().toPromise().then(done, done.fail)); afterEach(done => host.restore().toPromise().then(done, done.fail)); - it('works', (done) => { + it('works', async () => { const overrides = { watch: true }; - runTargetSpec(host, karmaTargetSpec, overrides).pipe( + const res = await runTargetSpec(host, karmaTargetSpec, overrides).pipe( debounceTime(500), tap((buildEvent) => expect(buildEvent.success).toBe(true)), take(1), - ).toPromise().then(done, done.fail); - }, 30000); + ).toPromise(); + + expect(res).toEqual({ success: true }); + }); it('recovers from compilation failures in watch mode', (done) => { const overrides = { watch: true }; - let buildNumber = 0; + let buildCount = 0; + let phase = 1; - runTargetSpec(host, karmaTargetSpec, overrides).pipe( + runTargetSpec(host, karmaTargetSpec, overrides, DefaultTimeout * 3).pipe( debounceTime(500), tap((buildEvent) => { - buildNumber += 1; - switch (buildNumber) { + buildCount += 1; + switch (phase) { case 1: // Karma run should succeed. // Add a compilation error. expect(buildEvent.success).toBe(true); - host.writeMultipleFiles({ - 'src/app/app.component.spec.ts': '

definitely not typescript

', - }); + // Add an syntax error to a non-main file. + host.appendToFile('src/app/app.component.spec.ts', `]]]`); + phase = 2; break; case 2: // Karma run should fail due to compilation error. Fix it. expect(buildEvent.success).toBe(false); - host.writeMultipleFiles({ 'src/foo.spec.ts': '' }); + host.replaceInFile('src/app/app.component.spec.ts', `]]]`, ''); + phase = 3; break; case 3: // Karma run should succeed again. expect(buildEvent.success).toBe(true); - break; - - default: + phase = 4; break; } }), - take(3), - ).toPromise().then(done, done.fail); - }, 30000); + takeWhile(() => phase < 4), + ).toPromise().then( + () => done(), + () => done.fail(`stuck at phase ${phase} [builds: ${buildCount}]`), + ); + }); it('does not rebuild when nothing changed', (done) => { - // Start the server in watch mode, wait for the first build to finish, touch - // test.js without changing it, wait 5s then exit unsuscribe, verify only one event was emitted. - }, 30000); + const overrides = { watch: true }; + let buildCount = 0; + let phase = 1; + + const stopSubject = new Subject(); + const stop$ = stopSubject.asObservable().pipe(delay(5000)); + + runTargetSpec(host, karmaTargetSpec, overrides, DefaultTimeout * 3).pipe( + debounceTime(500), + tap((buildEvent) => { + buildCount += 1; + switch (phase) { + case 1: + // Karma run should succeed. + // Add a compilation error. + expect(buildEvent.success).toBe(true); + // Touch the file. + host.appendToFile('src/app/app.component.spec.ts', ``); + // Signal the stopper, which delays emission by 5s. + // If there's no rebuild within that time then the test is successful. + stopSubject.next(); + phase = 2; + break; + + case 2: + // Should never trigger this second build. + expect(true).toBeFalsy('Should not trigger second build.'); + break; + } + }), + takeUntil(stop$), + ).toPromise().then( + () => done(), + () => done.fail(`stuck at phase ${phase} [builds: ${buildCount}]`), + ); + }); }); diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts index ba47634abbc5..7689c4cf4a09 100644 --- a/packages/ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/ngtools/webpack/src/angular_compiler_plugin.ts @@ -39,7 +39,7 @@ import { Compiler, compilation } from 'webpack'; import { time, timeEnd } from './benchmark'; import { WebpackCompilerHost, workaroundResolve } from './compiler_host'; import { resolveEntryModuleFromMain } from './entry_resolver'; -import { gatherDiagnostics, hasErrors } from './gather_diagnostics'; +import { DiagnosticMode, gatherDiagnostics, hasErrors } from './gather_diagnostics'; import { TypeScriptPathsPlugin } from './paths-plugin'; import { WebpackResourceLoader } from './resource_loader'; import { @@ -284,6 +284,7 @@ export class AngularCompilerPlugin { if (options.forkTypeChecker !== undefined) { this._forkTypeChecker = options.forkTypeChecker; } + // this._forkTypeChecker = false; // Add custom platform transformers. if (options.platformTransformers !== undefined) { @@ -1038,6 +1039,8 @@ export class AngularCompilerPlugin { time('AngularCompilerPlugin._emit'); const program = this._program; const allDiagnostics: Array = []; + const diagMode = (this._firstRun || !this._forkTypeChecker) ? + DiagnosticMode.All : DiagnosticMode.Syntactic; let emitResult: ts.EmitResult | undefined; try { @@ -1051,10 +1054,8 @@ export class AngularCompilerPlugin { timeEnd('AngularCompilerPlugin._emit.ts.getOptionsDiagnostics'); } - if ((this._firstRun || !this._forkTypeChecker) && this._program) { - allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode, - 'AngularCompilerPlugin._emit.ts')); - } + allDiagnostics.push(...gatherDiagnostics(tsProgram, this._JitMode, + 'AngularCompilerPlugin._emit.ts', diagMode)); if (!hasErrors(allDiagnostics)) { if (this._firstRun || sourceFiles.length > 20) { @@ -1098,10 +1099,8 @@ export class AngularCompilerPlugin { timeEnd('AngularCompilerPlugin._emit.ng.getNgOptionDiagnostics'); } - if ((this._firstRun || !this._forkTypeChecker) && this._program) { - allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode, - 'AngularCompilerPlugin._emit.ng')); - } + allDiagnostics.push(...gatherDiagnostics(angularProgram, this._JitMode, + 'AngularCompilerPlugin._emit.ng', diagMode)); if (!hasErrors(allDiagnostics)) { time('AngularCompilerPlugin._emit.ng.emit'); diff --git a/packages/ngtools/webpack/src/gather_diagnostics.ts b/packages/ngtools/webpack/src/gather_diagnostics.ts index ccde7b02774d..3a8b40cae6a7 100644 --- a/packages/ngtools/webpack/src/gather_diagnostics.ts +++ b/packages/ngtools/webpack/src/gather_diagnostics.ts @@ -9,6 +9,13 @@ import { Diagnostic, Diagnostics, Program } from '@angular/compiler-cli'; import * as ts from 'typescript'; import { time, timeEnd } from './benchmark'; +export enum DiagnosticMode { + Syntactic = 1 << 0, + Semantic = 1 << 1, + + All = Syntactic | Semantic, + Default = All, +} export class CancellationToken implements ts.CancellationToken { private _isCancelled = false; @@ -36,6 +43,7 @@ export function gatherDiagnostics( program: ts.Program | Program, jitMode: boolean, benchmarkLabel: string, + mode = DiagnosticMode.All, cancellationToken?: CancellationToken, ): Diagnostics { const allDiagnostics: Array = []; @@ -52,34 +60,44 @@ export function gatherDiagnostics( } } + const gatherSyntacticDiagnostics = (mode & DiagnosticMode.Syntactic) != 0; + const gatherSemanticDiagnostics = (mode & DiagnosticMode.Semantic) != 0; + if (jitMode) { const tsProgram = program as ts.Program; - // Check syntactic diagnostics. - time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`); - checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram)); - timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`); - - // Check semantic diagnostics. - time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`); - checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram)); - timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`); + if (gatherSyntacticDiagnostics) { + // Check syntactic diagnostics. + time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`); + checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram)); + timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`); + } + + if (gatherSemanticDiagnostics) { + // Check semantic diagnostics. + time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`); + checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram)); + timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`); + } } else { const angularProgram = program as Program; + if (gatherSyntacticDiagnostics) { + // Check TypeScript syntactic diagnostics. + time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`); + checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram)); + timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`); + } - // Check TypeScript syntactic diagnostics. - time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`); - checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram)); - timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`); - - // Check TypeScript semantic and Angular structure diagnostics. - time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`); - checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram)); - timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`); + if (gatherSemanticDiagnostics) { + // Check TypeScript semantic and Angular structure diagnostics. + time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`); + checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram)); + timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`); - // Check Angular semantic diagnostics - time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`); - checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram)); - timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`); + // Check Angular semantic diagnostics + time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`); + checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram)); + timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`); + } } return allDiagnostics; diff --git a/packages/ngtools/webpack/src/type_checker.ts b/packages/ngtools/webpack/src/type_checker.ts index 17503fb1b18d..97263292ac7f 100644 --- a/packages/ngtools/webpack/src/type_checker.ts +++ b/packages/ngtools/webpack/src/type_checker.ts @@ -18,7 +18,7 @@ import { import * as ts from 'typescript'; import { time, timeEnd } from './benchmark'; import { WebpackCompilerHost } from './compiler_host'; -import { CancellationToken, gatherDiagnostics } from './gather_diagnostics'; +import { CancellationToken, DiagnosticMode, gatherDiagnostics } from './gather_diagnostics'; import { LogMessage, TypeCheckerMessage } from './type_checker_messages'; @@ -101,7 +101,7 @@ export class TypeChecker { private _diagnose(cancellationToken: CancellationToken) { const allDiagnostics = gatherDiagnostics( - this._program, this._JitMode, 'TypeChecker', cancellationToken); + this._program, this._JitMode, 'TypeChecker', DiagnosticMode.Semantic, cancellationToken); // Report diagnostics. if (!cancellationToken.isCancellationRequested()) { diff --git a/packages/schematics/angular/workspace/files/package.json b/packages/schematics/angular/workspace/files/package.json index b1440a336451..a3ea4c379f53 100644 --- a/packages/schematics/angular/workspace/files/package.json +++ b/packages/schematics/angular/workspace/files/package.json @@ -34,7 +34,7 @@ "codelyzer": "~4.5.0", "jasmine-core": "~2.99.1", "jasmine-spec-reporter": "~4.2.1", - "karma": "~3.0.0", + "karma": "~3.1.1", "karma-chrome-launcher": "~2.2.0", "karma-coverage-istanbul-reporter": "~2.0.1", "karma-jasmine": "~1.1.2", diff --git a/yarn.lock b/yarn.lock index 061c3fb7b8f0..11f88d92b126 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5542,10 +5542,10 @@ karma@alexeagle/karma#fa1a84ac881485b5657cb669e9b4e5da77b79f0a: tmp "0.0.33" useragent "^2.1.12" -karma@~3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/karma/-/karma-3.0.0.tgz#6da83461a8a28d8224575c3b5b874e271b4730c3" - integrity sha512-ZTjyuDXVXhXsvJ1E4CnZzbCjSxD6sEdzEsFYogLuZM0yqvg/mgz+O+R1jb0J7uAQeuzdY8kJgx6hSNXLwFuHIQ== +karma@~3.1.1: + version "3.1.1" + resolved "https://registry.yarnpkg.com/karma/-/karma-3.1.1.tgz#94c8edd20fb9597ccde343326da009737fb0423a" + integrity sha512-NetT3wPCQMNB36uiL9LLyhrOt8SQwrEKt0xD3+KpTCfm0VxVyUJdPL5oTq2Ic5ouemgL/Iz4wqXEbF3zea9kQQ== dependencies: bluebird "^3.3.0" body-parser "^1.16.1"