Skip to content

Commit ac38d99

Browse files
committed
fix(migrate): preserve Vitest config and assertion semantics
1 parent e769acc commit ac38d99

4 files changed

Lines changed: 232 additions & 16 deletions

File tree

packages/cli/src/migration/__tests__/vitest-v5.spec.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,98 @@ export default defineConfig({ test: { projects: [
203203
expect(config(input, false).content).toBe(input);
204204
});
205205

206+
it.each([
207+
`export default mergeConfig(
208+
defineConfig({ test: { clearMocks: true } }),
209+
defineConfig({ test: { environment: 'jsdom' } }),
210+
);`,
211+
`const base = defineConfig({ test: { clearMocks: true } });
212+
const overrides = defineConfig({ test: { environment: 'jsdom' } });
213+
export default mergeConfig(base, overrides);`,
214+
`export default defineConfig(mergeConfig(
215+
{ test: { clearMocks: true } },
216+
defineProject({ test: { environment: 'jsdom' } }),
217+
));`,
218+
])('does not insert defaults into merged config fragments: %s', (body) => {
219+
const input = `import { defineConfig, defineProject, mergeConfig } from 'vitest/config';\n${body}`;
220+
const result = config(input);
221+
expect(result.content).toBe(input);
222+
expect(result.findings).toContainEqual(
223+
expect.objectContaining({ code: 'merged-config-defaults', severity: 'review' }),
224+
);
225+
});
226+
227+
it('leaves defaults in imported merge fragments for review across migration runs', () => {
228+
const base = `import { defineConfig } from 'vitest/config';
229+
export default defineConfig({ test: { clearMocks: true, browser: { locators: { exact: true } } } });`;
230+
const overrides = `import { defineConfig } from 'vitest/config';
231+
export default defineConfig({ test: { browser: { enabled: true } } });`;
232+
const root = project({
233+
'vitest.config.ts': `import { mergeConfig } from 'vitest/config';
234+
import base from './base';
235+
import overrides from './overrides';
236+
export default mergeConfig(base, overrides);`,
237+
'base.ts': base,
238+
'overrides.ts': overrides,
239+
});
240+
const plan = planProject(root);
241+
expect(plan.changes).toEqual([]);
242+
expect(plan.findings).toContainEqual(
243+
expect.objectContaining({
244+
file: path.join(root, 'overrides.ts'),
245+
code: 'merged-config-defaults',
246+
}),
247+
);
248+
applyVitestV5Migration(plan);
249+
const findings = finishVitestV5Migration(plan);
250+
expect(fs.readFileSync(path.join(root, 'base.ts'), 'utf8')).toBe(base);
251+
expect(fs.readFileSync(path.join(root, 'overrides.ts'), 'utf8')).toBe(overrides);
252+
expect(findings.some(({ code }) => code === 'merged-config-defaults')).toBe(true);
253+
expect(planProject(root).changes).toEqual([]);
254+
});
255+
256+
it.each(['true', 'false'])(
257+
'preserves inherited locators.exact: %s without a child override',
258+
(exact) => {
259+
const result = config(`export default { test: {
260+
browser: { enabled: true, locators: { exact: ${exact} } },
261+
projects: [{ extends: true, test: { browser: { enabled: true } } }],
262+
} };`);
263+
expect(result.content.match(/locators:/g)).toHaveLength(1);
264+
expect(result.content).toContain(`locators: { exact: ${exact} }`);
265+
expect(result.findings).toEqual([]);
266+
expect(config(result.content).content).toBe(result.content);
267+
},
268+
);
269+
270+
it('adds locator defaults to an inheriting child when its parent has no browser options', () => {
271+
const result = config(`export default { test: {
272+
projects: [{ extends: true, test: { browser: { enabled: true } } }],
273+
} };`);
274+
expect(result.content).toContain('locators: { exact: false }');
275+
expect(result.findings).toEqual([]);
276+
});
277+
278+
it('adds the locator default only to the parent browser config', () => {
279+
const result = config(`export default { test: {
280+
browser: { enabled: true },
281+
projects: [{ extends: true, test: { browser: { enabled: true } } }],
282+
} };`);
283+
expect(result.content.match(/locators:/g)).toHaveLength(1);
284+
expect(result.content).toContain('locators: { exact: false }');
285+
});
286+
287+
it('does not override dynamic inherited browser settings', () => {
288+
const result = config(`export default { test: {
289+
browser: sharedBrowser,
290+
projects: [{ extends: true, test: { browser: { enabled: true } } }],
291+
} };`);
292+
expect(result.content).not.toContain('exact: false');
293+
expect(result.findings).toContainEqual(
294+
expect.objectContaining({ code: 'dynamic-project', severity: 'review' }),
295+
);
296+
});
297+
206298
it('does not guess dynamic projects or spread options', () => {
207299
const result = config(`export default { test: { ...shared, projects: [() => project] } };`);
208300
expect(result.content).not.toContain('clearMocks');
@@ -403,6 +495,28 @@ test('x', () => assert(() => {}).toThrow(''));`;
403495
expect(result.content).toContain('toThrow(/^$/)');
404496
});
405497

498+
it.each(['@vitest/expect', 'vite-plus/test/plugins/expect'])(
499+
'migrates assertions from %s in the same pass as their imports',
500+
(specifier) => {
501+
const result = source(`import { expect as check } from '${specifier}';
502+
import { test } from 'vitest';
503+
function helper(check) { check(() => {}).toThrow(''); }
504+
test('works', () => {
505+
check(() => { throw new Error('boom'); }).not.toThrow('');
506+
check(Promise.resolve(1)).resolves.toBe(1);
507+
check.element(el).toHaveTextContent('partial');
508+
});`);
509+
expect(result.content).toContain("import { expect as check } from 'vite-plus/test'");
510+
expect(result.content).toContain("function helper(check) { check(() => {}).toThrow(''); }");
511+
expect(result.content).toContain('.not.toThrow(/^$/)');
512+
expect(result.content).toContain("test('works', async () =>");
513+
expect(result.content).toContain('await check(Promise.resolve(1)).resolves.toBe(1)');
514+
expect(result.content).toContain("await check.element(el).toMatchTextContent('partial')");
515+
expect(result.findings).toEqual([]);
516+
expect(source(result.content).content).toBe(result.content);
517+
},
518+
);
519+
406520
it('only makes async-compatible callbacks async', () => {
407521
const result = source(`import { test, describe, expect } from 'vitest';
408522
import { render as mount } from 'vitest-browser-vue';

packages/cli/src/migration/migrator/vitest-v5.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { type SourceOptions, type VitestV5Finding } from '../vitest-v5/ast.ts';
1313
import { migrateVitestV5Command } from '../vitest-v5/commands.ts';
1414
import {
1515
findVitestV5ConfigFiles,
16+
findVitestV5MergedConfigFiles,
1617
migrateVitestV5Config,
1718
resolveVitestV5BrowserModes,
1819
} from '../vitest-v5/config.ts';
@@ -468,6 +469,7 @@ function scanAndRewriteFile(
468469
source: string,
469470
project: ProjectPlan,
470471
browserMode: boolean | undefined,
472+
mergedConfig: boolean,
471473
): { content: string; findings: VitestV5Finding[] } {
472474
const findings: VitestV5Finding[] = [];
473475
scanNode(file, source, findings);
@@ -504,7 +506,7 @@ function scanAndRewriteFile(
504506
findings.push(...review.findings.filter(({ code }) => pending.includes(code)));
505507
}
506508
if (project.configFiles.has(file)) {
507-
const configResult = migrateVitestV5Config(file, content, project.options);
509+
const configResult = migrateVitestV5Config(file, content, project.options, mergedConfig);
508510
content = configResult.content;
509511
findings.push(...configResult.findings);
510512
}
@@ -700,6 +702,7 @@ export function planVitestV5Migration(
700702
}
701703
}
702704
const allConfigs = findVitestV5ConfigFiles(allSources);
705+
const mergedConfigs = findVitestV5MergedConfigFiles(allSources, allConfigs);
703706
const browserPossible = [...allSources.values()].some((source) => BROWSER_SIGNAL.test(source));
704707
const browserCliOverride = [...allSources].some(
705708
([file, source]) =>
@@ -786,6 +789,7 @@ export function planVitestV5Migration(
786789
source,
787790
project,
788791
browserCliOverride ? undefined : browserModes.get(file),
792+
mergedConfigs.has(file),
789793
);
790794
findings.push(...result.findings);
791795
if (source !== result.content) {
@@ -910,6 +914,10 @@ export function finishVitestV5Migration(plan: VitestV5MigrationPlan): VitestV5Fi
910914
const findings: VitestV5Finding[] = [];
911915
const state = structuredClone(plan.state);
912916
const projectConfigs = currentProjectConfigs(plan);
917+
const configSources = new Map(
918+
[...projectConfigs.values()].flat().map((file) => [file, fs.readFileSync(file, 'utf8')]),
919+
);
920+
const mergedConfigs = findVitestV5MergedConfigFiles(configSources, new Set(configSources.keys()));
913921
for (const project of plan.projects) {
914922
if (!project.active || !project.sourceVersion) {
915923
continue;
@@ -918,7 +926,12 @@ export function finishVitestV5Migration(plan: VitestV5MigrationPlan): VitestV5Fi
918926
for (const file of configs) {
919927
const before = fs.readFileSync(file, 'utf8');
920928
try {
921-
const result = migrateVitestV5Config(file, before, project.options);
929+
const result = migrateVitestV5Config(
930+
file,
931+
before,
932+
project.options,
933+
mergedConfigs.has(file),
934+
);
922935
findings.push(...result.findings);
923936
if (
924937
!result.findings.some((finding) => finding.severity === 'block') &&

packages/cli/src/migration/vitest-v5/config.ts

Lines changed: 98 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,70 @@ export function findVitestV5ConfigFiles(sources: ReadonlyMap<string, string>): S
148148
return files;
149149
}
150150

151+
function hasConfigMerge(editor: SourceEditor): boolean {
152+
let found = false;
153+
traverse(editor.ast, {
154+
CallExpression(p) {
155+
if (importedName(p, p.node.callee, CONFIG_SOURCES) === 'mergeConfig') {
156+
found = true;
157+
}
158+
},
159+
});
160+
return found;
161+
}
162+
163+
/** Defaults on one merge fragment can override explicit settings in another.
164+
* Include local imported configs so the preflight and finalization agree. */
165+
export function findVitestV5MergedConfigFiles(
166+
sources: ReadonlyMap<string, string>,
167+
configFiles: ReadonlySet<string>,
168+
): Set<string> {
169+
const merged = new Set<string>();
170+
const imports = new Map<string, string[]>();
171+
const extensions = ['.ts', '.mts', '.cts', '.js', '.mjs', '.cjs', '.tsx', '.jsx'];
172+
for (const file of configFiles) {
173+
try {
174+
const editor = new SourceEditor(file, sources.get(file)!);
175+
if (hasConfigMerge(editor)) {
176+
merged.add(file);
177+
}
178+
const dependencies: string[] = [];
179+
for (const node of editor.ast.program.body) {
180+
if (
181+
node.type !== 'ImportDeclaration' &&
182+
node.type !== 'ExportNamedDeclaration' &&
183+
node.type !== 'ExportAllDeclaration'
184+
) {
185+
continue;
186+
}
187+
const reference = node.source?.value;
188+
if (!reference?.startsWith('.')) {
189+
continue;
190+
}
191+
const target = path.resolve(path.dirname(file), reference);
192+
const dependency = [
193+
target,
194+
target.replace(/\.([cm]?)js$/, '.$1ts'),
195+
...extensions.map((extension) => `${target}${extension}`),
196+
...extensions.map((extension) => path.join(target, `index${extension}`)),
197+
].find((candidate) => configFiles.has(candidate));
198+
if (dependency) {
199+
dependencies.push(dependency);
200+
}
201+
}
202+
imports.set(file, dependencies);
203+
} catch {
204+
// The normal config pass reports unsupported syntax.
205+
}
206+
}
207+
for (const file of merged) {
208+
for (const dependency of imports.get(file) ?? []) {
209+
merged.add(dependency);
210+
}
211+
}
212+
return merged;
213+
}
214+
151215
interface BrowserTestScope {
152216
root?: string;
153217
include?: string[];
@@ -373,9 +437,23 @@ export function resolveVitestV5BrowserModes(
373437
);
374438
}
375439

376-
export function migrateVitestV5Config(file: string, source: string, options: SourceOptions) {
440+
export function migrateVitestV5Config(
441+
file: string,
442+
source: string,
443+
options: SourceOptions,
444+
mergedConfig = false,
445+
) {
377446
const editor = new SourceEditor(file, source);
378447
const visited = new Set<t.ObjectExpression>();
448+
const merged = mergedConfig || hasConfigMerge(editor);
449+
const preserveDefaults = options.preserveV4 && !merged;
450+
if (merged && (options.preserveV4 || options.reviewV4)) {
451+
editor.report(
452+
undefined,
453+
'merged-config-defaults',
454+
'Review the effective merged config before adding v4 defaults for clearMocks, browser locators, fake timers, reporters, and projects. Defaults were not added to config fragments because they can override explicit settings in another fragment.',
455+
);
456+
}
379457

380458
function nested(
381459
object: t.ObjectExpression,
@@ -442,7 +520,7 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
442520
}
443521
}
444522
}
445-
if (!options.preserveV4 || !['json', 'junit'].includes(name.value)) {
523+
if (!preserveDefaults || !['json', 'junit'].includes(name.value)) {
446524
continue;
447525
}
448526
if (output && (!staticObject(output.value) || objectProperty(output.value, name.value))) {
@@ -475,14 +553,21 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
475553
}
476554
}
477555

478-
function testOptions(test: t.ObjectExpression, inherits: boolean) {
479-
if (options.preserveV4 && !inherits) {
556+
function testOptions(
557+
test: t.ObjectExpression,
558+
inherits: boolean,
559+
parentTest?: t.ObjectExpression,
560+
) {
561+
if (preserveDefaults && !inherits) {
480562
editor.add(test, 'clearMocks', 'false');
481563
}
482564
const browser = objectProperty(test, 'browser');
483565
if (browser && staticObject(browser.value)) {
484566
const value = browser.value;
485-
if (options.preserveV4) {
567+
// An inherited browser config already receives its defaults at the parent.
568+
// Do not replace its explicit or dynamic locator setting in the child.
569+
const inheritsBrowser = inherits && parentTest && objectProperty(parentTest, 'browser');
570+
if (preserveDefaults && !inheritsBrowser) {
486571
nested(value, 'locators', 'exact: false', (locators) =>
487572
editor.add(locators, 'exact', 'false'),
488573
);
@@ -526,7 +611,7 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
526611
);
527612
}
528613

529-
if (options.preserveV4 && options.temporalPolyfill) {
614+
if (preserveDefaults && options.temporalPolyfill) {
530615
nested(test, 'fakeTimers', "toNotFake: ['Temporal']", (timers) =>
531616
editor.add(timers, 'toNotFake', "['Temporal']"),
532617
);
@@ -545,7 +630,7 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
545630
}
546631
const thresholds = objectProperty(coverage.value, 'thresholds');
547632
if (
548-
options.preserveV4 &&
633+
preserveDefaults &&
549634
thresholds &&
550635
staticObject(thresholds.value) &&
551636
isTrue(objectProperty(thresholds.value, 'perFile')?.value)
@@ -614,7 +699,7 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
614699
return;
615700
}
616701
const hasInline = projects.value.elements.some((item) => item && item.type !== 'StringLiteral');
617-
if (hasInline && options.preserveV4) {
702+
if (hasInline && preserveDefaults) {
618703
editor.add(test, 'sharedViteServer', 'false');
619704
}
620705
for (const project of projects.value.elements) {
@@ -630,29 +715,29 @@ export function migrateVitestV5Config(file: string, source: string, options: Sou
630715
continue;
631716
}
632717
const extendsValue = objectProperty(project, 'extends');
633-
if (options.preserveV4) {
718+
if (preserveDefaults) {
634719
editor.add(project, 'extends', 'false');
635720
}
636-
config(project, isTrue(extendsValue?.value));
721+
config(project, isTrue(extendsValue?.value), test);
637722
}
638723
}
639724

640-
function config(object: t.ObjectExpression, inherits = false) {
725+
function config(object: t.ObjectExpression, inherits = false, parentTest?: t.ObjectExpression) {
641726
if (visited.has(object)) {
642727
return;
643728
}
644729
visited.add(object);
645730
const test = objectProperty(object, 'test');
646731
if (!test) {
647-
if (options.preserveV4 && !inherits) {
732+
if (preserveDefaults && !inherits) {
648733
editor.add(
649734
object,
650735
'test',
651736
`{ clearMocks: false${options.temporalPolyfill ? ", fakeTimers: { toNotFake: ['Temporal'] }" : ''} }`,
652737
);
653738
}
654739
} else if (staticObject(test.value)) {
655-
testOptions(test.value, inherits);
740+
testOptions(test.value, inherits, parentTest);
656741
} else {
657742
editor.report(
658743
test,

packages/cli/src/migration/vitest-v5/source.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,11 @@ function chain(
149149
export function migrateVitestV5Source(file: string, source: string, options: SourceOptions) {
150150
const editor = new SourceEditor(file, source);
151151
const asyncFunctions = new Set<t.Node>();
152-
const apiName = (p: NodePath, node: t.Node) => testApiName(p, node, options.globals);
152+
// Import edits are offset-based, so bindings still refer to the old module
153+
// while this traversal visits the assertions that must migrate with them.
154+
const apiName = (p: NodePath, node: t.Node) =>
155+
testApiName(p, node, options.globals) ??
156+
(importedName(p, node, EXPECT_SOURCES) === 'expect' ? 'expect' : undefined);
153157
const canAwait = (p: NodePath) => {
154158
const fn = p.getFunctionParent();
155159
if (!fn) {

0 commit comments

Comments
 (0)