From 3164fc9b9fe25e764a469f85ac8fca922b6f5055 Mon Sep 17 00:00:00 2001 From: "Bajohr, Rayk" Date: Thu, 28 May 2026 09:39:59 +0200 Subject: [PATCH] fix(@schematics/angular): preserve Jasmine stub-by-default behavior for bare spyOn calls Bare `spyOn`/`spyOnProperty` calls in Jasmine create stubs that return `undefined` by default. Vitest's `vi.spyOn` passes through to the real implementation instead, changing observable behavior of migrated tests. Wrap bare spy calls with `.mockReturnValue(undefined)` to preserve the Jasmine semantics, except for `spyOnProperty(obj, prop, 'set')` where setter semantics already match. --- .../refactor/jasmine-vitest/index_spec.ts | 26 +++-- .../test-file-transformer.integration_spec.ts | 2 +- .../test-file-transformer_add-imports_spec.ts | 6 +- .../transformers/jasmine-spy.ts | 108 +++++++++++++++--- .../transformers/jasmine-spy_spec.ts | 56 ++++++++- 5 files changed, 163 insertions(+), 35 deletions(-) diff --git a/packages/schematics/angular/refactor/jasmine-vitest/index_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/index_spec.ts index fcb804886286..c3b402efced9 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/index_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/index_spec.ts @@ -59,7 +59,7 @@ describe('Jasmine to Vitest Schematic', () => { ); const newContent = tree.readContent(specFilePath); - expect(newContent).toContain(`vi.spyOn(service, 'myMethod');`); + expect(newContent).toContain(`vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`); }); it('should only transform files matching the fileSuffix option', async () => { @@ -94,7 +94,7 @@ describe('Jasmine to Vitest Schematic', () => { expect(unchangedContent).not.toContain(`vi.spyOn(window, 'alert');`); const changedContent = tree.readContent(testFilePath); - expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`); + expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`); }); it('should print verbose logs when the verbose option is true', async () => { @@ -120,7 +120,11 @@ describe('Jasmine to Vitest Schematic', () => { expect(logs).toContain('Detailed Transformation Log:'); expect(logs).toContain(`Processing: /${specFilePath}`); - expect(logs.some((log) => log.includes('Transformed `spyOn` to `vi.spyOn`'))).toBe(true); + expect( + logs.some((log) => + log.includes('Transformed `spyOn` to `vi.spyOn(...).mockReturnValue(undefined)`'), + ), + ).toBe(true); }); describe('with `include` option', () => { @@ -144,7 +148,7 @@ describe('Jasmine to Vitest Schematic', () => { ); const changedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts'); - expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`); + expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`); const unchangedContent = tree.readContent('projects/bar/src/app/app.spec.ts'); expect(unchangedContent).toContain(`spyOn(window, 'alert');`); @@ -158,7 +162,7 @@ describe('Jasmine to Vitest Schematic', () => { ); const changedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts'); - expect(changedContent).toContain(`vi.spyOn(window, 'confirm');`); + expect(changedContent).toContain(`vi.spyOn(window, 'confirm').mockReturnValue(undefined);`); const unchangedContent = tree.readContent('projects/bar/src/app/app.spec.ts'); expect(unchangedContent).toContain(`spyOn(window, 'alert');`); @@ -177,10 +181,12 @@ describe('Jasmine to Vitest Schematic', () => { ); const changedAppContent = tree.readContent('projects/bar/src/app/app.spec.ts'); - expect(changedAppContent).toContain(`vi.spyOn(window, 'alert');`); + expect(changedAppContent).toContain(`vi.spyOn(window, 'alert').mockReturnValue(undefined);`); const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts'); - expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`); + expect(changedNestedContent).toContain( + `vi.spyOn(window, 'confirm').mockReturnValue(undefined);`, + ); const unchangedContent = tree.readContent('projects/bar/src/other/other.spec.ts'); expect(unchangedContent).toContain(`spyOn(window, 'close');`); @@ -194,10 +200,12 @@ describe('Jasmine to Vitest Schematic', () => { ); const changedAppContent = tree.readContent('projects/bar/src/app/app.spec.ts'); - expect(changedAppContent).toContain(`vi.spyOn(window, 'alert');`); + expect(changedAppContent).toContain(`vi.spyOn(window, 'alert').mockReturnValue(undefined);`); const changedNestedContent = tree.readContent('projects/bar/src/app/nested/nested.spec.ts'); - expect(changedNestedContent).toContain(`vi.spyOn(window, 'confirm');`); + expect(changedNestedContent).toContain( + `vi.spyOn(window, 'confirm').mockReturnValue(undefined);`, + ); }); it('should throw if the include path does not exist', async () => { diff --git a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts index b84aeba57411..5b30e9f24f4b 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts @@ -109,7 +109,7 @@ describe('Jasmine to Vitest Transformer - Integration Tests', () => { }); it('should handle user click', () => { - vi.spyOn(window, 'alert'); + vi.spyOn(window, 'alert').mockReturnValue(undefined); const button = fixture.nativeElement.querySelector('button'); button.click(); fixture.detectChanges(); diff --git a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts index 82b76ee31782..f4b10d485920 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts @@ -13,7 +13,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => { const input = `spyOn(foo, 'bar');`; const expected = ` import { vi } from 'vitest'; - vi.spyOn(foo, 'bar'); + vi.spyOn(foo, 'bar').mockReturnValue(undefined); `; await expectTransformation(input, expected, true); }); @@ -27,7 +27,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => { import { type Mock, vi } from 'vitest'; let mySpy: Mock; - vi.spyOn(foo, 'bar'); + vi.spyOn(foo, 'bar').mockReturnValue(undefined); `; await expectTransformation(input, expected, true); }); @@ -41,7 +41,7 @@ describe('Jasmine to Vitest Transformer - addImports option', () => { import type { Mock } from 'vitest'; let mySpy: Mock; - vi.spyOn(foo, 'bar'); + vi.spyOn(foo, 'bar').mockReturnValue(undefined); `; await expectTransformation(input, expected, false); }); diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts index c840c374976d..cef25b17a49e 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts @@ -34,19 +34,7 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. ts.isIdentifier(node.expression) && (node.expression.text === 'spyOn' || node.expression.text === 'spyOnProperty') ) { - addVitestValueImport(pendingVitestValueImports, 'vi'); - reporter.reportTransformation( - sourceFile, - node, - `Transformed \`${node.expression.text}\` to \`vi.spyOn\`.`, - ); - - return ts.factory.updateCallExpression( - node, - createPropertyAccess('vi', 'spyOn'), - node.typeArguments, - node.arguments, - ); + return transformSpyExpression(node, refactorCtx); } if (ts.isPropertyAccessExpression(node.expression)) { @@ -93,8 +81,8 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. ); const returnValues = node.arguments; if (returnValues.length === 0) { - // No values, so it's a no-op. Just transform the spyOn call. - return transformSpies(spyCall, refactorCtx); + // No values, preserve Jasmine's stub-by-default behavior. + return transformSpyExpression(spyCall, refactorCtx, true); } // spy.and.returnValues(a, b) -> spy.mockReturnValueOnce(a).mockReturnValueOnce(b) let chainedCall: ts.Expression = spyCall; @@ -119,7 +107,8 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. 'Removed redundant `.and.callThrough()` call.', ); - return transformSpies(spyCall, refactorCtx); // .and.callThrough() is redundant, just transform spyOn. + // .and.callThrough() is redundant, just transform spyOn. + return transformSpyExpression(spyCall, refactorCtx, false); case 'stub': { reporter.reportTransformation( sourceFile, @@ -219,6 +208,67 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. return node; } +function transformSpyExpression( + node: ts.Expression, + refactorCtx: RefactorContext, + forceStubBehavior?: boolean, +): ts.Expression { + const { sourceFile, reporter, pendingVitestValueImports } = refactorCtx; + + if ( + ts.isCallExpression(node) && + ts.isIdentifier(node.expression) && + (node.expression.text === 'spyOn' || node.expression.text === 'spyOnProperty') + ) { + const spyFnName = node.expression.text; + addVitestValueImport(pendingVitestValueImports, 'vi'); + + const viSpyOnCall = ts.factory.updateCallExpression( + node, + createPropertyAccess('vi', 'spyOn'), + node.typeArguments, + node.arguments, + ); + + const shouldStub = forceStubBehavior ?? shouldStubBareSpyOn(node); + + if (shouldStub) { + reporter.reportTransformation( + sourceFile, + node, + `Transformed \`${spyFnName}\` to \`vi.spyOn(...).mockReturnValue(undefined)\` ` + + `to preserve Jasmine's stub-by-default behavior.`, + ); + + return ts.factory.createCallExpression( + createPropertyAccess(viSpyOnCall, 'mockReturnValue'), + undefined, + [ts.factory.createIdentifier('undefined')], + ); + } + + reporter.reportTransformation( + sourceFile, + node, + `Transformed \`${spyFnName}\` to \`vi.spyOn\`.`, + ); + + return viSpyOnCall; + } + + // Variable reference (e.g. `spy` from `spy.and.returnValues()`). + // There is no spyOn call to transform — only stub-wrapping is relevant here. + if (forceStubBehavior) { + return ts.factory.createCallExpression( + createPropertyAccess(node, 'mockReturnValue'), + undefined, + [ts.factory.createIdentifier('undefined')], + ); + } + + return node; +} + export function transformCreateSpy(node: ts.Node, ctx: RefactorContext): ts.Node { const { reporter, sourceFile, pendingVitestValueImports } = ctx; if (!isJasmineCallExpression(node, 'createSpy')) { @@ -395,6 +445,32 @@ export function transformSpyReset( return node; } +function shouldStubBareSpyOn(node: ts.CallExpression): boolean { + if ( + ts.isIdentifier(node.expression) && + node.expression.text === 'spyOnProperty' && + node.arguments.length >= 3 + ) { + const accessType = node.arguments[2]; + if (ts.isStringLiteralLike(accessType) && accessType.text === 'set') { + return false; + } + } + + const parent = node.parent; + if ( + parent && + ts.isPropertyAccessExpression(parent) && + parent.expression === node && + ts.isIdentifier(parent.name) && + parent.name.text === 'and' + ) { + return false; + } + + return true; +} + function getSpyIdentifierFromCalls(node: ts.PropertyAccessExpression): ts.Expression | undefined { if (ts.isIdentifier(node.name) && node.name.text === 'calls') { return node.expression; diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts index 97881049c1d5..f8e2beae4c36 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts @@ -11,9 +11,10 @@ import { expectTransformation } from '../test-helpers'; describe('Jasmine to Vitest Transformer - transformSpies', () => { const testCases = [ { - description: 'should transform spyOn(object, "method") to vi.spyOn(object, "method")', + description: + 'should transform bare spyOn to vi.spyOn(...).mockReturnValue(undefined) to preserve Jasmine stub-by-default semantics', input: `spyOn(service, 'myMethod');`, - expected: `vi.spyOn(service, 'myMethod');`, + expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`, }, { description: 'should transform .and.returnValue(...) to .mockReturnValue(...)', @@ -58,9 +59,10 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => { expected: `const mySpy = vi.fn(() => 'foo').mockName('mySpy');`, }, { - description: 'should transform spyOnProperty(object, "prop") to vi.spyOn(object, "prop")', + description: + 'should transform bare spyOnProperty to vi.spyOn(...).mockReturnValue(undefined) to preserve Jasmine stub-by-default semantics', input: `spyOnProperty(service, 'myProp');`, - expected: `vi.spyOn(service, 'myProp');`, + expected: `vi.spyOn(service, 'myProp').mockReturnValue(undefined);`, }, { description: 'should transform .and.stub() to .mockImplementation(() => {})', @@ -80,9 +82,11 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => { expected: `const mySpy = vi.fn().mockName('mySpy').mockReturnValue(true);`, }, { - description: 'should handle .and.returnValues() with no arguments', + description: + 'should transform .and.returnValues() with no args to vi.spyOn(...).mockReturnValue(undefined) ' + + 'because Jasmine reverts to stub-by-default', input: `spyOn(service, 'myMethod').and.returnValues();`, - expected: `vi.spyOn(service, 'myMethod');`, + expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`, }, { description: @@ -117,6 +121,46 @@ describe('Jasmine to Vitest Transformer - transformSpies', () => { expected: `// TODO: vitest-migration: Unsupported spy strategy ".and.unknownStrategy()" found. Please migrate this manually. See: https://vitest.dev/api/mocked.html#mock vi.spyOn(service, 'myMethod').and.unknownStrategy();`, }, + { + description: 'should preserve stub-by-default semantics for spyOn assigned to a variable', + input: `const spy = spyOn(service, 'myMethod');`, + expected: `const spy = vi.spyOn(service, 'myMethod').mockReturnValue(undefined);`, + }, + { + description: + 'should wrap spyOnProperty with explicit "get" access type to preserve stub-by-default semantics', + input: `spyOnProperty(service, 'myProp', 'get');`, + expected: `vi.spyOn(service, 'myProp', 'get').mockReturnValue(undefined);`, + }, + { + description: + 'should NOT wrap spyOnProperty with "set" access type because setter semantics already match', + input: `spyOnProperty(service, 'myProp', 'set');`, + expected: `vi.spyOn(service, 'myProp', 'set');`, + }, + { + description: + 'should wrap spyOn used as an expression argument to preserve stub-by-default semantics', + input: `expect(spyOn(service, 'myMethod')).toBeDefined();`, + expected: `expect(vi.spyOn(service, 'myMethod').mockReturnValue(undefined)).toBeDefined();`, + }, + { + description: + 'should wrap a spy variable with .and.returnValues() with no args to preserve stub-by-default semantics', + input: `spy.and.returnValues();`, + expected: `spy.mockReturnValue(undefined);`, + }, + { + description: 'should remove .and.callThrough() on a spy variable', + input: `spy.and.callThrough();`, + expected: `spy;`, + }, + { + description: + 'should preserve stub-by-default on spyOn with .calls.reset() chained (not an .and accessor)', + input: `spyOn(service, 'myMethod').calls.reset();`, + expected: `vi.spyOn(service, 'myMethod').mockReturnValue(undefined).mockClear();`, + }, ]; testCases.forEach(({ description, input, expected }) => {