Skip to content

Commit 8556db8

Browse files
clydinclydin
committed
fix(@schematics/angular): preserve Jasmine stub-by-default semantics for bare spies
The refactor-jasmine-vitest schematic previously migrated bare spyOn and spyOnProperty calls as a direct mechanical rename to vi.spyOn. Since the APIs feature opposing default behaviors (with jasmine.spyOn stubbing by default and vi.spyOn calling through), this caused migrated test suites to silently change behavior. This update structurally analyzes the AST during translation to detect chained strategies, appending .mockReturnValue(undefined) precisely for bare spies to retain original Jasmine semantics. Fixes #33253
1 parent 9eccdef commit 8556db8

5 files changed

Lines changed: 109 additions & 28 deletions

File tree

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

Lines changed: 12 additions & 8 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 () => {
@@ -144,7 +144,7 @@ describe('Jasmine to Vitest Schematic', () => {
144144
);
145145

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

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

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

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

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

182182
const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
183-
expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`);
183+
expect(changedNestedContent).toContain(
184+
`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`,
185+
);
184186

185187
const unchangedContent = tree.readContent('projects/bar/src/other/other.spec.ts');
186188
expect(unchangedContent).toContain(`spyOn(window, 'close');`);
@@ -194,10 +196,12 @@ describe('Jasmine to Vitest Schematic', () => {
194196
);
195197

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

199201
const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts');
200-
expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`);
202+
expect(changedNestedContent).toContain(
203+
`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`,
204+
);
201205
});
202206

203207
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: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,37 @@ import { addTodoComment } from '../utils/comment-helpers';
2424
import { RefactorContext } from '../utils/refactor-context';
2525
import { createViCallExpression } from '../utils/refactor-helpers';
2626

27+
function isChainedWithAnd(node: ts.Node): boolean {
28+
let parent = node.parent;
29+
while (parent) {
30+
if (ts.isPropertyAccessExpression(parent)) {
31+
if (ts.isIdentifier(parent.name) && parent.name.text === 'and') {
32+
return true;
33+
}
34+
} else if (ts.isElementAccessExpression(parent)) {
35+
if (
36+
ts.isStringLiteral(parent.argumentExpression) &&
37+
parent.argumentExpression.text === 'and'
38+
) {
39+
return true;
40+
}
41+
} else if (
42+
ts.isParenthesizedExpression(parent) ||
43+
ts.isAsExpression(parent) ||
44+
ts.isNonNullExpression(parent) ||
45+
ts.isTypeAssertionExpression(parent) ||
46+
ts.isSatisfiesExpression(parent)
47+
) {
48+
parent = parent.parent;
49+
continue;
50+
}
51+
break;
52+
}
53+
54+
return false;
55+
}
56+
57+
/* eslint-disable max-lines-per-function */
2758
export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.Node {
2859
const { sourceFile, reporter, pendingVitestValueImports } = refactorCtx;
2960
if (!ts.isCallExpression(node)) {
@@ -35,29 +66,53 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
3566
(node.expression.text === 'spyOn' || node.expression.text === 'spyOnProperty')
3667
) {
3768
addVitestValueImport(pendingVitestValueImports, 'vi');
38-
reporter.reportTransformation(
39-
sourceFile,
40-
node,
41-
`Transformed \`${node.expression.text}\` to \`vi.spyOn\`.`,
42-
);
4369

44-
return ts.factory.updateCallExpression(
70+
const viSpyOnCall = ts.factory.updateCallExpression(
4571
node,
4672
createPropertyAccess('vi', 'spyOn'),
4773
node.typeArguments,
4874
node.arguments,
4975
);
76+
77+
if (isChainedWithAnd(node)) {
78+
reporter.reportTransformation(
79+
sourceFile,
80+
node,
81+
`Transformed \`${node.expression.text}\` to \`vi.spyOn\`.`,
82+
);
83+
84+
return viSpyOnCall;
85+
}
86+
87+
reporter.reportTransformation(
88+
sourceFile,
89+
node,
90+
`Transformed \`${node.expression.text}\` to \`vi.spyOn\`, ` +
91+
`appending \`.mockReturnValue(undefined)\` to preserve stub-by-default semantics.`,
92+
);
93+
94+
return ts.factory.createCallExpression(
95+
createPropertyAccess(viSpyOnCall, 'mockReturnValue'),
96+
undefined,
97+
[ts.factory.createIdentifier('undefined')],
98+
);
5099
}
51100

52101
if (ts.isPropertyAccessExpression(node.expression)) {
53102
const pae = node.expression;
54103

55-
if (
56-
ts.isPropertyAccessExpression(pae.expression) &&
57-
ts.isIdentifier(pae.expression.name) &&
58-
pae.expression.name.text === 'and'
59-
) {
60-
const spyCall = pae.expression.expression;
104+
const isChainedWithAndProperty =
105+
(ts.isPropertyAccessExpression(pae.expression) &&
106+
ts.isIdentifier(pae.expression.name) &&
107+
pae.expression.name.text === 'and') ||
108+
(ts.isElementAccessExpression(pae.expression) &&
109+
ts.isStringLiteral(pae.expression.argumentExpression) &&
110+
pae.expression.argumentExpression.text === 'and');
111+
112+
if (isChainedWithAndProperty) {
113+
const spyCall = ts.isPropertyAccessExpression(pae.expression)
114+
? pae.expression.expression
115+
: (pae.expression as ts.ElementAccessExpression).expression;
61116
let newMethodName: string | undefined;
62117
let args = node.arguments;
63118

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

Lines changed: 26 additions & 4 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 spyOn(object, "method") to vi.spyOn(object, "method").mockReturnValue(undefined)',
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 spyOnProperty(object, "prop") to vi.spyOn(object, "prop").mockReturnValue(undefined)',
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(() => {})',
@@ -117,6 +119,26 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => {
117119
expected: `// TODO: vitest-migration: Unsupported spy strategy ".and.unknownStrategy()" found. Please migrate this manually. See: https://vitest.dev/api/mocked.html#mock
118120
vi.spyOn(service, 'myMethod').and.unknownStrategy();`,
119121
},
122+
{
123+
description: 'should correctly identify chained spies with element access (bracket notation)',
124+
input: `spyOn(service, 'myMethod')['and'].returnValue(42);`,
125+
expected: `vi.spyOn(service, 'myMethod').mockReturnValue(42);`,
126+
},
127+
{
128+
description: 'should correctly identify chained spies with non-null assertion',
129+
input: `(spyOn(service, 'myMethod')!).and.returnValue(42);`,
130+
expected: `(vi.spyOn(service, 'myMethod')!).mockReturnValue(42);`,
131+
},
132+
{
133+
description: 'should correctly identify chained spies with type assertion',
134+
input: `(<any>spyOn(service, 'myMethod')).and.returnValue(42);`,
135+
expected: `(<any>vi.spyOn(service, 'myMethod')).mockReturnValue(42);`,
136+
},
137+
{
138+
description: 'should correctly identify chained spies with satisfies expression',
139+
input: `(spyOn(service, 'myMethod') satisfies any).and.returnValue(42);`,
140+
expected: `(vi.spyOn(service, 'myMethod') satisfies any).mockReturnValue(42);`,
141+
},
120142
];
121143

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

0 commit comments

Comments
 (0)