feat(@angular-devkit/build-angular): Switch to karma-coverage

This commit switches coverage tooling from karma-coverage-istanbul-reporter
to karma-coverage since it's better supported.

Author: kyliau

integration/angular_cli/karma.conf.js

@@ -12,16 +12,19 @@ module.exports = function (config) {
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
       require('karma-jasmine-html-reporter'),
-      require('karma-coverage-istanbul-reporter'),
+      require('karma-coverage'),
       require('@angular-devkit/build-angular/plugins/karma'),
     ],
     client: {
       clearContext: false, // leave Jasmine Spec Runner output visible in browser
     },
-    coverageIstanbulReporter: {
+    coverageReporter: {
       dir: path.join(__dirname, 'coverage'),
-      reports: ['html', 'lcovonly'],
-      fixWebpackSourcePaths: true,
+      subdir: '.',
+      reporters: [
+        {type: 'html'},
+        {type: 'lcov'},
+      ],
     },
     reporters: ['progress', 'kjhtml'],
     port: 9876,

integration/angular_cli/package.json

@@ -36,7 +36,7 @@
     "jasmine-spec-reporter": "~5.0.0",
     "karma": "~4.4.1",
     "karma-chrome-launcher": "~3.1.0",
-    "karma-coverage-istanbul-reporter": "~2.1.0",
+    "karma-coverage": "~2.0.2",
     "karma-jasmine": "~3.3.0",
     "karma-jasmine-html-reporter": "^1.5.0",
     "protractor": "~5.4.3",

package.json

@@ -159,7 +159,7 @@
     "jquery": "^3.3.1",
     "karma": "~5.0.0",
     "karma-chrome-launcher": "~3.1.0",
-    "karma-coverage-istanbul-reporter": "~3.0.0",
+    "karma-coverage": "~2.0.2",
     "karma-jasmine": "~3.3.0",
     "karma-jasmine-html-reporter": "^1.5.0",
     "karma-source-map-support": "1.4.0",

packages/angular_devkit/build_angular/BUILD.bazel

@@ -267,10 +267,11 @@ LARGE_SPECS = {
         "extra_deps": [
             "@npm//karma",
             "@npm//karma-chrome-launcher",
-            "@npm//karma-coverage-istanbul-reporter",
+            "@npm//karma-coverage",
             "@npm//karma-jasmine",
             "@npm//karma-jasmine-html-reporter",
             "@npm//puppeteer",
+            "@npm//@types/karma",
         ],
     },
     "protractor": {

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/karma.ts

@@ -20,6 +20,7 @@ import { createConsoleLogger } from '@angular-devkit/core/node';
 import { logging } from '@angular-devkit/core';
 import { WebpackTestOptions } from '../models/build-options';
 import { normalizeSourceMaps } from '../../utils/index';
+import {TestResults} from 'karma';
 
 /**
  * Enumerate needed (but not require/imported) dependencies from this file
@@ -33,8 +34,8 @@ import { normalizeSourceMaps } from '../../utils/index';
 let blocked: any[] = [];
 let isBlocked = false;
 let webpackMiddleware: any;
-let successCb: () => void;
-let failureCb: () => void;
+let successCb: (results: TestResults) => void;
+let failureCb: (results: TestResults) => void;
 
 // Add files to the Karma files array.
 function addKarmaFiles(files: any[], newFiles: any[], prepend = false) {
@@ -72,8 +73,13 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => {
 
   // When using code-coverage, auto-add coverage-istanbul.
   config.reporters = config.reporters || [];
-  if (options.codeCoverage && config.reporters.indexOf('coverage-istanbul') === -1) {
-    config.reporters.push('coverage-istanbul');
+  // 'coverage-istanbul' comes from karma-coverage-istanbul-reporter package.
+  // It is deprecated in favor of karma-coverage.
+  const hasCoverageIstanbul = config.reporters.includes('coverage-istanbul');
+  // 'coverage' comes from karma-coverage package.
+  const hasKarmaCoverage = config.reporters.includes('coverage');
+  if (options.codeCoverage && !hasCoverageIstanbul && !hasKarmaCoverage) {
+    config.reporters.push('coverage');
   }
 
   // Add a reporter that fixes sourcemap urls.
@@ -188,7 +194,13 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => {
       logger.error(statsErrorsToString(json, statsConfig));
       lastCompilationHash = undefined;
       // Emit a failure build event if there are compilation errors.
-      failureCb && failureCb();
+      failureCb({
+        disconnected: false,
+        error: true,
+        exitCode: 1,
+        failed: 0,
+        success: 0,
+      });
     } else if (stats.hash != lastCompilationHash) {
       // Refresh karma only when there are no webpack errors, and if the compilation changed.
       lastCompilationHash = stats.hash;
@@ -269,9 +281,9 @@ const eventReporter: any = function (this: any, baseReporterDecorator: any, conf
 
   this.onRunComplete = function (_browsers: any, results: any) {
     if (results.exitCode === 0) {
-      successCb && successCb();
+      successCb(results);
     } else {
-      failureCb && failureCb();
+      failureCb(results);
     }
   }
 

packages/angular_devkit/build_angular/src/karma/code-coverage_spec.ts

@@ -7,8 +7,19 @@
  */
 import { Architect } from '@angular-devkit/architect';
 import { normalize, virtualFs } from '@angular-devkit/core';
+import { TestResults } from 'karma';
+import { promisify } from 'util';
 import { createArchitect, host, karmaTargetSpec } from '../test-utils';
 
+// In each of the test below we'll have to call setTimeout to wait for the coverage
+// analysis to be done. This is because karma-coverage performs the analysis
+// asynchronously but the promise that it returns is not awaited by Karma.
+// Coverage analysis begins when onRunComplete() is invoked, and output files
+// are subsequently written to disk. For more information, see
+// https://github.com/karma-runner/karma-coverage/blob/32acafa90ed621abd1df730edb44ae55a4009c2c/lib/reporter.js#L221
+
+const setTimeoutPromise = promisify(setTimeout);
+
 describe('Karma Builder code coverage', () => {
   const coverageFilePath = normalize('coverage/lcov.info');
   let architect: Architect;
@@ -27,12 +38,14 @@ describe('Karma Builder code coverage', () => {
 
     await run.stop();
 
+    await setTimeoutPromise(1000);
+
     const exists = host.scopedSync().exists(coverageFilePath);
-    expect(exists).toBe(true);
+    expect(exists).toBe(true, `${coverageFilePath} does not exist`);
 
     if (exists) {
       const content = virtualFs.fileBufferToString(host.scopedSync().read(coverageFilePath));
-      expect(content).toContain('polyfills.ts');
+      expect(content).toContain('app.component.ts');
       expect(content).toContain('test.ts');
     }
   }, 120000);
@@ -41,7 +54,6 @@ describe('Karma Builder code coverage', () => {
     const overrides = {
       codeCoverage: true,
       codeCoverageExclude: [
-        'src/polyfills.ts',
         '**/test.ts',
       ],
     };
@@ -52,12 +64,13 @@ describe('Karma Builder code coverage', () => {
 
     await run.stop();
 
+    await setTimeoutPromise(1000);
+
     const exists = host.scopedSync().exists(coverageFilePath);
     expect(exists).toBe(true);
 
     if (exists) {
       const content = virtualFs.fileBufferToString(host.scopedSync().read(coverageFilePath));
-      expect(content).not.toContain('polyfills.ts');
       expect(content).not.toContain('test.ts');
     }
   }, 120000);
@@ -102,6 +115,8 @@ describe('Karma Builder code coverage', () => {
 
     await run.stop();
 
+    await setTimeoutPromise(1000);
+
     const exists = host.scopedSync().exists(coverageFilePath);
     expect(exists).toBe(true);
 
@@ -112,19 +127,17 @@ describe('Karma Builder code coverage', () => {
   }, 120000);
 
   it(`should fail when coverage is below threhold and 'emitWarning' is false`, async () => {
-    host.replaceInFile('karma.conf.js', 'fixWebpackSourcePaths: true',
-      `
-      fixWebpackSourcePaths: true,
-      thresholds: {
-        emitWarning: false,
-        global: {
-          statements: 100,
-          lines: 100,
-          branches: 100,
-          functions: 100
+    host.replaceInFile('karma.conf.js', 'coverageReporter: {', `
+      coverageReporter: {
+        check: {
+          global: {
+            statements: 100,
+            lines: 100,
+            branches: 100,
+            functions: 100
+          }
         },
-      }`,
-    );
+    `);
 
     host.appendToFile('src/app/app.component.ts', `
       export function nonCovered(): boolean {
@@ -133,7 +146,13 @@ describe('Karma Builder code coverage', () => {
     `);
 
     const run = await architect.scheduleTarget(karmaTargetSpec, { codeCoverage: true });
-    await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: false }));
     await run.stop();
+
+    const result = await run.result;
+    // success is true because all tests passed
+    expect(result.success).toBeTrue();
+    const karmaResults: TestResults = result.karmaResults as never;
+    // exitCode is non-zero because coverage failed
+    expect(karmaResults.exitCode).toBe(1);
   }, 120000);
 });

packages/angular_devkit/build_angular/src/karma/index.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
-import { getSystemPath, join, normalize } from '@angular-devkit/core';
+import { JsonObject, getSystemPath, join, normalize } from '@angular-devkit/core';
 import { dirname, resolve } from 'path';
 import { Observable, from } from 'rxjs';
 import { defaultIfEmpty, switchMap } from 'rxjs/operators';
@@ -143,8 +143,14 @@ export function execute(
             options,
             webpackConfig,
             // Pass onto Karma to emit BuildEvents.
-            successCb: () => subscriber.next({ success: true }),
-            failureCb: () => subscriber.next({ success: false }),
+            successCb: (karmaResults: JsonObject) => subscriber.next({
+              success: true,
+              karmaResults,
+            }),
+            failureCb: (karmaResults: JsonObject) => subscriber.next({
+              success: false,
+              karmaResults,
+            }),
             // Workaround for https://github.com/karma-runner/karma/issues/3154
             // When this workaround is removed, user projects need to be updated to use a Karma
             // version that has a fix for this issue.

packages/angular_devkit/build_angular/test/hello-world-app/karma.conf.js

@@ -19,16 +19,19 @@ module.exports = function(config) {
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
       require('karma-jasmine-html-reporter'),
-      require('karma-coverage-istanbul-reporter'),
+      require('karma-coverage'),
       require('@angular-devkit/build-angular/plugins/karma'),
     ],
     client: {
       clearContext: false, // leave Jasmine Spec Runner output visible in browser
     },
-    coverageIstanbulReporter: {
-      dir: path.join(__dirname, 'coverage'),
-      reports: ['html', 'lcovonly'],
-      fixWebpackSourcePaths: true,
+    coverageReporter: {
+      dir: path.join(__dirname, './coverage'),
+      subdir: '.',
+      reporters: [
+        {type: 'html'},
+        {type: 'lcov'},
+      ],
     },
     reporters: ['progress', 'kjhtml'],
     port: 9876,

packages/angular_devkit/build_ng_packagr/test/ng-packaged/projects/lib/karma.conf.js

@@ -16,16 +16,19 @@ module.exports = function (config) {
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
       require('karma-jasmine-html-reporter'),
-      require('karma-coverage-istanbul-reporter'),
+      require('karma-coverage'),
       require('@angular-devkit/build-angular/plugins/karma')
     ],
     client: {
       clearContext: false // leave Jasmine Spec Runner output visible in browser
     },
-    coverageIstanbulReporter: {
+    coverageReporter: {
       dir: require('path').join(__dirname, 'coverage'),
-      reports: ['html', 'lcovonly'],
-      fixWebpackSourcePaths: true
+      subdir: '.',
+      reporters: [
+        {type: 'html'},
+        {type: 'lcov'},
+      ],
     },
     reporters: ['progress', 'kjhtml'],
     port: 9876,

packages/schematics/angular/application/files/karma.conf.js.template

@@ -9,16 +9,20 @@ module.exports = function (config) {
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
       require('karma-jasmine-html-reporter'),
-      require('karma-coverage-istanbul-reporter'),
+      require('karma-coverage'),
       require('@angular-devkit/build-angular/plugins/karma')
     ],
     client: {
       clearContext: false // leave Jasmine Spec Runner output visible in browser
     },
-    coverageIstanbulReporter: {
+    coverageReporter: {
       dir: require('path').join(__dirname, '<%= relativePathToWorkspaceRoot %>/coverage/<%= appName%>'),
-      reports: ['html', 'lcovonly', 'text-summary'],
-      fixWebpackSourcePaths: true
+      subdir: '.',
+      reporters: [
+        {type: 'html'},
+        {type: 'lcov'},
+        {type: 'text-summary'},
+      ],
     },
     reporters: ['progress', 'kjhtml'],
     port: 9876,

packages/schematics/angular/library/files/karma.conf.js.template

@@ -9,16 +9,20 @@ module.exports = function (config) {
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
       require('karma-jasmine-html-reporter'),
-      require('karma-coverage-istanbul-reporter'),
+      require('karma-coverage'),
       require('@angular-devkit/build-angular/plugins/karma')
     ],
     client: {
       clearContext: false // leave Jasmine Spec Runner output visible in browser
     },
-    coverageIstanbulReporter: {
+    coverageReporter: {
       dir: require('path').join(__dirname, '<%= relativePathToWorkspaceRoot %>/coverage/<%= folderName %>'),
-      reports: ['html', 'lcovonly', 'text-summary'],
-      fixWebpackSourcePaths: true
+      subdir: '.',
+      reporters: [
+        {type: 'html'},
+        {type: 'lcov'},
+        {type: 'text-summary'},
+      ],
     },
     reporters: ['progress', 'kjhtml'],
     port: 9876,

packages/schematics/angular/workspace/files/package.json.template

@@ -34,7 +34,7 @@
     "jasmine-spec-reporter": "~5.0.0",
     "karma": "~5.0.0",
     "karma-chrome-launcher": "~3.1.0",
-    "karma-coverage-istanbul-reporter": "~3.0.2",
+    "karma-coverage": "~2.0.2",
     "karma-jasmine": "~3.3.0",
     "karma-jasmine-html-reporter": "^1.5.0",
     "protractor": "~7.0.0",<% } %>