Skip to content

Commit aed407d

Browse files
committed
fix(@schematics/angular): defer karma config deletion in Karma to Vitest migration
This commit resolves a race condition in the Karma-to-Vitest migration where shared configuration files were deleted prematurely within the schematic transaction. Previously, the routine erased the target file upon discovering it was identical to boilerplate, which blinded other referring projects to extraction metadata. The solution introduces analysis object caching so AST parsing occurs exactly once per discrete path. Deletion operations are deferred and batch-executed at the end of workspace processing. This mechanism guarantees continuous readability across iterating dependencies and yields operational speed improvements via memoized checks.
1 parent edfa782 commit aed407d

3 files changed

Lines changed: 98 additions & 25 deletions

File tree

packages/schematics/angular/migrations/migrate-karma-to-vitest/karma-processor.ts

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,42 +119,47 @@ function extractCoverageSettings(
119119
}
120120
}
121121

122+
export interface KarmaConfigProcessingResult {
123+
analysis: KarmaConfigAnalysis;
124+
isRemovable: boolean;
125+
}
126+
122127
export async function processKarmaConfig(
123128
karmaConfig: string,
124129
options: Record<string, json.JsonValue | undefined>,
125130
projectName: string,
126131
context: SchematicContext,
127132
tree: Tree,
128-
removableKarmaConfigs: Map<string, boolean>,
133+
cache: Map<string, KarmaConfigProcessingResult>,
129134
needDevkitPlugin: boolean,
130135
manualMigrationFiles: string[],
131136
): Promise<void> {
132-
if (tree.exists(karmaConfig)) {
137+
let cachedResult = cache.get(karmaConfig);
138+
139+
if (!cachedResult && tree.exists(karmaConfig)) {
133140
const content = tree.readText(karmaConfig);
134141
const analysis = analyzeKarmaConfig(content);
135142

136-
extractReporters(analysis, options, projectName, context);
137-
extractCoverageSettings(analysis, options, projectName, context);
138-
139-
let isRemovable = removableKarmaConfigs.get(karmaConfig);
140-
if (isRemovable === undefined) {
141-
if (analysis.hasUnsupportedValues) {
142-
isRemovable = false;
143-
} else {
144-
const diff = await compareKarmaConfigToDefault(
145-
analysis,
146-
projectName,
147-
karmaConfig,
148-
needDevkitPlugin,
149-
);
150-
isRemovable = !hasDifferences(diff) && diff.isReliable;
151-
}
152-
removableKarmaConfigs.set(karmaConfig, isRemovable);
143+
let isRemovable = false;
144+
if (!analysis.hasUnsupportedValues) {
145+
const diff = await compareKarmaConfigToDefault(
146+
analysis,
147+
projectName,
148+
karmaConfig,
149+
needDevkitPlugin,
150+
);
151+
isRemovable = !hasDifferences(diff) && diff.isReliable;
153152
}
154153

155-
if (isRemovable) {
156-
tree.delete(karmaConfig);
157-
} else {
154+
cachedResult = { analysis, isRemovable };
155+
cache.set(karmaConfig, cachedResult);
156+
}
157+
158+
if (cachedResult) {
159+
extractReporters(cachedResult.analysis, options, projectName, context);
160+
extractCoverageSettings(cachedResult.analysis, options, projectName, context);
161+
162+
if (!cachedResult.isRemovable) {
158163
context.logger.warn(
159164
`Project "${projectName}" uses a custom Karma configuration file "${karmaConfig}". ` +
160165
`Tests have been migrated to use Vitest, but you may need to manually migrate custom settings ` +
@@ -164,5 +169,6 @@ export async function processKarmaConfig(
164169
manualMigrationFiles.push(karmaConfig);
165170
}
166171
}
172+
167173
delete options['karmaConfig'];
168174
}

packages/schematics/angular/migrations/migrate-karma-to-vitest/migration.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import { latestVersions } from '../../utility/latest-versions';
1414
import { TargetDefinition, allTargetOptions, updateWorkspace } from '../../utility/workspace';
1515
import { Builders } from '../../utility/workspace-models';
1616
import { BUILD_OPTIONS_KEYS } from './constants';
17-
import { processKarmaConfig } from './karma-processor';
17+
import { KarmaConfigProcessingResult, processKarmaConfig } from './karma-processor';
1818

1919
async function processTestTargetOptions(
2020
testTarget: TargetDefinition,
2121
projectName: string,
2222
context: SchematicContext,
2323
tree: Tree,
24-
removableKarmaConfigs: Map<string, boolean>,
24+
removableKarmaConfigs: Map<string, KarmaConfigProcessingResult>,
2525
customBuildOptions: Record<string, Record<string, json.JsonValue | undefined>>,
2626
needDevkitPlugin: boolean,
2727
manualMigrationFiles: string[],
@@ -122,7 +122,7 @@ async function processTestTargetOptions(
122122
function updateProjects(tree: Tree, context: SchematicContext): Rule {
123123
return updateWorkspace(async (workspace) => {
124124
let needsCoverage = false;
125-
const removableKarmaConfigs = new Map<string, boolean>();
125+
const removableKarmaConfigs = new Map<string, KarmaConfigProcessingResult>();
126126
const migratedProjects: string[] = [];
127127
const skippedNonApplications: string[] = [];
128128
const skippedMissingAppBuilder: string[] = [];
@@ -235,6 +235,13 @@ function updateProjects(tree: Tree, context: SchematicContext): Rule {
235235
migratedProjects.push(projectName);
236236
}
237237

238+
// Perform cleanup of removable karma config files
239+
for (const [configPath, result] of removableKarmaConfigs) {
240+
if (result.isRemovable && tree.exists(configPath)) {
241+
tree.delete(configPath);
242+
}
243+
}
244+
238245
// Log summary
239246
context.logger.info('\n--- Karma to Vitest Migration Summary ---');
240247
context.logger.info(`Projects migrated: ${migratedProjects.length}`);

packages/schematics/angular/migrations/migrate-karma-to-vitest/migration_spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ module.exports = function (config) {
322322
const newTree = await schematicRunner.runSchematic('migrate-karma-to-vitest', {}, tree);
323323
expect(newTree.exists('karma.conf.js')).toBeFalse();
324324
});
325+
325326
it('should shift main compilation entry file directly into setupFiles array', async () => {
326327
const { projects } = tree.readJson('/angular.json') as any;
327328
projects.app.targets.test.options.main = 'src/test.ts';
@@ -333,6 +334,7 @@ module.exports = function (config) {
333334
expect(newProjects.app.targets.test.options.setupFiles).toEqual(['src/test.ts']);
334335
expect(newProjects.app.targets.test.options.main).toBeUndefined();
335336
});
337+
336338
it('should generate unique testing configuration name preventing collision overwrites', async () => {
337339
const { projects } = tree.readJson('/angular.json') as any;
338340
projects.app.targets.build.configurations = {
@@ -350,6 +352,7 @@ module.exports = function (config) {
350352
]);
351353
expect(newProjects.app.targets.test.options.buildTarget).toBe(':build:testing-2');
352354
});
355+
353356
it('should inject @vitest/coverage-v8 whenever coverage presence is detected', async () => {
354357
const { projects } = tree.readJson('/angular.json') as any;
355358
projects.app.targets.test.options.codeCoverage = true;
@@ -360,4 +363,61 @@ module.exports = function (config) {
360363

361364
expect(devDependencies['@vitest/coverage-v8']).toBe(latestVersions['@vitest/coverage-v8']);
362365
});
366+
367+
it('should successfully extract settings across multiple projects sharing the same removable karma config', async () => {
368+
const { projects } = tree.readJson('/angular.json') as any;
369+
projects.app.targets.test.builder = '@angular-devkit/build-angular:karma';
370+
371+
// Add a second project sharing the exact same configuration
372+
projects.app2 = {
373+
...JSON.parse(JSON.stringify(projects.app)),
374+
root: 'app2',
375+
};
376+
377+
tree.overwrite('/angular.json', JSON.stringify({ version: 1, projects }));
378+
379+
const DEFAULT_KARMA_CONFIG = `
380+
module.exports = function (config) {
381+
config.set({
382+
basePath: '',
383+
frameworks: ['jasmine', '@angular-devkit/build-angular'],
384+
plugins: [
385+
require('karma-jasmine'),
386+
require('karma-chrome-launcher'),
387+
require('karma-jasmine-html-reporter'),
388+
require('karma-coverage'),
389+
require('@angular-devkit/build-angular/plugins/karma')
390+
],
391+
client: {
392+
jasmine: {},
393+
},
394+
jasmineHtmlReporter: {
395+
suppressAll: true
396+
},
397+
coverageReporter: {
398+
dir: require('path').join(__dirname, './coverage/app'),
399+
subdir: '.',
400+
reporters: [
401+
{ type: 'html' },
402+
{ type: 'text-summary' }
403+
]
404+
},
405+
reporters: ['progress', 'kjhtml'],
406+
browsers: ['Chrome'],
407+
restartOnFileChange: true
408+
});
409+
};
410+
`;
411+
tree.create('karma.conf.js', DEFAULT_KARMA_CONFIG);
412+
413+
const newTree = await schematicRunner.runSchematic('migrate-karma-to-vitest', {}, tree);
414+
const { projects: newProjects } = newTree.readJson('/angular.json') as any;
415+
416+
// Assert BOTH projects got the extraction logic mapped correctly
417+
expect(newProjects.app.targets.test.options.reporters).toEqual(['default']);
418+
expect(newProjects.app2.targets.test.options.reporters).toEqual(['default']);
419+
420+
// Assert that the deletion deferred successfully until BOTH extracted the data
421+
expect(newTree.exists('karma.conf.js')).toBeFalse();
422+
});
363423
});

0 commit comments

Comments
 (0)