jestjs / jest

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

DISCUSSION: Why are mocks static in Jest? #2577

Closed faceyspacey closed 6 years ago

faceyspacey commented 7 years ago

It means you need to create unnecessary files to use different mocks for the groups of code that you're testing. And that just gets messy having additional test files that really should be part of one test suite. And when you don't do that, you resolve to sub-optimal workarounds--perhaps not writing unit tests as granularly as you should.

I'm sure there are precise reasons, but this isn't ideal and not expected. I write this now after spending weeks testing a code-base and getting very familiar with Jest, not just after my first usage of Jest. The "ideal Jest" allows you to dynamically create different mocks per test. Is there any hope for this future?

Ultimately, you should be able to do this:

it('test something', () => {
     const mod = jest.genMockFromModule('../myModule') // i know this is not current interface, but if you could do this, dynamically, you wouldn't need to both mock it and require it twice
     expect(mod.methodThatCallsMockedMethod()).toEqual(false)

     mod.oneOfManyMethods = () => true
     expect(mod.methodThatCallsMockedMethod()).toEqual(true)
})

// this ultimately was the main example I was speaking of that you 
// should be able to do on a per-test basis:

it('test something else in same file', () => {
     jest.mock('../myModule', () => require('../__mocks__/myModule'))

     const mod = require('../anotherModuleThatUsesMyModule')
     expect(mod.methodThatCallsMockedMethodFromMyModule()).toEqual(true)
})

Without that you're married to one mock per imported module per test file. Jest is lovely, but you're really put in a hard place if you're constantly being forced to question whether you should just make another test file that contains one test associated with a larger test suite so that that one test can do something a big different with mocks. I.e. often you'll have 2 layers to your abstraction in one module (i.e. 2 functions, one that calls the other) and you want to write both unit tests testing the functions individually where one of the 2 functions must be mocked, and other times you want 2 them both in coordination. That's the prime example. So you can't do that nicely currently with Jest.

To give a frame of reference, what you CAN do nicely with jest is what I just described but with modules instead of functions, i.e. where you have 2 modules in separate files where you want to test them together and independently and in one test suite file. What you do is this:

const moduleA = require.requireActual('../moduleA')
const moduleB = require.requireActual('../moduleB')

jest.mock('../moduleB', () => require('../__mocks/moduleB'))

it('test moduleA that under the hood imports moduleB, and therefore uses the mock', () => {
      expect(moduleA.doSomething).toEqual('foo')
})

it('test moduleB directly unmocked since we used requireActual', () => {
      expect(moduleB.doSomethingElse).toEqual('bar')
})

