jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.28k stars 6.47k forks source link

Rename jest.fn() to jest.spy() #1592

Closed gaearon closed 8 years ago

gaearon commented 8 years ago

I understand it’s a big change but should be super easy to find/replace if we do this. Why:

cpojer commented 8 years ago

It used to be called jest.genMockFunction() which was very confusing. In reality, this is used a lot and the longer it is, the fewer people use it and the more annoying it is. It litters the code a lot if it is longer too. I'll argue that expect is not really a better place for it.

My hope was that jest function becomes something that people think is synonymous with "mock function" or "stub function". I understand that jest.fn is more obvious to people that have used Jest for a long time (like engineers at Facebook) but I would like to take a revamp of our mock capabilities a lot further than just a rename: I want to build the best mocking library that's out there. We should come up with a perfect API for this and then go and build it.

We can also make a special fake module available in tests that gives you the jest runtime object. We can't really call it jest because then you cannot require jest inside of a test (like we do in Jest's own tests). What would we call such a module? require('⚒')? :D

Note: jest is not technically a global, it is injected into the module scope and every module has a separate copy of the jest runtime object. This doesn't concern users, but it is nevertheless interesting.

gaearon commented 8 years ago

The reason I’m saying it is as much as I like Jest, I just feel uncomfortable replacing expect.createSpy() in my code with jest.fn(). It feels wrong to use something implicitly injected called jest for something non-Jest-specific like creating a spy.

I can understand jest.mock() or jest.useFakeTimers() because those are Jest-specific features, but typing jest.fn() for every spy feels like Jest is trying to tie me down to itself.

I know it’s irrational, but programming is irrational in many ways. I’m sure I’m not the only one feeling uneasy because of such a silly reason.

cpojer commented 8 years ago

Wait, is expect.createSpy a thing other frameworks use? In that case I'm happy to support it if its common.

gaearon commented 8 years ago

It’s not commonly put on expect global but there’s a fairly popular expect library doing that.

fson commented 8 years ago

expect in that case is the name of the library, though. If we were to look for a "pattern" in the wild, it would be to put the function on the global of the library. For example:

In Jest docs it's called mock function, not spy or stub, so jest.fn() practically follows this pattern already (jest.genMockFunction() really being too long name for a function that gets used so much). Renaming it to expect.createSpy also wouldn't make it work out of the box with code written for expect, because the signature is different: you can pass the function implementation to jest.fn: jest.fn(() => 3), whereas with expect you would do expect.createSpy().andReturn(3). So if you were to move to Jest mocks, you'd have to codemod that anyway.

Note also that if you want to port existing code gradually and not yet buy into all of Jest's built-in features, you can always use expect assertions and spies or sinon spies and stubs with Jest and it will work.

gaearon commented 8 years ago

OK. Can jest.fn() become jest.spy()? It’s just one more letter and way more descriptive. We can change the docs to call them “spies” anytime too.

fson commented 8 years ago

I think jest.spy() would be a good name. It's short without being an abbreviation, spy is a common name for this functionality in other libraries, and even Jest docs already acknowledge it:

screen shot 2016-09-03 at 15 01 09
gaearon commented 8 years ago

The reason I’d like to rename them to “spies” completely is because this makes the distinction between “mocking” feature (for module system, depends on Jest) and “spies” (for functions, not really Jest-specific) more clear.

Daniel15 commented 8 years ago

Won't jest.spy get confused with jasmine.createSpy, which can also be used in Jest tests?

gaearon commented 8 years ago

AFAIK none of jasmine APIs are officially supported in Jest and the plan is to eventually remove them. The official Jest API is here: https://facebook.github.io/jest/docs/api.html. The rest just happens to work.

I might be wrong on this though.

quantizor commented 8 years ago

It's actually closer to a stub than a spy I think.

Daniel15 commented 8 years ago

It's actually closer to a stub than a spy I think.

True - I don't think Jest provides an easy way to call the original implementation (ie. and.callThrough() in Jasmine), which is one of the things that differentiates spies from stubs.

cpojer commented 8 years ago

To me, a spy is something that wraps around an existing function. We haven't added a helper for this yet, but I'd like to add

jest.spyOn(MyClass.prototype, 'onClick');

which would return a mock function. Therefore I'm unsure whether spy is really a good terminology for this.

In the end, all it is is really just a function that has a mock property with some data on it. Therefore, mock function and jest.fn seemed like a really convenient shortcut.

I do however understand Dan's concern about jest being a global. As mentioned before, we could make the jest runtime require-able but then it needs to have a unique name that isn't jest.

cpojer commented 8 years ago

This is gonna be part of renaming jest-matchers to expect. I'll close this issue for now. https://github.com/facebook/jest/issues/1679

oscarmorrison commented 7 years ago

+1

nemanjacosovic commented 6 years ago

+1

colshacol commented 6 years ago

I know this is quite old and closed, but I just couldn't help myself.

but then it needs to have a unique name that isn't jest.

*drumroll*

jestr

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.