Skip to content

Commit f38af30

Browse files
committed
refactor(@angular/build): clean up duplicate of code in the karma builders
This ensures a cleaner and easier to follow code and also avoid having the fix bugs in multiple places.
1 parent 98307df commit f38af30

File tree

5 files changed

+157
-150
lines changed

5 files changed

+157
-150
lines changed

packages/angular/build/src/builders/karma/application_builder.ts

Lines changed: 97 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { randomUUID } from 'node:crypto';
1212
import * as fs from 'node:fs/promises';
1313
import type { IncomingMessage, ServerResponse } from 'node:http';
1414
import { createRequire } from 'node:module';
15-
import * as path from 'node:path';
15+
import path from 'node:path';
1616
import { ReadableStreamController } from 'node:stream/web';
1717
import { globSync } from 'tinyglobby';
1818
import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
@@ -24,7 +24,9 @@ import { ApplicationBuilderInternalOptions } from '../application/options';
2424
import { Result, ResultFile, ResultKind } from '../application/results';
2525
import { OutputHashing } from '../application/schema';
2626
import { findTests, getTestEntrypoints } from './find-tests';
27-
import { NormalizedKarmaBuilderOptions } from './options';
27+
import { NormalizedKarmaBuilderOptions, normalizeOptions } from './options';
28+
import { Schema as KarmaBuilderOptions } from './schema';
29+
import type { KarmaBuilderTransformsOptions } from './index';
2830

2931
const localResolve = createRequire(__filename).resolve;
3032
const isWindows = process.platform === 'win32';
@@ -275,21 +277,20 @@ function injectKarmaReporter(
275277
}
276278

