Skip to content

Commit dbcea96

Browse files
alan-agius4clydin
authored andcommitted
refactor(@angular-devkit/build-angular): simplify how specs are found and loaded
Prior to this change specs where found and loaded using Webpack's `require.context` API. The `require.context` is found in the users project `test.ts`. This resulted in a complex and hacky setup especially to filter tests when the `include` builder option is provided, were we had to amend the `test.ts` in memory. With this change we find all the specs files and add them as part of the main entrypoint. Closes #23751 and closes #22531
1 parent 9c13fce commit dbcea96

File tree

11 files changed

+99
-158
lines changed

11 files changed

+99
-158
lines changed

packages/angular_devkit/build_angular/src/builders/karma/find-tests.ts renamed to packages/angular_devkit/build_angular/src/builders/karma/find-tests-plugin.ts

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,73 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9+
import assert from 'assert';
910
import { PathLike, constants, promises as fs } from 'fs';
1011
import glob, { hasMagic } from 'glob';
1112
import { basename, dirname, extname, join, relative } from 'path';
1213
import { promisify } from 'util';
14+
import type { Compilation, Compiler } from 'webpack';
15+
import { addError } from '../../utils/webpack-diagnostics';
1316

1417
const globPromise = promisify(glob);
1518

19+
/**
20+
* The name of the plugin provided to Webpack when tapping Webpack compiler hooks.
21+
*/
22+
const PLUGIN_NAME = 'angular-find-tests-plugin';
23+
24+
export interface FindTestsPluginOptions {
25+
include?: string[];
26+
workspaceRoot: string;
27+
projectSourceRoot: string;
28+
}
29+
30+
export class FindTestsPlugin {
31+
private compilation: Compilation | undefined;
32+
33+
constructor(private options: FindTestsPluginOptions) {}
34+
35+
apply(compiler: Compiler): void {
36+
const { include = ['**/*.spec.ts'], projectSourceRoot, workspaceRoot } = this.options;
37+
const webpackOptions = compiler.options;
38+
const entry =
39+
typeof webpackOptions.entry === 'function' ? webpackOptions.entry() : webpackOptions.entry;
40+
41+
let originalImport: string[] | undefined;
42+
43+
// Add tests files are part of the entry-point.
44+
webpackOptions.entry = async () => {
45+
const specFiles = await findTests(include, workspaceRoot, projectSourceRoot);
46+
47+
if (!specFiles.length) {
48+
assert(this.compilation, 'Compilation cannot be undefined.');
49+
addError(
50+
this.compilation,
51+
`Specified patterns: "${include.join(', ')}" did not match any spec files.`,
52+
);
53+
}
54+
55+
const entrypoints = await entry;
56+
const entrypoint = entrypoints['main'];
57+
if (!entrypoint.import) {
58+
throw new Error(`Cannot find 'main' entrypoint.`);
59+
}
60+
61+
originalImport ??= entrypoint.import;
62+
entrypoint.import = [...originalImport, ...specFiles];
63+
64+
return entrypoints;
65+
};
66+
67+
compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
68+
this.compilation = compilation;
69+
compilation.contextDependencies.add(projectSourceRoot);
70+
});
71+
}
72+
}
73+
1674
// go through all patterns and find unique list of files
17-
export async function findTests(
75+
async function findTests(
1876
patterns: string[],
1977
workspaceRoot: string,
2078
projectSourceRoot: string,
@@ -37,6 +95,10 @@ async function findMatchingTests(
3795
): Promise<string[]> {
3896
// normalize pattern, glob lib only accepts forward slashes
3997
let normalizedPattern = normalizePath(pattern);
98+
if (normalizedPattern.charAt(0) === '/') {
99+
normalizedPattern = normalizedPattern.substring(1);
100+
}
101+
40102
const relativeProjectRoot = normalizePath(relative(workspaceRoot, projectSourceRoot) + '/');
41103

42104
// remove relativeProjectRoot to support relative paths from root
@@ -54,12 +116,13 @@ async function findMatchingTests(
54116
const fileExt = extname(normalizedPattern);
55117
// Replace extension to `.spec.ext`. Example: `src/app/app.component.ts`-> `src/app/app.component.spec.ts`
56118
const potentialSpec = join(
119+
projectSourceRoot,
57120
dirname(normalizedPattern),
58121
`${basename(normalizedPattern, fileExt)}.spec${fileExt}`,
59122
);
60123

61-
if (await exists(join(projectSourceRoot, potentialSpec))) {
62-
return [normalizePath(potentialSpec)];
124+
if (await exists(potentialSpec)) {
125+
return [potentialSpec];
63126
}
64127
}
65128
}
@@ -68,6 +131,7 @@ async function findMatchingTests(
68131
cwd: projectSourceRoot,
69132
root: projectSourceRoot,
70133
nomount: true,
134+
absolute: true,
71135
});
72136
}
73137

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

Lines changed: 16 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ import { purgeStaleBuildCache } from '../../utils/purge-cache';
1717
import { assertCompatibleAngularVersion } from '../../utils/version';
1818
import { generateBrowserWebpackConfigFromContext } from '../../utils/webpack-browser-config';
1919
import { getCommonConfig, getStylesConfig } from '../../webpack/configs';
20-
import { SingleTestTransformLoader } from '../../webpack/plugins/single-test-transform';
2120
import { Schema as BrowserBuilderOptions, OutputHashing } from '../browser/schema';
22-
import { findTests } from './find-tests';
21+
import { FindTestsPlugin } from './find-tests-plugin';
2322
import { Schema as KarmaBuilderOptions } from './schema';
2423

2524
export type KarmaConfigOptions = ConfigOptions & {
@@ -62,10 +61,7 @@ async function initialize(
6261

6362
const karma = await import('karma');
6463

65-
return [
66-
karma,
67-
webpackConfigurationTransformer ? await webpackConfigurationTransformer(config) : config,
68-
];
64+
return [karma, (await webpackConfigurationTransformer?.(config)) ?? config];
6965
}
7066

7167
/**
@@ -110,45 +106,22 @@ export function execute(
110106
}
111107
}
112108

113-
// prepend special webpack loader that will transform test.ts
114-
if (options.include?.length) {
115-
const projectName = context.target?.project;
116-
if (!projectName) {
117-
throw new Error('The builder requires a target.');
118-
}
109+
const projectName = context.target?.project;
110+
if (!projectName) {
111+
throw new Error('The builder requires a target.');
112+
}
119113

120-
const projectMetadata = await context.getProjectMetadata(projectName);
121-
const sourceRoot = (projectMetadata.sourceRoot ?? projectMetadata.root ?? '') as string;
122-
const projectSourceRoot = path.join(context.workspaceRoot, sourceRoot);
114+
const projectMetadata = await context.getProjectMetadata(projectName);
115+
const sourceRoot = (projectMetadata.sourceRoot ?? projectMetadata.root ?? '') as string;
123116

124-
const files = await findTests(options.include, context.workspaceRoot, projectSourceRoot);
125-
// early exit, no reason to start karma
126-
if (!files.length) {
127-
throw new Error(
128-
`Specified patterns: "${options.include.join(', ')}" did not match any spec files.`,
129-
);
130-
}
131-
132-
// Get the rules and ensure the Webpack configuration is setup properly
133-
const rules = webpackConfig.module?.rules || [];
134-
if (!webpackConfig.module) {
135-
webpackConfig.module = { rules };
136-
} else if (!webpackConfig.module.rules) {
137-
webpackConfig.module.rules = rules;
138-
}
139-
140-
rules.unshift({
141-
test: path.resolve(context.workspaceRoot, options.main),
142-
use: {
143-
// cannot be a simple path as it differs between environments
144-
loader: SingleTestTransformLoader,
145-
options: {
146-
files,
147-
logger: context.logger,
148-
},
149-
},
150-
});
151-
}
117+
webpackConfig.plugins ??= [];
118+
webpackConfig.plugins.push(
119+
new FindTestsPlugin({
120+
include: options.include,
121+
workspaceRoot: context.workspaceRoot,
122+
projectSourceRoot: path.join(context.workspaceRoot, sourceRoot),
123+
}),
124+
);
152125

153126
karmaOptions.buildWebpack = {
154127
options,

packages/angular_devkit/build_angular/src/builders/karma/schema.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
"items": {
127127
"type": "string"
128128
},
129+
"default": ["**/*.spec.ts"],
129130
"description": "Globs of files to include, relative to workspace or project root. \nThere are 2 special cases:\n - when a path to directory is provided, all spec files ending \".spec.@(ts|tsx)\" will be included\n - when a path to a file is provided, and a matching spec file exists it will be included instead."
130131
},
131132
"sourceMap": {

packages/angular_devkit/build_angular/src/builders/karma/tests/options/include_spec.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,15 @@ describeBuilder(execute, KARMA_BUILDER_INFO, (harness) => {
1717
include: ['abc.spec.ts', 'def.spec.ts'],
1818
});
1919

20-
const { error } = await harness.executeOnce({
21-
outputLogsOnException: false,
22-
});
23-
expect(error?.message).toBe(
24-
'Specified patterns: "abc.spec.ts, def.spec.ts" did not match any spec files.',
20+
const { result, logs } = await harness.executeOnce();
21+
expect(result?.success).toBeFalse();
22+
expect(logs).toContain(
23+
jasmine.objectContaining({
24+
level: 'error',
25+
message: jasmine.stringContaining(
26+
'Specified patterns: "abc.spec.ts, def.spec.ts" did not match any spec files.',
27+
),
28+
}),
2529
);
2630
});
2731

packages/angular_devkit/build_angular/src/webpack/configs/common.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,8 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
342342
strictExportPresence: true,
343343
parser: {
344344
javascript: {
345+
// TODO(alanagius): disable the below once we have a migration to remove `require.context` from test.ts file in users projects.
346+
requireContext: true,
345347
// Disable auto URL asset module creation. This doesn't effect `new Worker(new URL(...))`
346348
// https://webpack.js.org/guides/asset-modules/#url-assets
347349
url: false,

packages/angular_devkit/build_angular/src/webpack/plugins/karma/karma.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,10 @@ const init: any = (config: any, emitter: any) => {
124124

125125
logger.error(statsErrorsToString(statsJson, { colors: true }));
126126

127-
// Notify potential listeners of the compile error.
128-
emitter.emit('compile_error', {
129-
errors: statsJson.errors?.map((e) => e.message),
130-
});
127+
if (config.singleRun) {
128+
// Notify potential listeners of the compile error.
129+
emitter.emit('load_error');
130+
}
131131

132132
// Finish Karma run early in case of compilation error.
133133
emitter.emit('run_complete', [], { exitCode: 1 });

packages/angular_devkit/build_angular/src/webpack/plugins/single-test-transform.ts

Lines changed: 0 additions & 57 deletions
This file was deleted.

packages/angular_devkit/build_angular/test/hello-world-app/src/test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,8 @@ import {
1515
platformBrowserDynamicTesting,
1616
} from '@angular/platform-browser-dynamic/testing';
1717

18-
declare const require: {
19-
context(path: string, deep?: boolean, filter?: RegExp): {
20-
keys(): string[];
21-
<T>(id: string): T;
22-
};
23-
};
24-
2518
// First, initialize the Angular testing environment.
2619
getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), {
2720
errorOnUnknownElements: true,
2821
errorOnUnknownProperties: true
2922
});
30-
// Then we find all the tests.
31-
const context = require.context('./', true, /\.spec\.ts$/);
32-
// And load the modules.
33-
context.keys().forEach(context);

packages/angular_devkit/build_angular/test/hello-world-lib/projects/lib/src/test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,8 @@ import {
1616
platformBrowserDynamicTesting
1717
} from '@angular/platform-browser-dynamic/testing';
1818

19-
declare const require: {
20-
context(path: string, deep?: boolean, filter?: RegExp): {
21-
keys(): string[];
22-
<T>(id: string): T;
23-
};
24-
};
25-
2619
// First, initialize the Angular testing environment.
2720
getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), {
2821
errorOnUnknownElements: true,
2922
errorOnUnknownProperties: true
3023
});
31-
// Then we find all the tests.
32-
const context = require.context('./', true, /\.spec\.ts$/);
33-
// And load the modules.
34-
context.keys().forEach(context);

