Skip to content

Commit 0ae507d

Browse files
authored
fix(@schematics/angular): add a TODO for spy strategies configured with withArgs in refactor-jasmine-vitest
Spy strategies configured for specific arguments, such as `spyOn(service, 'calc').withArgs(1).and.returnValue(2)`, were converted to `vi.spyOn(service, 'calc').mockReturnValue(undefined).withArgs(1).mockReturnValue(2)` without a TODO comment. Vitest mocks have no `withArgs` method, so the migrated test failed with a TypeError.
1 parent 3436f92 commit 0ae507d

3 files changed

Lines changed: 100 additions & 8 deletions

File tree

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

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,20 +72,36 @@ function getUnwrappedParent(node: ts.Node): ts.Node | undefined {
7272
}
7373

7474
function isChainedWithAnd(node: ts.Node): boolean {
75+
let previous: ts.Node = node;
7576
let parent = node.parent;
7677
while (parent) {
77-
if (ts.isPropertyAccessExpression(parent)) {
78-
if (ts.isIdentifier(parent.name) && parent.name.text === 'and') {
78+
if (ts.isPropertyAccessExpression(parent) && ts.isIdentifier(parent.name)) {
79+
if (parent.name.text === 'and') {
7980
return true;
8081
}
81-
} else if (ts.isElementAccessExpression(parent)) {
82-
if (
83-
ts.isStringLiteralLike(parent.argumentExpression) &&
84-
parent.argumentExpression.text === 'and'
85-
) {
82+
// `withArgs(...)` returns the spy, so a strategy may still follow.
83+
if (parent.name.text === 'withArgs') {
84+
previous = parent;
85+
parent = parent.parent;
86+
continue;
87+
}
88+
} else if (
89+
ts.isElementAccessExpression(parent) &&
90+
ts.isStringLiteralLike(parent.argumentExpression)
91+
) {
92+
if (parent.argumentExpression.text === 'and') {
8693
return true;
8794
}
88-
} else if (isTransparentWrapper(parent)) {
95+
if (parent.argumentExpression.text === 'withArgs') {
96+
previous = parent;
97+
parent = parent.parent;
98+
continue;
99+
}
100+
} else if (
101+
isTransparentWrapper(parent) ||
102+
(ts.isCallExpression(parent) && parent.expression === previous)
103+
) {
104+
previous = parent;
89105
parent = parent.parent;
90106
continue;
91107
}
@@ -95,6 +111,33 @@ function isChainedWithAnd(node: ts.Node): boolean {
95111
return false;
96112
}
97113

114+
/**
115+
* Checks whether a spy expression chain contains a `withArgs(...)` link.
116+
*/
117+
function chainContainsWithArgs(expression: ts.Expression): boolean {
118+
let current: ts.Expression = unwrapExpression(expression);
119+
while (true) {
120+
if (ts.isCallExpression(current)) {
121+
current = unwrapExpression(current.expression);
122+
} else if (ts.isPropertyAccessExpression(current)) {
123+
if (ts.isIdentifier(current.name) && current.name.text === 'withArgs') {
124+
return true;
125+
}
126+
current = unwrapExpression(current.expression);
127+
} else if (ts.isElementAccessExpression(current)) {
128+
if (
129+
ts.isStringLiteralLike(current.argumentExpression) &&
130+
current.argumentExpression.text === 'withArgs'
131+
) {
132+
return true;
133+
}
134+
current = unwrapExpression(current.expression);
135+
} else {
136+
return false;
137+
}
138+
}
139+
}
140+
98141
function isSpyCreation(node: ts.Expression): boolean {
99142
const expression = unwrapExpression(node);
100143
if (!ts.isCallExpression(expression)) {
@@ -181,6 +224,19 @@ function transformSpyStrategy(node: ts.CallExpression, refactorCtx: RefactorCont
181224
}
182225

183226
if (spyCall) {
227+
// Vitest mocks have no `withArgs`, so a strategy for specific arguments cannot be converted.
228+
// The whole statement is left untouched, with a single TODO on the outermost strategy call;
229+
// inner `.and` chains reached while the visitor descends are covered by that TODO.
230+
if (chainContainsWithArgs(spyCall)) {
231+
if (!isChainedWithAnd(node)) {
232+
const category = 'withArgs';
233+
reporter.recordTodo(category, sourceFile, node);
234+
addTodoComment(node, category);
235+
}
236+
237+
return node;
238+
}
239+
184240
let newMethodName: string | undefined;
185241
let args = node.arguments;
186242

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,36 @@ getSpy().and.callThrough();`,
145145
input: `spyOn(service, 'myMethod').and.unknownStrategy();`,
146146
expected: `// TODO: vitest-migration: Unsupported spy strategy ".and.unknownStrategy()" found. Please migrate this manually. See: https://vitest.dev/api/mocked.html#mock
147147
vi.spyOn(service, 'myMethod').and.unknownStrategy();`,
148+
},
149+
{
150+
description: 'should add a TODO for a spy strategy configured with .withArgs()',
151+
input: `spyOn(service, 'myMethod').withArgs(1).and.returnValue(2);`,
152+
expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for spy.withArgs(). Please migrate this manually, for example with mockImplementation() that checks the arguments. See: https://vitest.dev/api/mock.html#mockimplementation
153+
vi.spyOn(service, 'myMethod').withArgs(1).and.returnValue(2);`,
154+
},
155+
{
156+
description: 'should add a single TODO for multiple chained .withArgs() strategies',
157+
input: `mySpy.withArgs(1).and.returnValue(10).withArgs(2).and.returnValue(20);`,
158+
expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for spy.withArgs(). Please migrate this manually, for example with mockImplementation() that checks the arguments. See: https://vitest.dev/api/mock.html#mockimplementation
159+
mySpy.withArgs(1).and.returnValue(10).withArgs(2).and.returnValue(20);`,
160+
},
161+
{
162+
description: 'should not transform a default strategy chained after .withArgs()',
163+
input: `mySpy.withArgs(1).and.returnValue(2).and.returnValue(0);`,
164+
expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for spy.withArgs(). Please migrate this manually, for example with mockImplementation() that checks the arguments. See: https://vitest.dev/api/mock.html#mockimplementation
165+
mySpy.withArgs(1).and.returnValue(2).and.returnValue(0);`,
166+
},
167+
{
168+
description: 'should add a TODO for .withArgs() on an existing spy',
169+
input: `mySpy.withArgs(1).and.returnValue(2);`,
170+
expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for spy.withArgs(). Please migrate this manually, for example with mockImplementation() that checks the arguments. See: https://vitest.dev/api/mock.html#mockimplementation
171+
mySpy.withArgs(1).and.returnValue(2);`,
172+
},
173+
{
174+
description: 'should add a TODO for a wrapped .withArgs() call',
175+
input: `mySpy.withArgs(1)!.and.returnValue(2);`,
176+
expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for spy.withArgs(). Please migrate this manually, for example with mockImplementation() that checks the arguments. See: https://vitest.dev/api/mock.html#mockimplementation
177+
mySpy.withArgs(1)!.and.returnValue(2);`,
148178
},
149179
{
150180
description: 'should correctly identify chained spies with element access (bracket notation)',

‎packages/schematics/angular/refactor/jasmine-vitest/utils/todo-notes.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ export const TODO_NOTES = {
165165
' the original implementation but also clears the recorded calls.',
166166
url: 'https://vitest.dev/api/mock.html#mockreset',
167167
},
168+
'withArgs': {
169+
message:
170+
'Vitest does not have a direct equivalent for spy.withArgs().' +
171+
' Please migrate this manually, for example with mockImplementation() that checks the arguments.',
172+
url: 'https://vitest.dev/api/mock.html#mockimplementation',
173+
},
168174
'mostRecent-without-args': {
169175
message:
170176
'Direct usage of mostRecent() is not supported.' +

0 commit comments

Comments
 (0)