Skip to content

fix(@schematics/angular): preserve Jasmine stub-by-default semantics for bare spies#33266

Open
clydin wants to merge 2 commits into
angular:mainfrom
clydin:fix/refactor-jasmine-vitest-stub
Open

fix(@schematics/angular): preserve Jasmine stub-by-default semantics for bare spies#33266
clydin wants to merge 2 commits into
angular:mainfrom
clydin:fix/refactor-jasmine-vitest-stub

Conversation

@clydin
Copy link
Copy Markdown
Member

@clydin clydin commented May 28, 2026

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.

@clydin clydin force-pushed the fix/refactor-jasmine-vitest-stub branch from 620ba09 to aa35769 Compare May 28, 2026 15:16
@clydin clydin marked this pull request as ready for review May 28, 2026 15:37
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the Jasmine-to-Vitest schematic spy transformer to append .mockReturnValue(undefined) by default to spyOn and spyOnProperty calls, preserving stub-by-default semantics unless chained with .and strategies. It also restructures the transformation logic into smaller helper functions to support chained spies with bracket notation, non-null assertions, type assertions, and satisfies expressions. The code review feedback recommends adding defensive checks to prevent potential runtime crashes when returnValue() or throwError() are called without arguments, and suggests using ts.isStringLiteralLike to more robustly handle bracket notation.

Comment thread packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts Outdated
Comment thread packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts Outdated
clydin added 2 commits May 28, 2026 11:52
…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 angular#33253
@clydin clydin force-pushed the fix/refactor-jasmine-vitest-stub branch from aa35769 to 1954ddb Compare May 28, 2026 15:53
@clydin clydin added target: rc This PR is targeted for the next release-candidate action: review The PR is still awaiting reviews from at least one requested reviewer labels May 28, 2026
@clydin clydin requested a review from hawkgs May 28, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: @schematics/angular target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor-jasmine-vitest: Bare spyOn() migration silently changes test behavior - should emit .mockReturnValue(undefined)

1 participant