Skip to content

Commit

Permalink
Make sinon.stub() mock implementation when nothing is returned (& oth…
Browse files Browse the repository at this point in the history 
…er tweaks) (#291)

Co-authored-by: Dan Beam <dan.beam@airbnb.com>
  • Loading branch information
Dan Beam and Dan Beam authored Jun 9, 2022
1 parent 93d99e0 commit 287b1e3
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 28 deletions.
43 changes: 31 additions & 12 deletions src/transformers/sinon.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@ beforeEach(() => {
jest.spyOn(console, 'warn').mockImplementation().mockClear()
})

function expectTransformation(source, expectedOutput, options = {}) {
const result = wrappedPlugin(source, options)
function expectTransformation(source, expectedOutput, warnings = []) {
const result = wrappedPlugin(source)
expect(result).toBe(expectedOutput)
expect(console.warn).toBeCalledTimes(0)
expect(console.warn).toBeCalledTimes(warnings.length)
warnings.forEach((warning, i) => {
expect(console.warn).nthCalledWith(i + 1, warning)
})
}

it('removes imports', () => {
Expand All @@ -34,21 +37,37 @@ describe('spies and stubs', () => {
expectTransformation(
`
import sinon from 'sinon-sandbox'
const stub = sinon.stub(Api, 'get')
const stub = sinon.stub(Api, 'get');
const putOk = sinon.stub(Api, 'put').returns(200)
sinon.stub(I18n); // unsupported
sinon.stub(I18n, 'extend');
sinon.stub(AirbnbUser, 'current').returns(currentUser);
sinon.stub(AirbnbUser, 'current').returnsArg(1);
sinon.stub(a, b, () => c, d) // too many args
sinon.spy(I18n, 'extend');
sinon.spy();
sinon.spy(() => 'foo');
sinon.spy(a, b, () => c, d) // too many args
`,
`
const stub = jest.spyOn(Api, 'get').mockClear()
jest.spyOn(I18n, 'extend').mockClear();
jest.spyOn(AirbnbUser, 'current').mockClear().mockReturnValue(currentUser);
const stub = jest.spyOn(Api, 'get').mockClear().mockImplementation();
const putOk = jest.spyOn(Api, 'put').mockClear().mockReturnValue(200)
sinon.stub(I18n); // unsupported
jest.spyOn(I18n, 'extend').mockClear().mockImplementation();
jest.spyOn(AirbnbUser, 'current').mockClear().mockImplementation((...args) => args[1]);
jest.spyOn(a, b).mockClear().mockImplementation(() => c) // too many args
jest.spyOn(I18n, 'extend').mockClear();
jest.fn();
jest.fn().mockImplementation(() => 'foo');
`
jest.fn(() => 'foo');
jest.spyOn(a, b).mockClear().mockImplementation(() => c) // too many args
`,
[
'jest-codemods warning: (test.js line 6) stubbing all methods in an object is not supported; stub each one you care about individually',
'jest-codemods warning: (test.js line 9) 4+ arguments found in sinon.stub call; did you mean to use this many?',
'jest-codemods warning: (test.js line 14) 4+ arguments found in sinon.spy call; did you mean to use this many?',
]
)
})

Expand Down Expand Up @@ -79,8 +98,8 @@ describe('spies and stubs', () => {
`,
`
beforeEach(() => {
jest.spyOn(Api, 'get').mockClear()
const s1 = jest.spyOn(I18n, 'extend').mockClear()
jest.spyOn(Api, 'get').mockClear().mockImplementation()
const s1 = jest.spyOn(I18n, 'extend').mockClear().mockImplementation()
const s2 = jest.spyOn(I18n, 'extend').mockClear().mockReturnValue('en')
jest.spyOn(L10n, 'language').mockClear().mockReturnValue('en')
jest.spyOn(I18n, 'extend').mockClear().mockImplementation(() => 'foo');
Expand Down
65 changes: 49 additions & 16 deletions src/transformers/sinon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '../utils/chai-chain-utils'
import finale from '../utils/finale'
import { removeDefaultImport } from '../utils/imports'
import logger from '../utils/logger'
import { findParentOfType } from '../utils/recast-helpers'
import {
expressionContainsProperty,
Expand Down Expand Up @@ -52,6 +53,7 @@ const SINON_MATCHERS_WITH_ARGS = {
}
const SINON_NTH_CALLS = new Set(['firstCall', 'secondCall', 'thirdCall', 'lastCall'])
const EXPECT_PREFIXES = new Set(['to'])
const isPrefix = (name) => EXPECT_PREFIXES.has(name)

/*
expect(spy.called).to.be(true) -> expect(spy).toHaveBeenCalled()
Expand Down Expand Up @@ -102,7 +104,6 @@ function transformCallCountAssertions(j, ast) {

const expectArgSinonMethod = expectArg.property.name

const isPrefix = (name) => EXPECT_PREFIXES.has(name)
const negated =
chainContains('not', node.callee, isPrefix) || node.arguments?.[0].value === false // eg: .to.be(false)
const rest = getAllBefore(isPrefix, node.callee, 'should')
Expand Down Expand Up @@ -164,7 +165,6 @@ function transformCalledWithAssertions(j, ast) {
})

const expectArgSinonMethod = expectArg.callee?.property?.name
const isPrefix = (name) => EXPECT_PREFIXES.has(name)
const negated =
chainContains('not', node.callee, isPrefix) || node.arguments?.[0].value === false // eg: .to.be(false)
const rest = getAllBefore(isPrefix, node.callee, 'should')
Expand All @@ -183,7 +183,7 @@ function transformCalledWithAssertions(j, ast) {
/*
sinon.stub(Api, 'get') -> jest.spyOn(Api, 'get')
*/
function transformStub(j: core.JSCodeshift, ast, sinonExpression) {
function transformStub(j: core.JSCodeshift, ast, sinonExpression, logWarning) {
ast
.find(j.CallExpression, {
callee: {
Expand All @@ -199,9 +199,18 @@ function transformStub(j: core.JSCodeshift, ast, sinonExpression) {
},
})
.replaceWith((np) => {
// stubbing/spyOn module
const args = np.value.arguments
const propertyName = np.node.callee.property.name

if (args.length === 1 && propertyName === 'stub') {
logWarning(
'stubbing all methods in an object is not supported; stub each one you care about individually',
np
)
return np.value
}

// stubbing/spyOn module
if (args.length >= 2) {
let spyOn = j.callExpression(
j.memberExpression(j.identifier('jest'), j.identifier('spyOn')),
Expand All @@ -212,27 +221,49 @@ function transformStub(j: core.JSCodeshift, ast, sinonExpression) {
spyOn = j.callExpression(j.memberExpression(spyOn, j.identifier('mockClear')), [])

// add mockImplementation call
if (args.length === 3) {
if (args.length >= 3) {
spyOn = j.callExpression(
j.memberExpression(spyOn, j.identifier('mockImplementation')),
[args[2]]
)

if (args.length >= 4) {
logWarning(
`4+ arguments found in sinon.${propertyName} call; did you mean to use this many?`,
np
)
}
} else if (propertyName === 'stub') {
const parent =
findParentOfType(np, 'VariableDeclaration') ||
findParentOfType(np, 'ExpressionStatement')

const hasReturn =
j(parent)
.find(j.CallExpression, {
callee: {
type: 'MemberExpression',
property: {
type: 'Identifier',
name: (name) => ['returns', 'returnsArg'].includes(name),
},
},
})
.size() > 0

if (!hasReturn) {
spyOn = j.callExpression(
j.memberExpression(spyOn, j.identifier('mockImplementation')),
[]
)
}
}

return spyOn
}

const jestFnCall = j.callExpression(j.identifier('jest.fn'), [])

if (args.length === 1) {
return j.callExpression(
j.memberExpression(jestFnCall, j.identifier('mockImplementation')),
args
)
}

// jest mock function
return jestFnCall
return j.callExpression(j.identifier('jest.fn'), args)
})
}

Expand Down Expand Up @@ -650,7 +681,9 @@ export default function transformer(fileInfo: FileInfo, api: API, options) {
return null
}

transformStub(j, ast, sinonExpression)
const logWarning = (msg, node) => logger(fileInfo, msg, node)

transformStub(j, ast, sinonExpression, logWarning)
transformMockTimers(j, ast)
transformMock(j, ast)
transformMockResets(j, ast)
Expand Down

0 comments on commit 287b1e3

Please sign in to comment.