So that works nicely there and is an idiomatic enough solution. But when it comes to operating at the function level, Jest does not quite shine as brightly :(. I don't mean this to be negative, but what are the thoughts on this and what are our future prospects to improve this?


Another related question is why in mock functions using jest.fn(() => /** everything must be defined here **/) must any variables be within the scope of fn? Again, that limits what you can do greatly.

It seems that various static checks are inhibiting a lot of possibilities with Jest that you would expect from a dynamic language like javascript at runtime. It takes a bit of getting used to and feels very different than every other way you're using javascript.

robertmain commented 6 years ago

I'm with you on this, I'm looking for another way to create mocks of my objects without creating actual mock objects. It would be nice to be able to define the mocks in the test suite.

faceyspacey commented 6 years ago

I found you can actually change them per test with doMock and resetting the mock cache, etc. it's a bit annoying but it totally works.

robertmain commented 6 years ago

Can you define them per-test though? If so - how?

On a side note: I'm starting to wonder how maintained Jest actually is...I thought it might be taken care of like React - but between scant documentation and a distinct absence on the issue tracker I'm starting to get the feeling they don't really give a damn

I kind of feel like the stone age peasants in Stargate who are still worshipping the Goa'uld thousands of years after their God upped and buggered off to another planet

faceyspacey commented 6 years ago

Jest works great. It's primarily one guy. One can you expect.

Lookup doMock

robertmain commented 6 years ago

It is?! In that case, I take back what I said - I thought it was like a huge facebook internal project like React....

faceyspacey commented 6 years ago

The dude works full time for Facebook. It is an internal FB project, just not a "huge" one lol. But it is massive. It's rough around the edges in a few places but once you find the workarounds you can make it frictionless. It's really fantastic from my perspective.

robertmain commented 6 years ago

It looks nice, but I'm running into quite a few weird edge cases that I can't seem to find workarounds - for example #4960

faceyspacey commented 6 years ago

do you use https://wallabyjs.com. it's the best thing that ever happened to javascript testing. it has coverage in there (but I assume it's using coverage reports generated by Jest). Either way, it's awesome, and the only real way to do TDD with js, as it re-runs your tests on every keypress, and shows in a web panel which tests pass/fail, and fail on what lines. It also shows in your editor on what lines fails are coming from.

One other key thing is it shows Jest snapshot diffs right there in their web panel. And lets you press a camera icon to accept changes.

robertmain commented 6 years ago

I honestly don't. I've never heard of it before. Yeah, I'm using Jest for coverage. I have visual studio code setup to run my tests every time I save a test or a file required by a test.

Jest has a --watchAll option too btw.

How does the coverage work in that - presumably it would still need Jest to collect the coverage data(which is what I think might be going astray)?

Also - it looks like wallaby personal licences cost $100 so I think I'll pass (not least because it's for someone else's GitHub project) :). Thanks for the heads up though - I'll bear it in mind for future.

cpojer commented 6 years ago

I'm starting to wonder how maintained Jest actually is...I thought it might be taken care of like React - but between scant documentation and a distinct absence on the issue tracker I'm starting to get the feeling they don't really give a damn I thought it was like a huge facebook internal project like React....

Hi! We don't actively work on the open source testing parts of Jest at this time so if you'd like to help improve the project we will absolutely appreciate your contributions. We do have a team that owns the Jest project but this team is also working on Yarn and Metro. We are only 8 people including me and Metro is our biggest focus right now. For comparison, the "big internal React project" is 7 people, including an engineering manager. Facebook has more than 20,000 employees. I hope that helps put things into perspective. Personally, I'm managing the team I talked about above and spend my weekends or mornings on Jest to keep the project moving but that isn't sufficient to write much code. It would be great if you could help out with productive discussions and pull requests to push for the changes that matter to you. I am also curious to understand what is missing in the Jest documentation. We spent an extensive amount writing it together with the community and your comment was frankly quite insulting to the people that spend their free time contributing to Jest. Thank you.

I kind of feel like the stone age peasants in Stargate who are still worshipping the Goa'uld thousands of years after their God upped and buggered off to another planet

Time to step up, stop worshipping the gods, and contribute. Can't rely on the Goa'uld forever to build you nice and shiny pyramids for free.

Another related question is why in mock functions using jest.fn(() => / everything must be defined here /) must any variables be within the scope of fn? Again, that limits what you can do greatly.

I think you are talking about jest.mock, not jest.fn. This is because of babel-plugin-jest-hoist which hoists jest.mock calls to the top of the scope so that you'll be able to use ES imports and jest.mock together. When we move this during the transformation step, it would create cycles with any uninitialized variables if that module is required during initialization of the module that requires it. If we didn't do this you would be unable to mock ES modules. There is an escape hatch as the error message describes if you know what you are doing.

For the actual main issue at hand, I have some trouble understanding the discussion and the example in the original post to be honest. That first example you gave me works if you reset the mock and module state. It entirely depends on the internal implementation. If you are using ES exports we are unable to reach into the module and swap out the implementation of an exported binding after the fact. What exactly are you unable to do and how would you like to do it? Jest offers a wide array of functionality to control the mock behavior completely and it should be up to you how to structure it. Have you tried jest.resetModules() which resets the module state and allows you to require fresh modules so you can mock them out? There are also API calls to reset or clear the mock state of specific mocks or all of them. See http://facebook.github.io/jest/docs/en/jest-object.html#jestresetallmocks and others.

faceyspacey commented 6 years ago

Have you read what I posted to the other guy? I wrote him the answer with doMock and resetModules. My issue is a year old. Just close it out brotha. Jest is the bomb! One love.

robertmain commented 6 years ago

Hi @cpojer.

My intention was not to insult the people that spend their free time contributing to Jest so I apologise if that's how it came across - at the same time I hope you also understand my position as someone looking for a test framework. If the test framework that that I'm looking at isn't currently being actively worked on(as you said yourself - "We don't actively work on the open source testing parts of Jest at this time") then that is concerning. I was also under the impression that Jest was being maintained by a large team (see also: https://github.com/facebook/jest/issues/2577#issuecomment-348360873 and https://github.com/facebook/jest/issues/2577#issuecomment-348362139) and was just being ignored. I would also add that adopting a defensive stance is unlikely to encourage a "productive discussion".

Usage of a library is a commitment on the part of the library consumer. Perhaps not one that benefits the project outright, but it's an investment of time and effort so it's important to make sure that this time and effort is well invested on a library that is well maintained and looked after. So, if - in a (relatively) new library - I'm running into odd behaviour that I can't find a reason for in the docs(#4960 and #2256) and there are issues sitting in the issue tracker from the beginning of the year I start to have concerns regarding whether the project is abandoned. Whilst I'm sure there are lots of popular projects on GitHub that have lots of outstanding issues (Mocha being an excellent example of this with almost 300 open issues) - these projects are often fairly well known and established and therefore probably aren't going away any time soon(Jest being comparatively new).

I would be interested in submitting patches in the future, as you suggested - though right now I'm submitting patches to (and implementing tests on) another library(which is ironically what brought me here in the first place).

I am also curious to understand what is missing in the Jest documentation.

I'm glad you brought this up. Perhaps I missed something, but it would be nice to have a section on automatic mocks beside Manual Mocks under guides explaining automatic mocks, why they were disabled by default how they work etc.

Also - I recall fs being mocked out for reading data in one of the examples. However - I'm not seeing anything about writing to and then making assertions on the mocked out fs.

Since I'm not a fan of having static mock objects in my code base - I'd also be interested to know if it's possible to define a mock implementation and what it should return directly in the test case kind of like you can do with PHPUnit. I recall @faceyspacey mentioning that this was possible with doMock() but it might be handy to have a mention of this under "Guides"

Finally - with regards to #2256 I think it might be useful to explain this behaviour here

cpojer commented 6 years ago

Thanks for the comments about how to improve our documentation. I encourage you, when you find the time, to send a PR to improve the current documentation to make it easier to understand for Jest's users. Your comments all make sense. For an FS mock, this has been on a list for a long time but again, nobody had enough time to build it.

As far as maintenance of this project concerned, Jest is doing great. We receive a healthy amount of contributions from the open source community and have a number of core contributors who are awesome and keep the Jest project going. However, I encourage you not to base your adoption of open source software on whether other people take care of a project but rather whether you can keep using and maintaining the project if nobody else is left to take care of it. For issues that have not be worked on for multiple years, this means that nobody felt strongly enough to volunteer in their free time to build it. If you want it, please build it and send pull requests to keep the project going.

Finally, the Jest project is actually almost seven years old and was open sourced almost four years ago. It's an integral part of JavaScript testing at Facebook. A number of packages part of Jest's platform are core to metro. The Jest project isn't going away.

robertmain commented 6 years ago

Sure, so I think part of the problem here is that I don't quite understand the inner workings of Jest well enough to document it - at least not yet. I wasn't so much suggesting an fs as just an update to the documentation. The docs explain how to read from a mocked file system, but not how to write to one...

However, I encourage you not to base your adoption of open source software on whether other people take care of a project but rather whether you can keep using and maintaining the project if nobody else is left to take care of it. For issues that have not be worked on for multiple years, this means that nobody felt strongly enough to volunteer in their free time to build it. If you want it, please build it and send pull requests to keep the project going.

Again - whilst I agree with this, the current project I'm working on isn't my own project. So, whilst I might be happy to commit to maintaining something - I don't think it's necessarily fair for me to make that commitment on someone else's behalf.

It's an integral part of JavaScript testing at Facebook. A number of packages part of Jest's platform are core to metro

That in itself is interesting. Do you happen to know why Facebook aren't lending your team more devs to help out? If it's a core project, I'd have thought they would.

cpojer commented 6 years ago

That's mostly because there is only a finite amount of time available and other things tend to have higher priority, for example right now the main focus of the team I work with is https://github.com/facebook/metro