Skip to content

Commit 06a7cc8

Browse files
spliffonespliffone
committed
fix(@schematics/angular): preserve Jasmine stub-by-default behavior for bare spyOn calls
Bare `spyOn`/`spyOnProperty` calls in Jasmine create stubs that return `undefined` by default. Vitest's `vi.spyOn` passes through to the real implementation instead, changing observable behavior of migrated tests. Wrap bare spy calls with `.mockReturnValue(undefined)` to preserve the Jasmine semantics, except for `spyOnProperty(obj, prop, 'set')` where setter semantics already match.
1 parent 8ae13a4 commit 06a7cc8

5 files changed

Lines changed: 118 additions & 35 deletions

File tree

‎packages/schematics/angular/refactor/jasmine-vitest/index_spec.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('Jasmine to Vitest Schematic', () => {
5959
);
6060

6161
const newContent = tree.readContent(specFilePath);
62-
expect(newContent).toContain(`vi.spyOn(service, 'myMethod');`);
62+
expect(newContent).toContain(`vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`);
6363
});
6464

6565
it('should only transform files matching the fileSuffix option', async () => {
@@ -94,7 +94,7 @@ describe('Jasmine to Vitest Schematic', () => {
9494
expect(unchangedContent).not.toContain(`vi.spyOn(window, 'alert');`);
9595

9696
const changedContent = tree.readContent(testFilePath);
97-
expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`);
97+
expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`);
9898
});
9999

100100
it('should print verbose logs when the verbose option is true', async () => {
@@ -120,7 +120,11 @@ describe('Jasmine to Vitest Schematic', () => {
120120

121121
expect(logs).toContain('Detailed Transformation Log:');
122122
expect(logs).toContain(`Processing: /${specFilePath}`);
123-
expect(logs.some((log) => log.includes('Transformed `spyOn` to `vi.spyOn`'))).toBe(true);
123+
expect(
124+
logs.some((log) =>
125+
log.includes('Transformed `spyOn` to `vi.spyOn(...).mockReturnValue(undefined)`'),
126+
),
127+
).toBe(true);
124128
});
125129

126130
describe('with `include` option', () => {
@@ -144,7 +148,7 @@ describe('Jasmine to Vitest Schematic', () => {
144148
);
145149

146150
const changedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
147-
expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`);
151+
expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`);
148152

149153
const unchangedContent = tree.readContent('projects/bar/src/app/app.spec.ts');
150154
expect(unchangedContent).toContain(`spyOn(window, 'alert');`);
@@ -158,7 +162,7 @@ describe('Jasmine to Vitest Schematic', () => {
158162
);
159163

160164
const changedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
161-
expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`);
165+
expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`);
162166

163167
const unchangedContent = tree.readContent('projects/bar/src/app/app.spec.ts');
164168
expect(unchangedContent).toContain(`spyOn(window, 'alert');`);
@@ -177,10 +181,12 @@ describe('Jasmine to Vitest Schematic', () => {
177181
);
178182

179183
const changedAppContent = tree.readContent('projects/bar/src/app/app.spec.ts');
180-
expect(changedAppContent).toContain(`vi.spyOn(window, 'alert');`);
184+
expect(changedAppContent).toContain(`vi.spyOn(window, 'alert').mockReturnValue(undefined);`);
181185

182186
const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
183-
expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`);
187+
expect(changedNestedContent).toContain(
188+
`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`,
189+
);
184190

185191
const unchangedContent = tree.readContent('projects/bar/src/other/other.spec.ts');
186192
expect(unchangedContent).toContain(`spyOn(window, 'close');`);
@@ -194,10 +200,12 @@ describe('Jasmine to Vitest Schematic', () => {
194200
);
195201

196202
const changedAppContent = tree.readContent('projects/bar/src/app/app.spec.ts');
197-
expect(changedAppContent).toContain(`vi.spyOn(window, 'alert');`);
203+
expect(changedAppContent).toContain(`vi.spyOn(window, 'alert').mockReturnValue(undefined);`);
198204

199205
const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
200-
expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`);
206+
expect(changedNestedContent).toContain(
207+
`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`,
208+
);
201209
});
202210

203211
it('should throw if the include path does not exist', async () => {

‎packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe('Jasmine to Vitest Transformer - Integration Tests', () => {
109109
});
110110
111111
it('should handle user click', () => {
112-
vi.spyOn(window, 'alert');
112+
vi.spyOn(window, 'alert').mockReturnValue(undefined);
113113
const button = fixture.nativeElement.querySelector('button');
114114
button.click();
115115
fixture.detectChanges();

‎packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => {
1313
const input = `spyOn(foo, 'bar');`;
1414
const expected = `
1515
import { vi } from 'vitest';
16-
vi.spyOn(foo, 'bar');
16+
vi.spyOn(foo, 'bar').mockReturnValue(undefined);
1717
`;
1818
await expectTransformation(input, expected, true);
1919
});
@@ -27,7 +27,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => {
2727
import { type Mock, vi } from 'vitest';
2828
2929
let mySpy: Mock;
30-
vi.spyOn(foo, 'bar');
30+
vi.spyOn(foo, 'bar').mockReturnValue(undefined);
3131
`;
3232
await expectTransformation(input, expected, true);
3333
});
@@ -41,7 +41,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => {
4141
import type { Mock } from 'vitest';
4242
4343
let mySpy: Mock;
44-
vi.spyOn(foo, 'bar');
44+
vi.spyOn(foo, 'bar').mockReturnValue(undefined);
4545
`;
4646
await expectTransformation(input, expected, false);
4747
});

‎packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts‎

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,7 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
3434
ts.isIdentifier(node.expression) &&
3535
(node.expression.text === 'spyOn' || node.expression.text === 'spyOnProperty')
3636
) {
37-
addVitestValueImport(pendingVitestValueImports, 'vi');
38-
reporter.reportTransformation(
39-
sourceFile,
40-
node,
41-
`Transformed \`${node.expression.text}\` to \`vi.spyOn\`.`,
42-
);
43-
44-
return ts.factory.updateCallExpression(
45-
node,
46-
createPropertyAccess('vi', 'spyOn'),
47-
node.typeArguments,
48-
node.arguments,
49-
);
37+
return transformBareSpyOn(node, refactorCtx);
5038
}
5139

5240
if (ts.isPropertyAccessExpression(node.expression)) {
@@ -93,8 +81,8 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
9381
);
9482
const returnValues = node.arguments;
9583
if (returnValues.length === 0) {
96-
// No values, so it's a no-op. Just transform the spyOn call.
97-
return transformSpies(spyCall, refactorCtx);
84+
// No values, preserve Jasmine's stub-by-default behavior.
85+
return transformBareSpyOn(spyCall as ts.CallExpression, refactorCtx, true);
9886
}
9987
// spy.and.returnValues(a, b) -> spy.mockReturnValueOnce(a).mockReturnValueOnce(b)
10088
let chainedCall: ts.Expression = spyCall;
@@ -119,7 +107,8 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
119107
'Removed redundant `.and.callThrough()` call.',
120108
);
121109

122-
return transformSpies(spyCall, refactorCtx); // .and.callThrough() is redundant, just transform spyOn.
110+
// .and.callThrough() is redundant, just transform spyOn.
111+
return transformBareSpyOn(spyCall as ts.CallExpression, refactorCtx, false);
123112
case 'stub': {
124113
reporter.reportTransformation(
125114
sourceFile,
@@ -219,6 +208,45 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
219208
return node;
220209
}
221210

211+
function transformBareSpyOn(
212+
node: ts.CallExpression,
213+
refactorCtx: RefactorContext,
214+
forceStubBehavior?: boolean,
215+
): ts.Node {
216+
const { sourceFile, reporter, pendingVitestValueImports } = refactorCtx;
217+
// node.expression is always a spyOn/spyOnProperty identifier at call sites.
218+
const spyFnName = (node.expression as ts.Identifier).text;
219+
addVitestValueImport(pendingVitestValueImports, 'vi');
220+
221+
const viSpyOnCall = ts.factory.updateCallExpression(
222+
node,
223+
createPropertyAccess('vi', 'spyOn'),
224+
node.typeArguments,
225+
node.arguments,
226+
);
227+
228+
const shouldStub = forceStubBehavior ?? shouldStubBareSpyOn(node);
229+
230+
if (shouldStub) {
231+
reporter.reportTransformation(
232+
sourceFile,
233+
node,
234+
`Transformed \`${spyFnName}\` to \`vi.spyOn(...).mockReturnValue(undefined)\` ` +
235+
`to preserve Jasmine's stub-by-default behavior.`,
236+
);
237+
238+
return ts.factory.createCallExpression(
239+
createPropertyAccess(viSpyOnCall, 'mockReturnValue'),
240+
undefined,
241+
[ts.factory.createIdentifier('undefined')],
242+
);
243+
}
244+
245+
reporter.reportTransformation(sourceFile, node, `Transformed \`${spyFnName}\` to \`vi.spyOn\`.`);
246+
247+
return viSpyOnCall;
248+
}
249+
222250
export function transformCreateSpy(node: ts.Node, ctx: RefactorContext): ts.Node {
223251
const { reporter, sourceFile, pendingVitestValueImports } = ctx;
224252
if (!isJasmineCallExpression(node, 'createSpy')) {
@@ -395,6 +423,26 @@ export function transformSpyReset(
395423
return node;
396424
}
397425

426+
function shouldStubBareSpyOn(node: ts.CallExpression): boolean {
427+
if (
428+
ts.isIdentifier(node.expression) &&
429+
node.expression.text === 'spyOnProperty' &&
430+
node.arguments.length >= 3
431+
) {
432+
const accessType = node.arguments[2];
433+
if (ts.isStringLiteralLike(accessType) && accessType.text === 'set') {
434+
return false;
435+
}
436+
}
437+
438+
const parent = node.parent;
439+
if (parent && ts.isPropertyAccessExpression(parent) && parent.expression === node) {
440+
return false;
441+
}
442+
443+
return true;
444+
}
445+
398446
function getSpyIdentifierFromCalls(node: ts.PropertyAccessExpression): ts.Expression | undefined {
399447
if (ts.isIdentifier(node.name) && node.name.text === 'calls') {
400448
return node.expression;

‎packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ import { expectTransformation } from '../test-helpers';
1111
describe('Jasmine to Vitest Transformer - transformSpies', () => {
1212
const testCases = [
1313
{
14-
description: 'should transform spyOn(object, "method") to vi.spyOn(object, "method")',
14+
description:
15+
'should transform bare spyOn to vi.spyOn(...).mockReturnValue(undefined) to preserve Jasmine stub-by-default semantics',
1516
input: `spyOn(service, 'myMethod');`,
16-
expected: `vi.spyOn(service, 'myMethod');`,
17+
expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`,
1718
},
1819
{
1920
description: 'should transform .and.returnValue(...) to .mockReturnValue(...)',
@@ -58,9 +59,10 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => {
5859
expected: `const mySpy = vi.fn(() => 'foo').mockName('mySpy');`,
5960
},
6061
{
61-
description: 'should transform spyOnProperty(object, "prop") to vi.spyOn(object, "prop")',
62+
description:
63+
'should transform bare spyOnProperty to vi.spyOn(...).mockReturnValue(undefined) to preserve Jasmine stub-by-default semantics',
6264
input: `spyOnProperty(service, 'myProp');`,
63-
expected: `vi.spyOn(service, 'myProp');`,
65+
expected: `vi.spyOn(service, 'myProp').mockReturnValue(undefined);`,
6466
},
6567
{
6668
description: 'should transform .and.stub() to .mockImplementation(() => {})',
@@ -80,9 +82,11 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => {
8082
expected: `const mySpy = vi.fn().mockName('mySpy').mockReturnValue(true);`,
8183
},
8284
{
83-
description: 'should handle .and.returnValues() with no arguments',
85+
description:
86+
'should transform .and.returnValues() with no args to vi.spyOn(...).mockReturnValue(undefined) ' +
87+
'because Jasmine reverts to stub-by-default',
8488
input: `spyOn(service, 'myMethod').and.returnValues();`,
85-
expected: `vi.spyOn(service, 'myMethod');`,
89+
expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`,
8690
},
8791
{
8892
description:
@@ -117,6 +121,29 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => {
117121
expected: `// TODO: vitest-migration: Unsupported spy strategy ".and.unknownStrategy()" found. Please migrate this manually. See: https://vitest.dev/api/mocked.html#mock
118122
vi.spyOn(service, 'myMethod').and.unknownStrategy();`,
119123
},
124+
{
125+
description: 'should preserve stub-by-default semantics for spyOn assigned to a variable',
126+
input: `const spy = spyOn(service, 'myMethod');`,
127+
expected: `const spy = vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`,
128+
},
129+
{
130+
description:
131+
'should wrap spyOnProperty with explicit "get" access type to preserve stub-by-default semantics',
132+
input: `spyOnProperty(service, 'myProp', 'get');`,
133+
expected: `vi.spyOn(service, 'myProp', 'get').mockReturnValue(undefined);`,
134+
},
135+
{
136+
description:
137+
'should NOT wrap spyOnProperty with "set" access type because setter semantics already match',
138+
input: `spyOnProperty(service, 'myProp', 'set');`,
139+
expected: `vi.spyOn(service, 'myProp', 'set');`,
140+
},
141+
{
142+
description:
143+
'should wrap spyOn used as an expression argument to preserve stub-by-default semantics',
144+
input: `expect(spyOn(service, 'myMethod')).toBeDefined();`,
145+
expected: `expect(vi.spyOn(service, 'myMethod').mockReturnValue(undefined)).toBeDefined();`,
146+
},
120147
];
121148

122149
testCases.forEach(({ description, input, expected }) => {

0 commit comments

Comments
 (0)