277279
export function execute(
278-
options: NormalizedKarmaBuilderOptions,
280+
options: KarmaBuilderOptions,
279281
context: BuilderContext,
280-
karmaOptions: ConfigOptions,
281-
transforms: {
282-
// The karma options transform cannot be async without a refactor of the builder implementation
283-
karmaOptions?: (options: ConfigOptions) => ConfigOptions;
284-
} = {},
282+
transforms?: KarmaBuilderTransformsOptions,
285283
): AsyncIterable<BuilderOutput> {
284+
const normalizedOptions = normalizeOptions(context, options);
285+
const karmaOptions = getBaseKarmaOptions(normalizedOptions, context);
286+
286287
let karmaServer: Server;
287288

288289
return new ReadableStream({
289290
async start(controller) {
290291
let init;
291292
try {
292-
init = await initializeApplication(options, context, karmaOptions, transforms);
293+
init = await initializeApplication(normalizedOptions, context, karmaOptions, transforms);
293294
} catch (err) {
294295
if (err instanceof ApplicationBuildError) {
295296
controller.enqueue({ success: false, message: err.message });
@@ -336,13 +337,9 @@ async function getProjectSourceRoot(context: BuilderContext): Promise<string> {
336337
return projectSourceRoot;
337338
}
338339

339-
function normalizePolyfills(polyfills: string | string[] | undefined): [string[], string[]] {
340-
if (typeof polyfills === 'string') {
341-
polyfills = [polyfills];
342-
} else if (!polyfills) {
343-
polyfills = [];
344-
}
345-
340+
function normalizePolyfills(
341+
polyfills: string[] | undefined = [],
342+
): [polyfills: string[], jasmineCleanup: string[]] {
346343
const jasmineGlobalEntryPoint = localResolve('./polyfills/jasmine_global.js');
347344
const jasmineGlobalCleanupEntrypoint = localResolve('./polyfills/jasmine_global_cleanup.js');
348345
const sourcemapEntrypoint = localResolve('./polyfills/init_sourcemaps.js');
@@ -379,9 +376,7 @@ async function initializeApplication(
379376
options: NormalizedKarmaBuilderOptions,
380377
context: BuilderContext,
381378
karmaOptions: ConfigOptions,
382-
transforms: {
383-
karmaOptions?: (options: ConfigOptions) => ConfigOptions;
384-
} = {},
379+
transforms?: KarmaBuilderTransformsOptions,
385380
): Promise<
386381
[typeof import('karma'), Config & ConfigOptions, BuildOptions, AsyncIterator<Result> | null]
387382
> {
@@ -423,13 +418,7 @@ async function initializeApplication(
423418
index: false,
424419
outputHashing: OutputHashing.None,
425420
optimization: false,
426-
sourceMap: options.codeCoverage
427-
? {
428-
scripts: true,
429-
styles: true,
430-
vendor: true,
431-
}
432-
: options.sourceMap,
421+
sourceMap: options.sourceMap,
433422
instrumentForCoverage,
434423
styles: options.styles,
435424
scripts: options.scripts,
@@ -551,8 +540,8 @@ async function initializeApplication(
551540
}
552541

553542
const parsedKarmaConfig: Config & ConfigOptions = await karma.config.parseConfig(
554-
options.karmaConfig && path.resolve(context.workspaceRoot, options.karmaConfig),
555-
transforms.karmaOptions ? transforms.karmaOptions(karmaOptions) : karmaOptions,
543+
options.karmaConfig,
544+
transforms?.karmaOptions ? await transforms.karmaOptions(karmaOptions) : karmaOptions,
556545
{ promiseConfig: true, throwErrors: true },
557546
);
558547

@@ -718,3 +707,83 @@ function getInstrumentationExcludedPaths(root: string, excludedPaths: string[]):
718707

719708
return excluded;
720709
}
710+
function getBaseKarmaOptions(
711+
options: NormalizedKarmaBuilderOptions,
712+
context: BuilderContext,
713+
): ConfigOptions {
714+
const singleRun = !options.watch;
715+
716+
// Determine project name from builder context target
717+
const projectName = context.target?.project;
718+
if (!projectName) {
719+
throw new Error(`The 'karma' builder requires a target to be specified.`);
720+
}
721+
722+
const karmaOptions: ConfigOptions = options.karmaConfig
723+
? {}
724+
: getBuiltInKarmaConfig(context.workspaceRoot, projectName);
725+
726+
karmaOptions.singleRun = singleRun;
727+
728+
// Workaround https://github.com/angular/angular-cli/issues/28271, by clearing context by default
729+
// for single run executions. Not clearing context for multi-run (watched) builds allows the
730+
// Jasmine Spec Runner to be visible in the browser after test execution.
731+
karmaOptions.client ??= {};
732+
karmaOptions.client.clearContext ??= singleRun ?? false; // `singleRun` defaults to `false` per Karma docs.
733+
734+
// Convert browsers from a string to an array
735+
if (options.browsers) {
736+
karmaOptions.browsers = options.browsers;
737+
}
738+
739+
if (options.reporters) {
740+
karmaOptions.reporters = options.reporters;
741+
}
742+
743+
return karmaOptions;
744+
}
745+
746+
function getBuiltInKarmaConfig(
747+
workspaceRoot: string,
748+
projectName: string,
749+
): ConfigOptions & Record<string, unknown> {
750+
let coverageFolderName = projectName.charAt(0) === '@' ? projectName.slice(1) : projectName;
751+
coverageFolderName = coverageFolderName.toLowerCase();
752+
753+
const workspaceRootRequire = createRequire(workspaceRoot + '/');
754+
755+
// Any changes to the config here need to be synced to: packages/schematics/angular/config/files/karma.conf.js.template
756+
return {
757+
basePath: '',
758+
frameworks: ['jasmine'],
759+
plugins: [
760+
'karma-jasmine',
761+
'karma-chrome-launcher',
762+
'karma-jasmine-html-reporter',
763+
'karma-coverage',
764+
].map((p) => workspaceRootRequire(p)),
765+
jasmineHtmlReporter: {
766+
suppressAll: true, // removes the duplicated traces
767+
},
768+
coverageReporter: {
769+
dir: path.join(workspaceRoot, 'coverage', coverageFolderName),
770+
subdir: '.',
771+
reporters: [{ type: 'html' }, { type: 'text-summary' }],
772+
},
773+
reporters: ['progress', 'kjhtml'],
774+
browsers: ['Chrome'],
775+
customLaunchers: {
776+
// Chrome configured to run in a bazel sandbox.
777+
// Disable the use of the gpu and `/dev/shm` because it causes Chrome to
778+
// crash on some environments.
779+
// See:
780+
// https://github.com/puppeteer/puppeteer/blob/v1.0.0/docs/troubleshooting.md#tips
781+
// https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t
782+
ChromeHeadlessNoSandbox: {
783+
base: 'ChromeHeadless',
784+
flags: ['--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage'],
785+
},
786+
},
787+
restartOnFileChange: true,
788+
};
789+
}

packages/angular/build/src/builders/karma/index.ts

Lines changed: 6 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -13,114 +13,24 @@ import {
1313
createBuilder,
1414
} from '@angular-devkit/architect';
1515
import type { ConfigOptions } from 'karma';
16-
import { createRequire } from 'node:module';
17-
import path from 'node:path';
18-
import { NormalizedKarmaBuilderOptions, normalizeOptions } from './options';
16+
1917
import type { Schema as KarmaBuilderOptions } from './schema';
2018

21-
export type KarmaConfigOptions = ConfigOptions & {
22-
buildWebpack?: unknown;
23-
configFile?: string;
24-
};
19+
export interface KarmaBuilderTransformsOptions {
20+
karmaOptions?: (options: ConfigOptions) => ConfigOptions | Promise<ConfigOptions>;
21+
}
2522

2623
/**
2724
* @experimental Direct usage of this function is considered experimental.
2825
*/
2926
export async function* execute(
3027
options: KarmaBuilderOptions,
3128
context: BuilderContext,
32-
transforms: {
33-
// The karma options transform cannot be async without a refactor of the builder implementation
34-
karmaOptions?: (options: KarmaConfigOptions) => KarmaConfigOptions;
35-
} = {},
29+
transforms?: KarmaBuilderTransformsOptions,
3630
): AsyncIterable<BuilderOutput> {
3731
const { execute } = await import('./application_builder');
38-
const normalizedOptions = normalizeOptions(options);
39-
const karmaOptions = getBaseKarmaOptions(normalizedOptions, context);
40-
41-
yield* execute(normalizedOptions, context, karmaOptions, transforms);
42-
}
43-
44-
function getBaseKarmaOptions(
45-
options: NormalizedKarmaBuilderOptions,
46-
context: BuilderContext,
47-
): KarmaConfigOptions {
48-
const singleRun = !options.watch;
49-
50-
// Determine project name from builder context target
51-
const projectName = context.target?.project;
52-
if (!projectName) {
53-
throw new Error(`The 'karma' builder requires a target to be specified.`);
54-
}
55-
56-
const karmaOptions: KarmaConfigOptions = options.karmaConfig
57-
? {}
58-
: getBuiltInKarmaConfig(context.workspaceRoot, projectName);
59-
60-
karmaOptions.singleRun = singleRun;
61-
62-
// Workaround https://github.com/angular/angular-cli/issues/28271, by clearing context by default
63-
// for single run executions. Not clearing context for multi-run (watched) builds allows the
64-
// Jasmine Spec Runner to be visible in the browser after test execution.
65-
karmaOptions.client ??= {};
66-
karmaOptions.client.clearContext ??= singleRun ?? false; // `singleRun` defaults to `false` per Karma docs.
67-
68-
// Convert browsers from a string to an array
69-
if (options.browsers) {
70-
karmaOptions.browsers = options.browsers;
71-
}
72-
73-
if (options.reporters) {
74-
karmaOptions.reporters = options.reporters;
75-
}
76-
77-
return karmaOptions;
78-
}
79-
80-
function getBuiltInKarmaConfig(
81-
workspaceRoot: string,
82-
projectName: string,
83-
): ConfigOptions & Record<string, unknown> {
84-
let coverageFolderName = projectName.charAt(0) === '@' ? projectName.slice(1) : projectName;
85-
coverageFolderName = coverageFolderName.toLowerCase();
86-
87-
const workspaceRootRequire = createRequire(workspaceRoot + '/');
8832

89-
// Any changes to the config here need to be synced to: packages/schematics/angular/config/files/karma.conf.js.template
90-
return {
91-
basePath: '',
92-
rootUrl: '/',
93-
frameworks: ['jasmine'],
94-
plugins: [
95-
'karma-jasmine',
96-
'karma-chrome-launcher',
97-
'karma-jasmine-html-reporter',
98-
'karma-coverage',
99-
].map((p) => workspaceRootRequire(p)),
100-
jasmineHtmlReporter: {
101-
suppressAll: true, // removes the duplicated traces
102-
},
103-
coverageReporter: {
104-
dir: path.join(workspaceRoot, 'coverage', coverageFolderName),
105-
subdir: '.',
106-
reporters: [{ type: 'html' }, { type: 'text-summary' }],
107-
},
108-
reporters: ['progress', 'kjhtml'],
109-
browsers: ['Chrome'],
110-
customLaunchers: {
111-
// Chrome configured to run in a bazel sandbox.
112-
// Disable the use of the gpu and `/dev/shm` because it causes Chrome to
113-
// crash on some environments.
114-
// See:
115-
// https://github.com/puppeteer/puppeteer/blob/v1.0.0/docs/troubleshooting.md#tips
116-
// https://stackoverflow.com/questions/50642308/webdriverexception-unknown-error-devtoolsactiveport-file-doesnt-exist-while-t
117-
ChromeHeadlessNoSandbox: {
118-
base: 'ChromeHeadless',
119-
flags: ['--no-sandbox', '--headless', '--disable-gpu', '--disable-dev-shm-usage'],
120-
},
121-
},
122-
restartOnFileChange: true,
123-
};
33+
yield* execute(options, context, transforms);
12434
}
12535

12636
export type { KarmaBuilderOptions };

packages/angular/build/src/builders/karma/options.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import type { BuilderContext } from '@angular-devkit/architect';
10+
import { resolve } from 'node:path';
911
import { Schema as KarmaBuilderOptions } from './schema';
1012

11-
export type NormalizedKarmaBuilderOptions = Awaited<ReturnType<typeof normalizeOptions>>;
13+
export type NormalizedKarmaBuilderOptions = ReturnType<typeof normalizeOptions>;
1214

13-
export function normalizeOptions(options: KarmaBuilderOptions) {
14-
const { watch = true, include = [], exclude = [], reporters = [], browsers, ...rest } = options;
15+
export function normalizeOptions(context: BuilderContext, options: KarmaBuilderOptions) {
16+
const {
17+
sourceMap,
18+
karmaConfig,
19+
browsers,
20+
watch = true,
21+
include = [],
22+
exclude = [],
23+
reporters = [],
24+
...rest
25+
} = options;
1526

1627
let normalizedBrowsers: string[] | undefined;
1728
if (typeof options.browsers === 'string' && options.browsers) {
@@ -25,12 +36,23 @@ export function normalizeOptions(options: KarmaBuilderOptions) {
2536
.reduce<string[]>((acc, curr) => acc.concat(curr.split(',')), [])
2637
.filter((x) => !!x);
2738

39+
// Sourcemaps are always needed when code coverage is enabled.
40+
const normalizedSourceMap = options.codeCoverage
41+
? {
42+
scripts: true,
43+
styles: true,
44+
vendor: true,
45+
}
46+
: sourceMap;
47+
2848
return {
49+
...rest,
50+
sourceMap: normalizedSourceMap,
51+
karmaConfig: karmaConfig ? resolve(context.workspaceRoot, karmaConfig) : undefined,
2952
reporters: normalizedReporters.length ? normalizedReporters : undefined,
3053
browsers: normalizedBrowsers,
3154
watch,
3255
include,
3356
exclude,
34-
...rest,
3557
};
3658
}

packages/angular/build/src/private.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export { buildApplicationInternal } from './builders/application';
2626
export type { ApplicationBuilderInternalOptions } from './builders/application/options';
2727
export { type Result, type ResultFile, ResultKind } from './builders/application/results';
2828
export { serveWithVite } from './builders/dev-server/vite-server';
29-
export { execute as executeKarmaInternal } from './builders/karma/application_builder';
3029

3130
// Tools
3231
export * from './tools/babel/plugins';

0 commit comments

Comments
 (0)