Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

fix(@angular/build): fix setup files duplicate modules #31791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yjaaidi wants to merge 2 commits into angular:main
base: main
Choose a base branch
Loading
from yjaaidi:fix/fix-test-bed-usage-in-setup-files
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { toPosixPath } from '../../../../utils/path';
import type { ApplicationBuilderInternalOptions } from '../../../application/options';
import { OutputHashing } from '../../../application/schema';
import { NormalizedUnitTestBuilderOptions, injectTestingPolyfills } from '../../options';
import { findTests, getTestEntrypoints } from '../../test-discovery';
import { findTests, getSetupEntrypoints, getTestEntrypoints } from '../../test-discovery';
import { RunnerOptions } from '../api';

function createTestBedInitVirtualFile(
Expand Down Expand Up @@ -88,12 +88,19 @@ export async function getVitestBuildOptions(
);
}

const entryPoints = getTestEntrypoints(testFiles, {
const testEntryPoints = getTestEntrypoints(testFiles, {
projectSourceRoot,
workspaceRoot,
removeTestExtension: true,
});
entryPoints.set('init-testbed', 'angular:test-bed-init');
const setupEntryPoints = getSetupEntrypoints(options.setupFiles, {
projectSourceRoot,
workspaceRoot,
removeTestExtension: true,
});
setupEntryPoints.set('init-testbed', 'angular:test-bed-init');

const entryPoints = new Map([...testEntryPoints, ...setupEntryPoints]);

// The 'vitest' package is always external for testing purposes
const externalDependencies = ['vitest'];
Expand Down Expand Up @@ -138,6 +145,6 @@ export async function getVitestBuildOptions(
virtualFiles: {
'angular:test-bed-init': testBedInitContents,
},
testEntryPointMappings: entryPoints,
testEntryPointMappings: testEntryPoints,
};
}
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import { setupBrowserConfiguration } from './browser-provider';
import { findVitestBaseConfig } from './configuration';
import { createVitestConfigPlugin, createVitestPlugins } from './plugins';
import { getSetupEntrypoints } from '../../test-discovery';

Check failure on line 29 in packages/angular/build/src/builders/unit-test/runners/vitest/executor.ts

View workflow job for this annotation

GitHub Actions / lint 

`../../test-discovery` import should occur before type import of `../api`

export class VitestExecutor implements TestExecutor {
private vitest: Vitest | undefined;
Expand Down Expand Up @@ -127,9 +128,19 @@
}

private prepareSetupFiles(): string[] {
const { setupFiles } = this.options;
const { setupFiles, workspaceRoot } = this.options;

const setupFilesEntrypoints = getSetupEntrypoints(setupFiles, {
projectSourceRoot: this.options.projectSourceRoot,
workspaceRoot,
removeTestExtension: true,
});
const setupFileNames = Array.from(setupFilesEntrypoints.keys()).map(
(entrypoint) => `${entrypoint}.js`,
);

// Add setup file entries for TestBed initialization and project polyfills
const testSetupFiles = ['init-testbed.js', ...setupFiles];
const testSetupFiles = ['init-testbed.js', ...setupFileNames];

// TODO: Provide additional result metadata to avoid needing to extract based on filename
if (this.buildResultFiles.has('polyfills.js')) {
Expand Down
48 changes: 41 additions & 7 deletions packages/angular/build/src/builders/unit-test/test-discovery.ts
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export async function findTests(
return [...resolvedTestFiles];
}

interface TestEntrypointsOptions {
interface EntrypointsOptions {
projectSourceRoot: string;
workspaceRoot: string;
removeTestExtension?: boolean;
Expand All @@ -89,15 +89,49 @@ interface TestEntrypointsOptions {
*/
export function getTestEntrypoints(
testFiles: string[],
{ projectSourceRoot, workspaceRoot, removeTestExtension }: TestEntrypointsOptions,
{ projectSourceRoot, workspaceRoot, removeTestExtension }: EntrypointsOptions,
): Map<string, string> {
return getEntrypoints(testFiles, {
projectSourceRoot,
workspaceRoot,
removeTestExtension,
prefix: 'spec',
});
}

/**
* @param setupFiles An array of absolute paths to setup files.
* @param options Configuration options for generating entry points.
* @returns A map where keys are the generated unique bundle names and values are the original file paths.
*/
export function getSetupEntrypoints(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy with this.
I would rather have a single getEntrypoints that returns both test files and setup files then we would have to forward the setup file names to the executor somehow.

setupFiles: string[],
{ projectSourceRoot, workspaceRoot, removeTestExtension }: EntrypointsOptions,
): Map<string, string> {
return getEntrypoints(setupFiles, {
projectSourceRoot,
workspaceRoot,
removeTestExtension,
prefix: 'setup',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This produces bundles named setup-test-setup which is weird.
An empty string would be better but collisions could happen with a setup file called spec-setup... or something.

});
}

function getEntrypoints(
files: string[],
{
projectSourceRoot,
workspaceRoot,
removeTestExtension,
prefix,
}: EntrypointsOptions & { prefix: string },
): Map<string, string> {
const seen = new Set<string>();
const roots = [projectSourceRoot, workspaceRoot];

return new Map(
Array.from(testFiles, (testFile) => {
const fileName = generateNameFromPath(testFile, roots, !!removeTestExtension);
const baseName = `spec-${fileName}`;
Array.from(files, (setupFile) => {
const fileName = generateNameFromPath(setupFile, roots, !!removeTestExtension);
const baseName = `${prefix}-${fileName}`;
let uniqueName = baseName;
let suffix = 2;
while (seen.has(uniqueName)) {
Expand All @@ -106,7 +140,7 @@ export function getTestEntrypoints(
}
seen.add(uniqueName);

return [uniqueName, testFile];
return [uniqueName, setupFile];
}),
);
}
Expand Down Expand Up @@ -136,7 +170,7 @@ function generateNameFromPath(
let endIndex = relativePath.length;
if (removeTestExtension) {
const infixes = TEST_FILE_INFIXES.map((p) => p.substring(1)).join('|');
const match = relativePath.match(new RegExp(`\\.(${infixes})\\.[^.]+$`));
const match = relativePath.match(new RegExp(`\\.((${infixes})\\.)?[^.]+$`));

if (match?.index) {
endIndex = match.index;
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ import { execute } from '../../index';
import {
BASE_OPTIONS,
describeBuilder,
UNIT_TEST_BUILDER_INFO,
setupApplicationTarget,
expectLog,
UNIT_TEST_BUILDER_INFO,
} from '../setup';

describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
xdescribe('Option: "setupFiles"', () => {
describe('Option: "setupFiles"', () => {
beforeEach(async () => {
setupApplicationTarget(harness);
});
Expand All @@ -27,31 +26,84 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
setupFiles: ['src/setup.ts'],
});

const { result, error } = await harness.executeOnce({ outputLogsOnFailure: false });
expect(result).toBeUndefined();
expect(error?.message).toMatch(`The specified setup file "src/setup.ts" does not exist.`);
const { result } = await harness.executeOnce();
expect(result?.success).toBeFalse();
// TODO: Re-enable once Vite logs are remapped through build system
// expectLog(logs, `The specified setup file "src/setup.ts" does not exist.`);
});

it('should include the setup files', async () => {
await harness.writeFiles({
'src/setup.ts': `console.log('Hello from setup.ts');`,
'src/setup.ts': `(globalThis as any).setupLoaded = true;`,
'src/app/app.component.spec.ts': `
import { describe, expect, test } from 'vitest'
describe('AppComponent', () => {
test('should create the app', () => {
expect(true).toBe(true);
expect((globalThis as any).setupLoaded).toBe(true);
});
});`,
});

await harness.modifyFile('src/tsconfig.spec.json', (content) => {
const tsConfig = JSON.parse(content);
tsConfig.files ??= [];
tsConfig.files.push('setup.ts');
return JSON.stringify(tsConfig);
});

harness.useTarget('test', {
...BASE_OPTIONS,
setupFiles: ['src/setup.ts'],
});

const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
});

it('should allow setup files to configure testing module', async () => {
await harness.writeFiles({
'src/setup.ts': `
import { TestBed } from '@angular/core/testing';
import { beforeEach } from 'vitest';
import { SETUP_LOADED_TOKEN } from './setup-loaded-token';

beforeEach(() => {
TestBed.configureTestingModule({
providers: [{provide: SETUP_LOADED_TOKEN, useValue: true}],
});
});
`,
'src/setup-loaded-token.ts': `
import { InjectionToken } from '@angular/core';

export const SETUP_LOADED_TOKEN = new InjectionToken<boolean>('SETUP_LOADED_TOKEN');
`,
'src/app/app.component.spec.ts': `
import { beforeEach, describe, expect, test } from 'vitest';
import { TestBed } from '@angular/core/testing';
import { SETUP_LOADED_TOKEN } from '../setup-loaded-token';

describe('AppComponent', () => {
test('should create the app', () => {
expect(TestBed.inject(SETUP_LOADED_TOKEN)).toBe(true);
});
});`,
});

await harness.modifyFile('src/tsconfig.spec.json', (content) => {
const tsConfig = JSON.parse(content);
tsConfig.files ??= [];
tsConfig.files.push('setup.ts');
return JSON.stringify(tsConfig);
});

harness.useTarget('test', {
...BASE_OPTIONS,
setupFiles: ['src/setup.ts'],
});

const { result, logs } = await harness.executeOnce();
const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
expectLog(logs, 'Hello from setup.ts');
});
});
});
Loading

AltStyle によって変換されたページ (->オリジナル) /