Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blacklist spyOn(). Add explicit spyOnProd() and spyOnDevAndProd() #11691

Merged
merged 3 commits into from Nov 28, 2017

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Nov 28, 2017

Seems like it's too easy to use spyOn when we shouldn't. Let's blacklist it in favor of explicitly-scoped spy methods.

There is a comment in scripts/jest/setupTests.js that I don't understand. It says:

// TODO: Stop using spyOn in all the test since that seem deprecated.

Why do we think spyOn is deprecated? The official Jest docs list jest.spyOn as being available for 19.0.0+. Is this comment maybe referring to the call-through behavioral differences or something?

I also updated scripts/jest/spec-equivalence-reporter/setupTests.js and scripts/jest/typescript/jest.d.ts although neither is necessary at this time.

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

I like this

@bvaughn bvaughn merged commit 53ab194 into facebook:master Nov 28, 2017
@bvaughn bvaughn deleted the spy-on-prod branch November 28, 2017 22:06
@SimenB
Copy link
Contributor

SimenB commented Dec 20, 2017

@bvaughn
Copy link
Contributor Author

bvaughn commented Dec 20, 2017

Thanks for the clarification, @SimenB 😄

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

Successfully merging this pull request may close these issues.

None yet

4 participants