Skip to content

Commit 67e42a4

Browse files
committed
refactor(@angular/build): consolidate glob calls and clarify test discovery logic
The test discovery logic previously executed a separate `glob` operation for each pattern in the `include` array. In projects with multiple test locations, this resulted in redundant and inefficient file system scans. This commit refactors the `findTests` function to perform only a single, consolidated `glob` call for all dynamic patterns, significantly improving performance. Additionally, the logic for handling static (non-glob) paths has been extracted into a dedicated function, and the entire module has been documented with JSDoc comments to improve readability and maintainability.
1 parent 00426e3 commit 67e42a4

File tree

1 file changed

+99
-37
lines changed

1 file changed

+99
-37
lines changed

packages/angular/build/src/builders/unit-test/test-discovery.ts

Lines changed: 99 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,59 @@ import { basename, dirname, extname, join, relative } from 'node:path';
1111
import { glob, isDynamicPattern } from 'tinyglobby';
1212
import { toPosixPath } from '../../utils/path';
1313

14-
/* Go through all patterns and find unique list of files */
14+
/**
15+
* Finds all test files in the project.
16+
*
17+
* @param include Glob patterns of files to include.
18+
* @param exclude Glob patterns of files to exclude.
19+
* @param workspaceRoot The absolute path to the workspace root.
20+
* @param projectSourceRoot The absolute path to the project's source root.
21+
* @returns A unique set of absolute paths to all test files.
22+
*/
1523
export async function findTests(
1624
include: string[],
1725
exclude: string[],
1826
workspaceRoot: string,
1927
projectSourceRoot: string,
2028
): Promise<string[]> {
21-
const matchingTestsPromises = include.map((pattern) =>
22-
findMatchingTests(pattern, exclude, workspaceRoot, projectSourceRoot),
29+
const staticMatches = new Set<string>();
30+
const dynamicPatterns: string[] = [];
31+
32+
const normalizedExcludes = exclude.map((p) =>
33+
normalizePattern(p, workspaceRoot, projectSourceRoot),
2334
);
24-
const files = await Promise.all(matchingTestsPromises);
2535

26-
// Unique file names
27-
return [...new Set(files.flat())];
36+
// 1. Separate static and dynamic patterns
37+
for (const pattern of include) {
38+
const normalized = normalizePattern(pattern, workspaceRoot, projectSourceRoot);
39+
if (isDynamicPattern(normalized)) {
40+
dynamicPatterns.push(normalized);
41+
} else {
42+
const result = await handleStaticPattern(normalized, projectSourceRoot);
43+
if (Array.isArray(result)) {
44+
result.forEach((file) => staticMatches.add(file));
45+
} else {
46+
// It was a static path that didn't resolve to a spec, treat as dynamic
47+
dynamicPatterns.push(result);
48+
}
49+
}
50+
}
51+
52+
// 2. Execute a single glob for all dynamic patterns
53+
if (dynamicPatterns.length > 0) {
54+
const globMatches = await glob(dynamicPatterns, {
55+
cwd: projectSourceRoot,
56+
absolute: true,
57+
ignore: ['**/node_modules/**', ...normalizedExcludes],
58+
});
59+
60+
for (const match of globMatches) {
61+
staticMatches.add(match);
62+
}
63+
}
64+
65+
// 3. Combine and de-duplicate results
66+
return [...staticMatches];
2867
}
2968

3069
interface TestEntrypointsOptions {
@@ -33,7 +72,14 @@ interface TestEntrypointsOptions {
3372
removeTestExtension?: boolean;
3473
}
3574

36-
/** Generate unique bundle names for a set of test files. */
75+
/**
76+
* Generates unique, dash-delimited bundle names for a set of test files.
77+
* This is used to create distinct output files for each test.
78+
*
79+
* @param testFiles An array of absolute paths to test files.
80+
* @param options Configuration options for generating entry points.
81+
* @returns A map where keys are the generated unique bundle names and values are the original file paths.
82+
*/
3783
export function getTestEntrypoints(
3884
testFiles: string[],
3985
{ projectSourceRoot, workspaceRoot, removeTestExtension }: TestEntrypointsOptions,
@@ -82,6 +128,10 @@ const removeRelativeRoot = (path: string, root: string): string => {
82128
return path;
83129
};
84130

131+
/**
132+
* Removes potential root paths from a file path, returning a relative path.
133+
* If no root path matches, it returns the file's basename.
134+
*/
85135
function removeRoots(path: string, roots: string[]): string {
86136
for (const root of roots) {
87137
if (path.startsWith(root)) {
@@ -92,12 +142,20 @@ function removeRoots(path: string, roots: string[]): string {
92142
return basename(path);
93143
}
94144

95-
async function findMatchingTests(
145+
/**
146+
* Normalizes a glob pattern by converting it to a POSIX path, removing leading slashes,
147+
* and making it relative to the project source root.
148+
*
149+
* @param pattern The glob pattern to normalize.
150+
* @param workspaceRoot The absolute path to the workspace root.
151+
* @param projectSourceRoot The absolute path to the project's source root.
152+
* @returns A normalized glob pattern.
153+
*/
154+
function normalizePattern(
96155
pattern: string,
97-
ignore: string[],
98156
workspaceRoot: string,
99157
projectSourceRoot: string,
100-
): Promise<string[]> {
158+
): string {
101159
// normalize pattern, glob lib only accepts forward slashes
102160
let normalizedPattern = toPosixPath(pattern);
103161
normalizedPattern = removeLeadingSlash(normalizedPattern);
@@ -106,40 +164,43 @@ async function findMatchingTests(
106164

107165
// remove relativeProjectRoot to support relative paths from root
108166
// such paths are easy to get when running scripts via IDEs
109-
normalizedPattern = removeRelativeRoot(normalizedPattern, relativeProjectRoot);
167+
return removeRelativeRoot(normalizedPattern, relativeProjectRoot);
168+
}
110169

111-
// special logic when pattern does not look like a glob
112-
if (!isDynamicPattern(normalizedPattern)) {
113-
if (await isDirectory(join(projectSourceRoot, normalizedPattern))) {
114-
normalizedPattern = `${normalizedPattern}/**/*.spec.@(ts|tsx)`;
115-
} else {
116-
// see if matching spec file exists
117-
const fileExt = extname(normalizedPattern);
118-
// Replace extension to `.spec.ext`. Example: `src/app/app.component.ts`-> `src/app/app.component.spec.ts`
119-
const potentialSpec = join(
120-
projectSourceRoot,
121-
dirname(normalizedPattern),
122-
`${basename(normalizedPattern, fileExt)}.spec${fileExt}`,
123-
);
124-
125-
if (await exists(potentialSpec)) {
126-
return [potentialSpec];
127-
}
128-
}
170+
/**
171+
* Handles static (non-glob) patterns by attempting to resolve them to a directory
172+
* of spec files or a corresponding `.spec` file.
173+
*
174+
* @param pattern The static path pattern.
175+
* @param projectSourceRoot The absolute path to the project's source root.
176+
* @returns A promise that resolves to either an array of found spec files, a new glob pattern,
177+
* or the original pattern if no special handling was applied.
178+
*/
179+
async function handleStaticPattern(
180+
pattern: string,
181+
projectSourceRoot: string,
182+
): Promise<string[] | string> {
183+
const fullPath = join(projectSourceRoot, pattern);
184+
if (await isDirectory(fullPath)) {
185+
return `${pattern}/**/*.spec.@(ts|tsx)`;
129186
}
130187

131-
// normalize the patterns in the ignore list
132-
const normalizedIgnorePatternList = ignore.map((pattern: string) =>
133-
removeRelativeRoot(removeLeadingSlash(toPosixPath(pattern)), relativeProjectRoot),
188+
const fileExt = extname(pattern);
189+
// Replace extension to `.spec.ext`. Example: `src/app/app.component.ts`-> `src/app/app.component.spec.ts`
190+
const potentialSpec = join(
191+
projectSourceRoot,
192+
dirname(pattern),
193+
`${basename(pattern, fileExt)}.spec${fileExt}`,
134194
);
135195

136-
return glob(normalizedPattern, {
137-
cwd: projectSourceRoot,
138-
absolute: true,
139-
ignore: ['**/node_modules/**', ...normalizedIgnorePatternList],
140-
});
196+
if (await exists(potentialSpec)) {
197+
return [potentialSpec];
198+
}
199+
200+
return pattern;
141201
}
142202

203+
/** Checks if a path exists and is a directory. */
143204
async function isDirectory(path: PathLike): Promise<boolean> {
144205
try {
145206
const stats = await fs.stat(path);
@@ -150,6 +211,7 @@ async function isDirectory(path: PathLike): Promise<boolean> {
150211
}
151212
}
152213

214+
/** Checks if a path exists on the file system. */
153215
async function exists(path: PathLike): Promise<boolean> {
154216
try {
155217
await fs.access(path, constants.F_OK);

0 commit comments

Comments
 (0)