From 580323c0b6e410ac86f2c3ee202f68ca98ceb51b Mon Sep 17 00:00:00 2001 From: rk Date: Tue, 29 Sep 2026 18:50:01 +0500 Subject: [PATCH] fix(@angular/build): resolve unit-test include patterns from the project root The `include` option is documented by the builder schema as relative to the project root, but patterns were only ever resolved from the project source root. A library that keeps a secondary entry point beside `src` had no supported spelling that reached its specs, and the run still reported success. A pattern that matches nothing under the source root is now retried from the project root. A pattern that already matches is not retried, so an existing pattern keeps matching exactly what it matches today. A project rooted at the workspace root is never retried, since that would search every project. Closes #33467 --- .../build/src/builders/unit-test/builder.ts | 1 + .../unit-test/runners/vitest/build-options.ts | 9 +- .../src/builders/unit-test/test-discovery.ts | 87 +++++++++++++---- .../builders/unit-test/test-discovery_spec.ts | 96 ++++++++++++++++++- 4 files changed, 173 insertions(+), 20 deletions(-) diff --git a/packages/angular/build/src/builders/unit-test/builder.ts b/packages/angular/build/src/builders/unit-test/builder.ts index bb1da57a7108..02ac220cf0f2 100644 --- a/packages/angular/build/src/builders/unit-test/builder.ts +++ b/packages/angular/build/src/builders/unit-test/builder.ts @@ -210,6 +210,7 @@ export async function* execute( normalizedOptions.exclude ?? [], normalizedOptions.workspaceRoot, normalizedOptions.projectSourceRoot, + normalizedOptions.projectRoot, ); context.logger.info('Discovered test files:'); diff --git a/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts b/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts index b3a7d8f37662..cbfe1c35b977 100644 --- a/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts +++ b/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts @@ -185,6 +185,7 @@ export async function getVitestBuildOptions( const { workspaceRoot, projectSourceRoot, + projectRoot, include, polyfills, exclude = [], @@ -194,7 +195,13 @@ export async function getVitestBuildOptions( } = options; // Find test files - const testFiles = await findTests(include, exclude, workspaceRoot, projectSourceRoot); + const testFiles = await findTests( + include, + exclude, + workspaceRoot, + projectSourceRoot, + projectRoot, + ); if (testFiles.length === 0) { throw new Error( 'No tests found matching the following patterns:\n' + diff --git a/packages/angular/build/src/builders/unit-test/test-discovery.ts b/packages/angular/build/src/builders/unit-test/test-discovery.ts index f2fc8c221646..213748fa7d8e 100644 --- a/packages/angular/build/src/builders/unit-test/test-discovery.ts +++ b/packages/angular/build/src/builders/unit-test/test-discovery.ts @@ -29,10 +29,17 @@ const MAX_FILENAME_LENGTH = 128; * test file. If a user provides a path to a directory, it will find all test * files within that directory. * + * A pattern that matches nothing under the project source root is retried from the + * project root, which is what the builder's schema documents patterns to be relative + * to. A pattern that already matches keeps matching exactly what it matches today, and + * a project rooted at the workspace root is never retried, so a search never widens to + * the whole workspace. + * * @param include Glob patterns of files to include. * @param exclude Glob patterns of files to exclude. * @param workspaceRoot The absolute path to the workspace root. * @param projectSourceRoot The absolute path to the project's source root. + * @param projectRoot The absolute path to the project root. Defaults to the source root. * @returns A unique set of absolute paths to all test files. */ export async function findTests( @@ -40,42 +47,86 @@ export async function findTests( exclude: string[], workspaceRoot: string, projectSourceRoot: string, + projectRoot = projectSourceRoot, ): Promise { await initializeHash(); const resolvedTestFiles = new Set(); - const dynamicPatterns: string[] = []; - const projectRootPrefix = toPosixPath(relative(workspaceRoot, projectSourceRoot) + '/'); - const normalizedExcludes = exclude.map((p) => normalizePattern(p, projectRootPrefix)); + const unmatched = await collectTests( + projectSourceRoot, + include, + exclude, + workspaceRoot, + resolvedTestFiles, + ); + + // Retrying from the workspace root would search every project, so it is skipped. + if (unmatched.length > 0 && projectRoot !== projectSourceRoot && projectRoot !== workspaceRoot) { + await collectTests(projectRoot, unmatched, exclude, workspaceRoot, resolvedTestFiles); + } + + return [...resolvedTestFiles]; +} + +/** + * Resolves include patterns against a single root, adding every match to `found`. + * + * @param searchRoot The absolute path the patterns are resolved against. + * @param include Glob patterns of files to include. + * @param exclude Glob patterns of files to exclude. + * @param workspaceRoot The absolute path to the workspace root. + * @param found Accumulates the absolute path of every matched file. + * @returns The patterns that matched nothing, so a caller can retry them elsewhere. + */ +async function collectTests( + searchRoot: string, + include: string[], + exclude: string[], + workspaceRoot: string, + found: Set, +): Promise { + const relativeRoot = relative(workspaceRoot, searchRoot); + const searchRootPrefix = relativeRoot ? toPosixPath(relativeRoot + '/') : ''; + const normalizedExcludes = exclude.map((p) => normalizePattern(p, searchRootPrefix)); + const unmatched: string[] = []; - // 1. Separate static and dynamic patterns for (const pattern of include) { - const normalized = normalizePattern(pattern, projectRootPrefix); + const normalized = normalizePattern(pattern, searchRootPrefix); + const dynamicPatterns: string[] = []; + + // 1. Separate static and dynamic patterns if (isDynamicPattern(pattern)) { dynamicPatterns.push(normalized); } else { - const { resolved, unresolved } = await resolveStaticPattern(normalized, projectSourceRoot); - resolved.forEach((file) => resolvedTestFiles.add(file)); - unresolved.forEach((p) => dynamicPatterns.push(p)); + const { resolved, unresolved } = await resolveStaticPattern(normalized, searchRoot); + if (resolved.length > 0) { + resolved.forEach((file) => found.add(file)); + continue; + } + dynamicPatterns.push(...unresolved); } - } - // 2. Execute a single glob for all dynamic patterns - if (dynamicPatterns.length > 0) { + // 2. Execute a glob for the pattern, so that a pattern which matches nothing here + // can be told apart from one that does and retried against another root. const globMatches = await glob(dynamicPatterns, { - cwd: projectSourceRoot, + cwd: searchRoot, absolute: true, expandDirectories: false, ignore: ['**/node_modules/**', ...normalizedExcludes], }); + if (globMatches.length === 0) { + unmatched.push(pattern); + continue; + } + + // 3. Combine and de-duplicate results for (const match of globMatches) { - resolvedTestFiles.add(toPosixPath(match)); + found.add(toPosixPath(match)); } } - // 3. Combine and de-duplicate results - return [...resolvedTestFiles]; + return unmatched; } interface TestEntrypointsOptions { @@ -255,7 +306,7 @@ function removeRoots(path: string, roots: string[]): string { * slashes, and making it relative to the project source root. * * @param pattern The glob pattern to normalize. - * @param projectRootPrefix The POSIX-formatted prefix of the project's source root relative to the workspace root. + * @param projectRootPrefix The POSIX-formatted prefix of the search root relative to the workspace root. * @returns A normalized glob pattern. */ function normalizePattern(pattern: string, projectRootPrefix: string): string { @@ -266,8 +317,8 @@ function normalizePattern(pattern: string, projectRootPrefix: string): string { return posixPattern; } - // For relative paths, ensure they are correctly relative to the project source root. - // This involves removing the project root prefix if the user provided a workspace-relative path. + // For relative paths, ensure they are correctly relative to the search root. + // This involves removing the root prefix if the user provided a workspace-relative path. const normalizedRelative = removePrefix(posixPattern, projectRootPrefix); return normalizedRelative; diff --git a/packages/angular/build/src/builders/unit-test/test-discovery_spec.ts b/packages/angular/build/src/builders/unit-test/test-discovery_spec.ts index 764924d9552b..fb10fcf30984 100644 --- a/packages/angular/build/src/builders/unit-test/test-discovery_spec.ts +++ b/packages/angular/build/src/builders/unit-test/test-discovery_spec.ts @@ -6,8 +6,11 @@ * found in the LICENSE file at https://angular.dev/license */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; import { initializeHash } from '../../utils/hash'; -import { generateNameFromPath, getTestEntrypoints } from './test-discovery'; +import { findTests, generateNameFromPath, getTestEntrypoints } from './test-discovery'; describe('getTestEntrypoints', () => { beforeAll(async () => { @@ -170,3 +173,94 @@ describe('generateNameFromPath', () => { expect(result).toBe(name); }); }); + +describe('findTests', () => { + let workspaceRoot: string; + let projectRoot: string; + let projectSourceRoot: string; + let insideSourceRoot: string; + let outsideSourceRoot: string; + + beforeAll(async () => { + await initializeHash(); + + // The macOS temporary directory is a symlink, and the globber does not resolve one. + workspaceRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'find-tests-'))); + projectRoot = path.join(workspaceRoot, 'projects', 'my-lib'); + projectSourceRoot = path.join(projectRoot, 'src'); + insideSourceRoot = path.join(projectSourceRoot, 'lib', 'inside.spec.ts'); + outsideSourceRoot = path.join(projectRoot, 'secondary', 'outside.spec.ts'); + + for (const file of [insideSourceRoot, outsideSourceRoot]) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, ''); + } + }); + + afterAll(() => { + fs.rmSync(workspaceRoot, { recursive: true, force: true }); + }); + + it('should find a spec below the source root', async () => { + const found = await findTests( + ['**/*.spec.ts'], + [], + workspaceRoot, + projectSourceRoot, + projectRoot, + ); + + expect(found).toEqual([insideSourceRoot]); + }); + + it('should find a spec below the project root but outside the source root', async () => { + const found = await findTests( + ['secondary/**/*.spec.ts'], + [], + workspaceRoot, + projectSourceRoot, + projectRoot, + ); + + expect(found).toEqual([outsideSourceRoot]); + }); + + it('should find a static path below the project root', async () => { + const found = await findTests( + ['secondary/outside.spec.ts'], + [], + workspaceRoot, + projectSourceRoot, + projectRoot, + ); + + expect(found).toEqual([outsideSourceRoot]); + }); + + it('should not widen a pattern that already matches below the source root', async () => { + const found = await findTests( + ['**/*.spec.ts', 'secondary/**/*.spec.ts'], + [], + workspaceRoot, + projectSourceRoot, + projectRoot, + ); + + // The first pattern stays anchored at the source root, so it must not also + // collect the file the second pattern is there to reach. + expect(found).toEqual([insideSourceRoot, outsideSourceRoot]); + }); + + it('should not retry from a project root that is the workspace root', async () => { + // Retrying there would search every project in the workspace. + const found = await findTests( + ['projects/my-lib/secondary/**/*.spec.ts'], + [], + workspaceRoot, + path.join(workspaceRoot, 'src'), + workspaceRoot, + ); + + expect(found).toEqual([]); + }); +});