Skip to content

Commit 982d6c5

Browse files
clydinclydin
committed
refactor(@angular/build): improve readability of Vitest executor
The `initializeVitest` method within the Vitest test executor was overly large and handled multiple distinct responsibilities, including setup file preparation and Vite plugin configuration. This change refactors the method by extracting these responsibilities into separate private methods: `prepareSetupFiles` and `createVitestPlugins`. This improves the overall readability, testability, and maintainability of the executor by better separating concerns without changing its behavior.
1 parent bf4f042 commit 982d6c5

File tree

2 files changed

+150
-133
lines changed

2 files changed

+150
-133
lines changed

‎packages/angular/build/BUILD.bazel‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ jasmine_test(
320320

321321
jasmine_test(
322322
name = "unit-test_integration_tests",
323-
size = "small",
323+
size = "medium",
324324
data = [":unit-test_integration_test_lib"],
325325
shard_count = 5,
326326
)

‎packages/angular/build/src/builders/unit-test/runners/vitest/executor.ts‎

Lines changed: 149 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,153 @@ export class VitestExecutor implements TestExecutor {
116116
await this.vitest?.close();
117117
}
118118

119+
private prepareSetupFiles(): string[] {
120+
const { setupFiles } = this.options;
121+
// Add setup file entries for TestBed initialization and project polyfills
122+
const testSetupFiles = ['init-testbed.js', ...setupFiles];
123+
124+
// TODO: Provide additional result metadata to avoid needing to extract based on filename
125+
if (this.buildResultFiles.has('polyfills.js')) {
126+
testSetupFiles.unshift('polyfills.js');
127+
}
128+
129+
return testSetupFiles;
130+
}
131+
132+
private createVitestPlugins(
133+
testSetupFiles: string[],
134+
browserOptions: Awaited<ReturnType<typeof setupBrowserConfiguration>>,
135+
): NonNullable<import('vite').PluginOption>[] {
136+
const { workspaceRoot, codeCoverage } = this.options;
137+
138+
return [
139+
{
140+
name: 'angular:project-init',
141+
// Type is incorrect. This allows a Promise<void>.
142+
// eslint-disable-next-line @typescript-eslint/no-misused-promises
143+
configureVitest: async (context) => {
144+
// Create a subproject that can be configured with plugins for browser mode.
145+
// Plugins defined directly in the vite overrides will not be present in the
146+
// browser specific Vite instance.
147+
const [project] = await context.injectTestProjects({
148+
test: {
149+
name: this.projectName,
150+
root: workspaceRoot,
151+
globals: true,
152+
setupFiles: testSetupFiles,
153+
// Use `jsdom` if no browsers are explicitly configured.
154+
// `node` is effectively no "environment" and the default.
155+
environment: browserOptions.browser ? 'node' : 'jsdom',
156+
browser: browserOptions.browser,
157+
include: this.options.include,
158+
...(this.options.exclude ? { exclude: this.options.exclude } : {}),
159+
},
160+
plugins: [
161+
{
162+
name: 'angular:test-in-memory-provider',
163+
enforce: 'pre',
164+
resolveId: (id, importer) => {
165+
if (importer && id.startsWith('.')) {
166+
let fullPath;
167+
let relativePath;
168+
if (this.testFileToEntryPoint.has(importer)) {
169+
fullPath = toPosixPath(path.join(this.options.workspaceRoot, id));
170+
relativePath = path.normalize(id);
171+
} else {
172+
fullPath = toPosixPath(path.join(path.dirname(importer), id));
173+
relativePath = path.relative(this.options.workspaceRoot, fullPath);
174+
}
175+
if (this.buildResultFiles.has(toPosixPath(relativePath))) {
176+
return fullPath;
177+
}
178+
}
179+
180+
if (this.testFileToEntryPoint.has(id)) {
181+
return id;
182+
}
183+
184+
assert(
185+
this.buildResultFiles.size > 0,
186+
'buildResult must be available for resolving.',
187+
);
188+
const relativePath = path.relative(this.options.workspaceRoot, id);
189+
if (this.buildResultFiles.has(toPosixPath(relativePath))) {
190+
return id;
191+
}
192+
},
193+
load: async (id) => {
194+
assert(
195+
this.buildResultFiles.size > 0,
196+
'buildResult must be available for in-memory loading.',
197+
);
198+
199+
// Attempt to load as a source test file.
200+
const entryPoint = this.testFileToEntryPoint.get(id);
201+
let outputPath;
202+
if (entryPoint) {
203+
outputPath = entryPoint + '.js';
204+
} else {
205+
// Attempt to load as a built artifact.
206+
const relativePath = path.relative(this.options.workspaceRoot, id);
207+
outputPath = toPosixPath(relativePath);
208+
}
209+
210+
const outputFile = this.buildResultFiles.get(outputPath);
211+
if (outputFile) {
212+
const sourceMapPath = outputPath + '.map';
213+
const sourceMapFile = this.buildResultFiles.get(sourceMapPath);
214+
const code =
215+
outputFile.origin === 'memory'
216+
? Buffer.from(outputFile.contents).toString('utf-8')
217+
: await readFile(outputFile.inputPath, 'utf-8');
218+
const map = sourceMapFile
219+
? sourceMapFile.origin === 'memory'
220+
? Buffer.from(sourceMapFile.contents).toString('utf-8')
221+
: await readFile(sourceMapFile.inputPath, 'utf-8')
222+
: undefined;
223+
224+
return {
225+
code,
226+
map: map ? JSON.parse(map) : undefined,
227+
};
228+
}
229+
},
230+
},
231+
{
232+
name: 'angular:html-index',
233+
transformIndexHtml: () => {
234+
// Add all global stylesheets
235+
if (this.buildResultFiles.has('styles.css')) {
236+
return [
237+
{
238+
tag: 'link',
239+
attrs: { href: 'styles.css', rel: 'stylesheet' },
240+
injectTo: 'head',
241+
},
242+
];
243+
}
244+
245+
return [];
246+
},
247+
},
248+
],
249+
});
250+
251+
// Adjust coverage excludes to not include the otherwise automatically inserted included unit tests.
252+
// Vite does this as a convenience but is problematic for the bundling strategy employed by the
253+
// builder's test setup. To workaround this, the excludes are adjusted here to only automatically
254+
// exclude the TypeScript source test files.
255+
project.config.coverage.exclude = [
256+
...(codeCoverage?.exclude ?? []),
257+
'**/*.{test,spec}.?(c|m)ts',
258+
];
259+
},
260+
},
261+
];
262+
}
263+
119264
private async initializeVitest(): Promise<Vitest> {
120-
const { codeCoverage, reporters, workspaceRoot, setupFiles, browsers, debug, watch } =
121-
this.options;
265+
const { codeCoverage, reporters, workspaceRoot, browsers, debug, watch } = this.options;
122266

123267
let vitestNodeModule;
124268
try {
@@ -148,13 +292,9 @@ export class VitestExecutor implements TestExecutor {
148292
this.buildResultFiles.size > 0,
149293
'buildResult must be available before initializing vitest',
150294
);
151-
// Add setup file entries for TestBed initialization and project polyfills
152-
const testSetupFiles = ['init-testbed.js', ...setupFiles];
153295

154-
// TODO: Provide additional result metadata to avoid needing to extract based on filename
155-
if (this.buildResultFiles.has('polyfills.js')) {
156-
testSetupFiles.unshift('polyfills.js');
157-
}
296+
const testSetupFiles = this.prepareSetupFiles();
297+
const plugins = this.createVitestPlugins(testSetupFiles, browserOptions);
158298

159299
const debugOptions = debug
160300
? {
@@ -185,130 +325,7 @@ export class VitestExecutor implements TestExecutor {
185325
// be enabled as it controls other internal behavior related to rerunning tests.
186326
watch: null,
187327
},
188-
plugins: [
189-
{
190-
name: 'angular:project-init',
191-
// Type is incorrect. This allows a Promise<void>.
192-
// eslint-disable-next-line @typescript-eslint/no-misused-promises
193-
configureVitest: async (context) => {
194-
// Create a subproject that can be configured with plugins for browser mode.
195-
// Plugins defined directly in the vite overrides will not be present in the
196-
// browser specific Vite instance.
197-
const [project] = await context.injectTestProjects({
198-
test: {
199-
name: this.projectName,
200-
root: workspaceRoot,
201-
globals: true,
202-
setupFiles: testSetupFiles,
203-
// Use `jsdom` if no browsers are explicitly configured.
204-
// `node` is effectively no "environment" and the default.
205-
environment: browserOptions.browser ? 'node' : 'jsdom',
206-
browser: browserOptions.browser,
207-
include: this.options.include,
208-
...(this.options.exclude ? { exclude: this.options.exclude } : {}),
209-
},
210-
plugins: [
211-
{
212-
name: 'angular:test-in-memory-provider',
213-
enforce: 'pre',
214-
resolveId: (id, importer) => {
215-
if (importer && id.startsWith('.')) {
216-
let fullPath;
217-
let relativePath;
218-
if (this.testFileToEntryPoint.has(importer)) {
219-
fullPath = toPosixPath(path.join(this.options.workspaceRoot, id));
220-
relativePath = path.normalize(id);
221-
} else {
222-
fullPath = toPosixPath(path.join(path.dirname(importer), id));
223-
relativePath = path.relative(this.options.workspaceRoot, fullPath);
224-
}
225-
if (this.buildResultFiles.has(toPosixPath(relativePath))) {
226-
return fullPath;
227-
}
228-
}
229-
230-
if (this.testFileToEntryPoint.has(id)) {
231-
return id;
232-
}
233-
234-
assert(
235-
this.buildResultFiles.size > 0,
236-
'buildResult must be available for resolving.',
237-
);
238-
const relativePath = path.relative(this.options.workspaceRoot, id);
239-
if (this.buildResultFiles.has(toPosixPath(relativePath))) {
240-
return id;
241-
}
242-
},
243-
load: async (id) => {
244-
assert(
245-
this.buildResultFiles.size > 0,
246-
'buildResult must be available for in-memory loading.',
247-
);
248-
249-
// Attempt to load as a source test file.
250-
const entryPoint = this.testFileToEntryPoint.get(id);
251-
let outputPath;
252-
if (entryPoint) {
253-
outputPath = entryPoint + '.js';
254-
} else {
255-
// Attempt to load as a built artifact.
256-
const relativePath = path.relative(this.options.workspaceRoot, id);
257-
outputPath = toPosixPath(relativePath);
258-
}
259-
260-
const outputFile = this.buildResultFiles.get(outputPath);
261-
if (outputFile) {
262-
const sourceMapPath = outputPath + '.map';
263-
const sourceMapFile = this.buildResultFiles.get(sourceMapPath);
264-
const code =
265-
outputFile.origin === 'memory'
266-
? Buffer.from(outputFile.contents).toString('utf-8')
267-
: await readFile(outputFile.inputPath, 'utf-8');
268-
const map = sourceMapFile
269-
? sourceMapFile.origin === 'memory'
270-
? Buffer.from(sourceMapFile.contents).toString('utf-8')
271-
: await readFile(sourceMapFile.inputPath, 'utf-8')
272-
: undefined;
273-
274-
return {
275-
code,
276-
map: map ? JSON.parse(map) : undefined,
277-
};
278-
}
279-
},
280-
},
281-
{
282-
name: 'angular:html-index',
283-
transformIndexHtml: () => {
284-
// Add all global stylesheets
285-
if (this.buildResultFiles.has('styles.css')) {
286-
return [
287-
{
288-
tag: 'link',
289-
attrs: { href: 'styles.css', rel: 'stylesheet' },
290-
injectTo: 'head',
291-
},
292-
];
293-
}
294-
295-
return [];
296-
},
297-
},
298-
],
299-
});
300-
301-
// Adjust coverage excludes to not include the otherwise automatically inserted included unit tests.
302-
// Vite does this as a convenience but is problematic for the bundling strategy employed by the
303-
// builder's test setup. To workaround this, the excludes are adjusted here to only automatically
304-
// exclude the TypeScript source test files.
305-
project.config.coverage.exclude = [
306-
...(codeCoverage?.exclude ?? []),
307-
'**/*.{test,spec}.?(c|m)ts',
308-
];
309-
},
310-
},
311-
],
328+
plugins,
312329
},
313330
);
314331
}

0 commit comments

Comments
 (0)