Skip to content

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

Merged
clydin merged 2 commits into
angular:mainfrom
clydin:fix/refactor-jasmine-vitest-stub
May 29, 2026
Merged

fix(@schematics/angular): preserve Jasmine stub-by-default semantics for bare spies#33266
clydin merged 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
@clydin clydin requested a review from alan-agius4 May 29, 2026 15:03
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 29, 2026
@alan-agius4 alan-agius4 removed the request for review from hawkgs May 29, 2026 15:21
@clydin clydin merged commit 0fdd724 into angular:main May 29, 2026
41 checks passed
@clydin
Copy link
Copy Markdown
Member Author

clydin commented May 29, 2026

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker 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)

2 participants