From b162679a26476fb53c44ba63f7844e96c21ad38c Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Wed, 10 Jun 2020 10:46:32 -0700 Subject: [PATCH] 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. Closes #17757 --- integration/angular_cli/karma.conf.js | 11 ++- integration/angular_cli/package.json | 2 +- package.json | 2 +- .../angular_devkit/build_angular/BUILD.bazel | 2 +- .../src/angular-cli-files/plugins/karma.ts | 36 +++++++-- .../src/karma/code-coverage_spec.ts | 75 +++++++++++++------ .../build_angular/src/karma/index.ts | 5 +- .../test/hello-world-app/karma.conf.js | 13 ++-- .../ng-packaged/projects/lib/karma.conf.js | 12 ++- .../application/files/karma.conf.js.template | 12 ++- .../library/files/karma.conf.js.template | 12 ++- .../workspace/files/package.json.template | 2 +- yarn.lock | 32 ++++---- 13 files changed, 142 insertions(+), 74 deletions(-) diff --git a/integration/angular_cli/karma.conf.js b/integration/angular_cli/karma.conf.js index 2784c587b390..a9c5297de149 100644 --- a/integration/angular_cli/karma.conf.js +++ b/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, diff --git a/integration/angular_cli/package.json b/integration/angular_cli/package.json index b63d473d69fc..feba5be5709a 100644 --- a/integration/angular_cli/package.json +++ b/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.3", "karma-jasmine": "~3.3.0", "karma-jasmine-html-reporter": "^1.5.0", "protractor": "~5.4.3", diff --git a/package.json b/package.json index b41ce21b2c67..7eea648e5c6c 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,7 @@ "jsonc-parser": "2.3.0", "karma": "~5.2.0", "karma-chrome-launcher": "~3.1.0", - "karma-coverage-istanbul-reporter": "~3.0.0", + "karma-coverage": "~2.0.3", "karma-jasmine": "~4.0.0", "karma-jasmine-html-reporter": "^1.5.0", "karma-source-map-support": "1.4.0", diff --git a/packages/angular_devkit/build_angular/BUILD.bazel b/packages/angular_devkit/build_angular/BUILD.bazel index 5f02e56a45fc..f69e08e555ac 100644 --- a/packages/angular_devkit/build_angular/BUILD.bazel +++ b/packages/angular_devkit/build_angular/BUILD.bazel @@ -282,7 +282,7 @@ 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", 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 4fa22ac44f8e..e329dfe67845 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 @@ -88,12 +88,32 @@ const init: any = (config: any, emitter: any, customFileHandlers: any) => { config.reporters.unshift('@angular-devkit/build-angular--event-reporter'); - // When using code-coverage, auto-add coverage-istanbul. - config.reporters = config.reporters || []; - if (options.codeCoverage && config.reporters.indexOf('coverage-istanbul') === -1) { - config.reporters.unshift('coverage-istanbul'); + // When using code-coverage, auto-add karma-coverage. + if (options.codeCoverage) { + config.plugins = config.plugins || []; + config.reporters = config.reporters || []; + const {plugins, reporters} = config; + const hasCoveragePlugin = plugins.some((p: {}) => 'reporter:coverage' in p); + const hasIstanbulPlugin = plugins.some((p: {}) => 'reporter:coverage-istanbul' in p); + const hasCoverageReporter = reporters.includes('coverage'); + const hasIstanbulReporter = reporters.includes('coverage-istanbul'); + if (hasCoveragePlugin && !hasCoverageReporter) { + reporters.push('coverage'); + } + else if (hasIstanbulPlugin && !hasIstanbulReporter) { + // coverage-istanbul is deprecated in favor of karma-coverage + reporters.push('coverage-istanbul'); + } + else { + throw new Error('karma-coverage must be installed in order to run code coverage'); + } + if (hasIstanbulPlugin) { + logger.warn(`'karma-coverage-istanbul-reporter' usage has been deprecated since version 11.\n` + + `Please install 'karma-coverage' and update 'karma.conf.js.' ` + + 'For more info, see https://github.com/karma-runner/karma-coverage/blob/master/README.md'); + } } - + // Add webpack config. const webpackConfig = config.buildWebpack.webpackConfig; const webpackMiddlewareConfig = { @@ -188,7 +208,7 @@ 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(); } else if (stats.hash != lastCompilationHash) { // Refresh karma only when there are no webpack errors, and if the compilation changed. lastCompilationHash = stats.hash; @@ -269,9 +289,9 @@ const eventReporter: any = function (this: any, baseReporterDecorator: any, conf this.onRunComplete = function (_browsers: any, results: any) { if (results.exitCode === 0) { - successCb && successCb(); + successCb(); } else { - failureCb && failureCb(); + failureCb(); } } diff --git a/packages/angular_devkit/build_angular/src/karma/code-coverage_spec.ts b/packages/angular_devkit/build_angular/src/karma/code-coverage_spec.ts index 5514beddd702..ab92ac8fd261 100644 --- a/packages/angular_devkit/build_angular/src/karma/code-coverage_spec.ts +++ b/packages/angular_devkit/build_angular/src/karma/code-coverage_spec.ts @@ -5,10 +5,24 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ + +// tslint:disable:no-implicit-dependencies + import { Architect } from '@angular-devkit/architect'; import { normalize, virtualFs } from '@angular-devkit/core'; +import { last, tap } from 'rxjs/operators'; +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; @@ -23,16 +37,19 @@ describe('Karma Builder code coverage', () => { it('supports code coverage option', async () => { const run = await architect.scheduleTarget(karmaTargetSpec, { codeCoverage: true }); - await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: true })); + const {success} = await run.result; + expect(success).toBe(true); 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,23 +58,24 @@ describe('Karma Builder code coverage', () => { const overrides = { codeCoverage: true, codeCoverageExclude: [ - 'src/polyfills.ts', '**/test.ts', ], }; const run = await architect.scheduleTarget(karmaTargetSpec, overrides); - await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: true })); + const {success} = await run.result; + expect(success).toBe(true); 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); @@ -98,10 +116,13 @@ describe('Karma Builder code coverage', () => { const run = await architect.scheduleTarget(karmaTargetSpec, { codeCoverage: true }); - await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: true })); + const {success} = await run.result; + expect(success).toBe(true); await run.stop(); + await setTimeoutPromise(1000); + const exists = host.scopedSync().exists(coverageFilePath); expect(exists).toBe(true); @@ -111,20 +132,18 @@ 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 + it('should exit with non-zero code when coverage is below threshold', async () => { + 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 +152,21 @@ describe('Karma Builder code coverage', () => { `); const run = await architect.scheduleTarget(karmaTargetSpec, { codeCoverage: true }); - await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: false })); + + // In incremental mode, karma-coverage does not have the ability to mark a + // run as failed if code coverage does not pass. This is because it does + // the coverage asynchoronously and Karma does not await the promise + // returned by the plugin. + expect((await run.result).success).toBeTrue(); + + // However the program must exit with non-zero exit code. + // This is a more common use case of coverage testing and must be supported. + await run.output.pipe( + last(), + tap(buildEvent => expect(buildEvent.success).toBeFalse()), + ).toPromise(); + await run.stop(); + }, 120000); }); diff --git a/packages/angular_devkit/build_angular/src/karma/index.ts b/packages/angular_devkit/build_angular/src/karma/index.ts index 9d901c51f687..3573e740e05d 100644 --- a/packages/angular_devkit/build_angular/src/karma/index.ts +++ b/packages/angular_devkit/build_angular/src/karma/index.ts @@ -155,7 +155,10 @@ export function execute( // Complete the observable once the Karma server returns. const karmaServer = new karma.Server( transforms.karmaOptions ? transforms.karmaOptions(karmaOptions) : karmaOptions, - () => subscriber.complete(), + (exitCode: number) => { + subscriber.next({ success: exitCode === 0 }); + subscriber.complete(); + }, ); // karma typings incorrectly define start's return value as void // tslint:disable-next-line:no-use-of-empty-return-value diff --git a/packages/angular_devkit/build_angular/test/hello-world-app/karma.conf.js b/packages/angular_devkit/build_angular/test/hello-world-app/karma.conf.js index c2f1cf7faa9f..9e73f21d849a 100644 --- a/packages/angular_devkit/build_angular/test/hello-world-app/karma.conf.js +++ b/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, diff --git a/packages/angular_devkit/build_ng_packagr/test/ng-packaged/projects/lib/karma.conf.js b/packages/angular_devkit/build_ng_packagr/test/ng-packaged/projects/lib/karma.conf.js index 2f49ecb6df61..0d2820cdc85b 100644 --- a/packages/angular_devkit/build_ng_packagr/test/ng-packaged/projects/lib/karma.conf.js +++ b/packages/angular_devkit/build_ng_packagr/test/ng-packaged/projects/lib/karma.conf.js @@ -16,16 +16,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, 'coverage'), - reports: ['html', 'lcovonly'], - fixWebpackSourcePaths: true + subdir: '.', + reporters: [ + {type: 'html'}, + {type: 'lcov'}, + {type: 'text-summary'}, + ], }, reporters: ['progress', 'kjhtml'], port: 9876, diff --git a/packages/schematics/angular/application/files/karma.conf.js.template b/packages/schematics/angular/application/files/karma.conf.js.template index 88505e166a08..df781efd080c 100644 --- a/packages/schematics/angular/application/files/karma.conf.js.template +++ b/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, diff --git a/packages/schematics/angular/library/files/karma.conf.js.template b/packages/schematics/angular/library/files/karma.conf.js.template index f6988d985b26..f91c6760ae2f 100644 --- a/packages/schematics/angular/library/files/karma.conf.js.template +++ b/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, diff --git a/packages/schematics/angular/workspace/files/package.json.template b/packages/schematics/angular/workspace/files/package.json.template index 6c632d39a53f..c1bcd75259e3 100644 --- a/packages/schematics/angular/workspace/files/package.json.template +++ b/packages/schematics/angular/workspace/files/package.json.template @@ -33,7 +33,7 @@ "jasmine-spec-reporter": "~5.0.0", "karma": "~5.2.0", "karma-chrome-launcher": "~3.1.0", - "karma-coverage-istanbul-reporter": "~3.0.2", + "karma-coverage": "~2.0.3", "karma-jasmine": "~4.0.0", "karma-jasmine-html-reporter": "^1.5.0", "protractor": "~7.0.0",<% } %> diff --git a/yarn.lock b/yarn.lock index ac75009475ce..2db592e6a836 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6629,11 +6629,6 @@ isstream@~0.1.2: resolved "https://registry.yarnpkg.com/isstream/-/isstream-0.1.2.tgz#47e63f7af55afa6f92e1500e690eb8b8529c099a" integrity sha1-R+Y/evVa+m+S4VAOaQ64uFKcCZo= -istanbul-lib-coverage@^2.0.5: - version "2.0.5" - resolved "https://registry.yarnpkg.com/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.5.tgz#675f0ab69503fad4b1d849f736baaca803344f49" - integrity sha512-8aXznuEPCJvGnMSRft4udDRDtb1V3pkQkMMI5LI+6HuQz5oQ4J2UFn1H82raA3qJtyOLkkwVqICBQkjnGtn5mA== - istanbul-lib-coverage@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/istanbul-lib-coverage/-/istanbul-lib-coverage-3.0.0.tgz#f5944a37c70b550b02a78a5c3b2055b280cec8ec" @@ -6658,18 +6653,16 @@ istanbul-lib-report@^3.0.0: make-dir "^3.0.0" supports-color "^7.1.0" -istanbul-lib-source-maps@^3.0.6: - version "3.0.6" - resolved "https://registry.yarnpkg.com/istanbul-lib-source-maps/-/istanbul-lib-source-maps-3.0.6.tgz#284997c48211752ec486253da97e3879defba8c8" - integrity sha512-R47KzMtDJH6X4/YW9XTx+jrLnZnscW4VpNN+1PViSYTejLVPWv7oov+Duf8YQSPyVRUvueQqz1TcsC6mooZTXw== +istanbul-lib-source-maps@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/istanbul-lib-source-maps/-/istanbul-lib-source-maps-4.0.0.tgz#75743ce6d96bb86dc7ee4352cf6366a23f0b1ad9" + integrity sha512-c16LpFRkR8vQXyHZ5nLpY35JZtzj1PQY1iZmesUbf1FZHbIupcWfjgOXBY9YHkLEQ6puz1u4Dgj6qmU/DisrZg== dependencies: debug "^4.1.1" - istanbul-lib-coverage "^2.0.5" - make-dir "^2.1.0" - rimraf "^2.6.3" + istanbul-lib-coverage "^3.0.0" source-map "^0.6.1" -istanbul-reports@^3.0.2: +istanbul-reports@^3.0.0, istanbul-reports@^3.0.2: version "3.0.2" resolved "https://registry.yarnpkg.com/istanbul-reports/-/istanbul-reports-3.0.2.tgz#d593210e5000683750cb09fc0644e4b6e27fd53b" integrity sha512-9tZvz7AiR3PEDNGiV9vIouQ/EAcqMXFmkcA1CDFTwOB98OZVDL0PH9glHotf5Ugp6GCOTypfzGWI/OqjWNCRUw== @@ -6938,15 +6931,16 @@ karma-chrome-launcher@~3.1.0: dependencies: which "^1.2.1" -karma-coverage-istanbul-reporter@~3.0.0: - version "3.0.3" - resolved "https://registry.yarnpkg.com/karma-coverage-istanbul-reporter/-/karma-coverage-istanbul-reporter-3.0.3.tgz#f3b5303553aadc8e681d40d360dfdc19bc7e9fe9" - integrity sha512-wE4VFhG/QZv2Y4CdAYWDbMmcAHeS926ZIji4z+FkB2aF/EposRb6DP6G5ncT/wXhqUfAb/d7kZrNKPonbvsATw== +karma-coverage@~2.0.3: + version "2.0.3" + resolved "https://registry.yarnpkg.com/karma-coverage/-/karma-coverage-2.0.3.tgz#c10f4711f4cf5caaaa668b1d6f642e7da122d973" + integrity sha512-atDvLQqvPcLxhED0cmXYdsPMCQuh6Asa9FMZW1bhNqlVEhJoB9qyZ2BY1gu7D/rr5GLGb5QzYO4siQskxaWP/g== dependencies: istanbul-lib-coverage "^3.0.0" + istanbul-lib-instrument "^4.0.1" istanbul-lib-report "^3.0.0" - istanbul-lib-source-maps "^3.0.6" - istanbul-reports "^3.0.2" + istanbul-lib-source-maps "^4.0.0" + istanbul-reports "^3.0.0" minimatch "^3.0.4" karma-jasmine-html-reporter@^1.5.0: