Skip to content

Commit 673dbaf

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 d4cc332 commit 673dbaf

5 files changed

Lines changed: 123 additions & 26 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: 71 additions & 10 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.isStringLiteralLike(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,58 @@ 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

104+
let spyCall: ts.Expression | undefined;
105+
55106
if (
56107
ts.isPropertyAccessExpression(pae.expression) &&
57108
ts.isIdentifier(pae.expression.name) &&
58109
pae.expression.name.text === 'and'
59110
) {
60-
const spyCall = pae.expression.expression;
111+
spyCall = pae.expression.expression;
112+
} else if (
113+
ts.isElementAccessExpression(pae.expression) &&
114+
ts.isStringLiteralLike(pae.expression.argumentExpression) &&
115+
pae.expression.argumentExpression.text === 'and'
116+
) {
117+
spyCall = pae.expression.expression;
118+
}
119+
120+
if (spyCall) {
61121
let newMethodName: string | undefined;
62122
let args = node.arguments;
63123

@@ -66,7 +126,8 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
66126
switch (strategyName) {
67127
case 'returnValue':
68128
{
69-
const result = getPromiseResolveRejectMethod(args[0]);
129+
const firstArg = args[0];
130+
const result = firstArg ? getPromiseResolveRejectMethod(firstArg) : null;
70131
if (result) {
71132
const methodMapping = {
72133
'resolve': 'mockResolvedValue',
@@ -146,12 +207,12 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts.
146207
);
147208
const errorArg = node.arguments[0];
148209
const throwStatement = ts.factory.createThrowStatement(
149-
ts.isNewExpression(errorArg)
210+
errorArg && ts.isNewExpression(errorArg)
150211
? errorArg
151212
: ts.factory.createNewExpression(
152213
ts.factory.createIdentifier('Error'),
153214
undefined,
154-
node.arguments,
215+
errorArg ? [errorArg] : [],
155216
),
156217
);
157218
const arrowFunction = ts.factory.createArrowFunction(

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

Lines changed: 36 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,36 @@ 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+
},
142+
{
143+
description: 'should handle and.returnValue() without arguments defensively',
144+
input: `spyOn(service, 'myMethod').and.returnValue();`,
145+
expected: `vi.spyOn(service, 'myMethod').mockReturnValue();`,
146+
},
147+
{
148+
description: 'should handle and.throwError() without arguments defensively',
149+
input: `spyOn(service, 'myMethod').and.throwError();`,
150+
expected: `vi.spyOn(service, 'myMethod').mockImplementation(() => { throw new Error() });`,
151+
},
120152
];
121153

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

0 commit comments

Comments
 (0)