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

Add docs section to highlight jest.requireActual #6651

Merged
merged 2 commits into from
Jul 9, 2018

Conversation

bdrobinson
Copy link
Contributor

First – thanks so much for this project, I've been using Jest for a couple of years and have benefited from it a lot!

Summary

I've been in the situation a few times where I've had a problem that could be solved by require.requireActual but didn't know about it because it seems to be quite hidden away in the docs (as far as I could tell, it's only really documented in the "Globals" section of the API reference). As a result of not knowing about it, I've wasted quite a lot of hours writing my own manual mocks rather than using Jest's automocking features. This PR adds a section to the docs entitled "Bypassing module mocks" which presents an issue similar to one I encountered recently, and talks the reader through how require.requireActual can solve the problem.

Let me know what you think. I've tried to match the tone of the rest of Jest's docs, and tried to pick an example that's simple enough to quickly make sense of but also resonates with the sort of tests people would actually write in real life.

I'm not at all wedded to the particular example I use – I'm mostly just keen that something gets added to the docs that draws attention to require.requireActual.

When I built the docs locally (by running yarn start in the website folder) it didn't seem reflect my changes as I made them, so I haven't been able to verify if the markdown gets rendered into the web page correctly. But I imagine that's a simple task.

Thanks very much.

Test plan

N/A

// AFTER
jest.mock("node-fetch")
import fetch from "node-fetch"
const { Response } = require.requireActual("node-fetch")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you change all require.requireActual to jest.requireActual?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice! Is that something jest supports? I can't see any reference to it in https://jestjs.io/docs/en/jest-object. Is it worth changing that part of the docs too?

Copy link
Member

Choose a reason for hiding this comment

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

jest.requireActual was added in #4260 as an alias - and we want to encourage people to use the jest object.

Absolutely worth updating the docs, yeah 🙂

@SimenB
Copy link
Member

SimenB commented Jul 7, 2018

@SimenB
Copy link
Member

SimenB commented Jul 7, 2018

Also, please run yarn lint:md to fix the lint errors on CI 🙂

@bdrobinson
Copy link
Contributor Author

I've run the linting and updated it to refer to jest.requireActual rather than require.requireActual 👍

@codecov-io
Copy link

Codecov Report

Merging #6651 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6651   +/-   ##
=======================================
  Coverage   63.73%   63.73%           
=======================================
  Files         235      235           
  Lines        8938     8938           
  Branches        3        4    +1     
=======================================
  Hits         5697     5697           
  Misses       3240     3240           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 664681a...474ea70. Read the comment docs.

@SimenB SimenB requested a review from rickhanlonii July 7, 2018 22:03
@thymikee thymikee changed the title Add docs section to highlight require.requireActual Add docs section to highlight jest.requireActual Jul 8, 2018
import {createUser} from './createUser';

test('createUser calls fetch with the right args and returns the user id', async () => {
fetch.mockReturnValue(Promise.resolve(new Response('4')));
Copy link
Member

Choose a reason for hiding this comment

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

Could use fetch.mockResolvedValue(new Response('4')) here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, didn't know that existed!

@rickhanlonii
Copy link
Member

Great doc, thanks!

@rickhanlonii rickhanlonii merged commit fa92643 into jestjs:master Jul 9, 2018
@github-actions
Copy link

This pull request 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants