Skip to content

Commit 3164fc9

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 3164fc9

5 files changed

Lines changed: 163 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: 92 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 transformSpyExpression(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 transformSpyExpression(spyCall, 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 transformSpyExpression(spyCall, refactorCtx, false);
123112
case 'stub': {
124113
reporter.reportTransformation(
125114
sourceFile,
@@ -219,6 +208,67 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
219208
return node;
220209
}
221210

211+
function transformSpyExpression(
212+
node: ts.Expression,
213+
refactorCtx: RefactorContext,
214+
forceStubBehavior?: boolean,
215+
): ts.Expression {
216+
const { sourceFile, reporter, pendingVitestValueImports } = refactorCtx;
217+
218+
if (
219+
ts.isCallExpression(node) &&
220+
ts.isIdentifier(node.expression) &&
221+
(node.expression.text === 'spyOn' || node.expression.text === 'spyOnProperty')
222+
) {
223+
const spyFnName = node.expression.text;
224+
addVitestValueImport(pendingVitestValueImports, 'vi');
225+
226+
const viSpyOnCall = ts.factory.updateCallExpression(
227+
node,
228+
createPropertyAccess('vi', 'spyOn'),
229+
node.typeArguments,
230+
node.arguments,
231+
);
232+
233+
const shouldStub = forceStubBehavior ?? shouldStubBareSpyOn(node);
234+
235+
if (shouldStub) {
236+
reporter.reportTransformation(
237+
sourceFile,
238+
node,
239+
`Transformed \`${spyFnName}\` to \`vi.spyOn(...).mockReturnValue(undefined)\` ` +
240+
`to preserve Jasmine's stub-by-default behavior.`,
241+
);
242+
243+
return ts.factory.createCallExpression(
244+
createPropertyAccess(viSpyOnCall, 'mockReturnValue'),
245+
undefined,
246+
[ts.factory.createIdentifier('undefined')],
247+
);
248+
}
249+
250+
reporter.reportTransformation(
251+
sourceFile,
252+
node,
253+
`Transformed \`${spyFnName}\` to \`vi.spyOn\`.`,
254+
);
255+
256+
return viSpyOnCall;
257+
}
258+
259+
// Variable reference (e.g. `spy` from `spy.and.returnValues()`).
260+
// There is no spyOn call to transform — only stub-wrapping is relevant here.
261+
if (forceStubBehavior) {
262+
return ts.factory.createCallExpression(
263+
createPropertyAccess(node, 'mockReturnValue'),
264+
undefined,
265+
[ts.factory.createIdentifier('undefined')],
266+
);
267+
}
268+
269+
return node;
270+
}
271+
222272
export function transformCreateSpy(node: ts.Node, ctx: RefactorContext): ts.Node {
223273
const { reporter, sourceFile, pendingVitestValueImports } = ctx;
224274
if (!isJasmineCallExpression(node, 'createSpy')) {
@@ -395,6 +445,32 @@ export function transformSpyReset(
395445
return node;
396446
}
397447

448+
function shouldStubBareSpyOn(node: ts.CallExpression): boolean {
449+
if (
450+
ts.isIdentifier(node.expression) &&
451+
node.expression.text === 'spyOnProperty' &&
452+
node.arguments.length >= 3
453+
) {
454+
const accessType = node.arguments[2];
455+
if (ts.isStringLiteralLike(accessType) && accessType.text === 'set') {
456+
return false;
457+
}
458+
}
459+
460+
const parent = node.parent;
461+
if (
462+
parent &&
463+
ts.isPropertyAccessExpression(parent) &&
464+
parent.expression === node &&
465+
ts.isIdentifier(parent.name) &&
466+
parent.name.text === 'and'
467+
) {
468+
return false;
469+
}
470+
471+
return true;
472+
}
473+
398474
function getSpyIdentifierFromCalls(node: ts.PropertyAccessExpression): ts.Expression | undefined {
399475
if (ts.isIdentifier(node.name) && node.name.text === 'calls') {
400476
return node.expression;

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

Lines changed: 50 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,46 @@ 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+
},
147+
{
148+
description:
149+
'should wrap a spy variable with .and.returnValues() with no args to preserve stub-by-default semantics',
150+
input: `spy.and.returnValues();`,
151+
expected: `spy.mockReturnValue(undefined);`,
152+
},
153+
{
154+
description: 'should remove .and.callThrough() on a spy variable',
155+
input: `spy.and.callThrough();`,
156+
expected: `spy;`,
157+
},
158+
{
159+
description:
160+
'should preserve stub-by-default on spyOn with .calls.reset() chained (not an .and accessor)',
161+
input: `spyOn(service, 'myMethod').calls.reset();`,
162+
expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined).mockClear();`,
163+
},
120164
];
121165

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

0 commit comments

Comments
 (0)