packages/schematics/angular/application/files/src/test.ts.template

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,8 @@ import {
77
platformBrowserDynamicTesting
88
} from '@angular/platform-browser-dynamic/testing';
99

10-
declare const require: {
11-
context(path: string, deep?: boolean, filter?: RegExp): {
12-
<T>(id: string): T;
13-
keys(): string[];
14-
};
15-
};
16-
1710
// First, initialize the Angular testing environment.
1811
getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), {
1912
errorOnUnknownElements: true,
2013
errorOnUnknownProperties: true
2114
});
22-
23-
// Then we find all the tests.
24-
const context = require.context('./', true, /\.spec\.ts$/);
25-
// And load the modules.
26-
context.keys().forEach(context);

packages/schematics/angular/library/files/src/test.ts.template

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,8 @@ import {
88
platformBrowserDynamicTesting
99
} from '@angular/platform-browser-dynamic/testing';
1010

11-
declare const require: {
12-
context(path: string, deep?: boolean, filter?: RegExp): {
13-
<T>(id: string): T;
14-
keys(): string[];
15-
};
16-
};
17-
1811
// First, initialize the Angular testing environment.
1912
getTestBed().initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), {
2013
errorOnUnknownElements: true,
2114
errorOnUnknownProperties: true
2215
});
23-
24-
// Then we find all the tests.
25-
const context = require.context('./', true, /\.spec\.ts$/);
26-
// And load the modules.
27-
context.keys().forEach(context);

0 commit comments

Comments
 (0)