Repository navigation
fix(@schematics/angular): add a TODO for spy strategies configured with withArgs in refactor-jasmine-vitest - #34620
Conversation
…th 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. Add a TODO comment for these strategies instead, since they need to be migrated manually, for example with `mockImplementation()`.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for handling Jasmine's .withArgs() spy strategy during the Jasmine-to-Vitest migration. Since Vitest does not have a direct equivalent, the transformer now identifies .withArgs() calls and inserts a TODO comment suggesting manual migration (e.g., using mockImplementation()). Corresponding unit tests and TODO note definitions have also been added. The feedback suggests using unwrapExpression(spyCall) to ensure that any transparent wrappers, such as parentheses or non-null assertions, are stripped before performing AST checks, preventing the withArgs check from being bypassed.
…ured with withArgs in refactor-jasmine-vitest
alan-agius4
left a comment
alan-agius4
left a comment
There was a problem hiding this comment.
Thanks for the PR. Please see the inline comments regarding .mockReturnValue(undefined) injection on spyOn(...).withArgs(...) and chained .withArgs(...) calls.
…ured with withArgs in refactor-jasmine-vitest
alan-agius4
added
action: merge
Spy strategies configured for specific arguments, such as
spyOn(service, 'calc').withArgs(1).and.returnValue(2), were converted tovi.spyOn(service, 'calc').mockReturnValue(undefined).withArgs(1).mockReturnValue(2)without a TODO comment. Vitest mocks have nowithArgsmethod, so the migrated test failed with a TypeError.Add a TODO comment for these strategies instead, since they need to be migrated manually, for example with
mockImplementation().