feat(@jest/mock): Add withImplementation #13281

jeppester · 2022-09-19T08:54:38Z

Summary

A follow-up for #9270

It's often useful to override mock implementations for specific tests, and mockImplementationOnce is there to help out.

It is however not a very elegant solution for situations where the mock will get called multiple times:

It's not intuitive that you can call the method multiple times to "plan" for multiple calls
The number of calls to the mock might be unimportant to the test. mockImplementationOnce makes the number of calls unecessarily important.
The number of calls to the mock becomes part of the "arrange" phase of the test rather than the "assert" phase.
mockImplementationOnce will bleed into the next test if the implementation does not end up getting called.

This PR implements a new method withImplementation which sets a temporary default implementation which will be available within a callback, after which the default implementation will be set back to what it was before. If the callback returns a promise, withImplemenation will return a promise that can be awaited in the test.

I had to add a couple of @ts-expect-error comments to get the returned value of withImplementation (promise or void) match the return value of the callback implementation. If you know a better way of achieving this, let me know.

Also, I could potentially be useful to print a warning if the callback does not trigger a call to the mock - in which case withImplementation was redundant. What do you think about that idea?

Test plan

I've added unit tests for the feature. Let me know if I need to add more tests.

For temporarily overriding mock implementations.

packages/jest-mock/src/index.ts

packages/jest-mock/README.md

packages/jest-mock/src/__tests__/index.test.ts

mrazauskas · 2022-09-19T15:03:13Z

Could you add type tests, please? For completeness and to make sure that generic types will work as expected for the user.

The tests live in this file. To run them build the library and run yarn test-types. Remember to rebuild after each change. If you are using IDE, ignore the red errors the test file. These are caught by expectError.

packages/jest-mock/src/index.ts

docs/MockFunctionAPI.md

Co-authored-by: Tom Mrazauskas <[email protected]>

@ts-expect-error

To avoid @ts-expect-error

jeppester · 2022-09-20T19:34:21Z

I believe all the feedback has been addressed now.
Let me know if I need to do more.

packages/jest-mock/__typetests__/mock-functions.test.ts

Co-authored-by: Tom Mrazauskas <[email protected]>

jeppester · 2022-09-22T06:22:23Z

@mrazauskas
I believe there are no issues left. What is the next step from here?

mrazauskas · 2022-09-22T06:24:23Z

Right, all looks good to me. So we have to wait for @SimenB, because only he is able to merge PRs.

SimenB · 2022-09-23T11:05:44Z

Also, I could potentially be useful to print a warning if the callback does not trigger a call to the mock - in which case withImplementation was redundant. What do you think about that idea?

I like that idea!

SimenB

nice stuff!

CHANGELOG.md

docs/MockFunctionAPI.md

packages/jest-mock/README.md

packages/jest-mock/src/__tests__/index.test.ts

SimenB · 2022-09-23T11:09:12Z

packages/jest-mock/src/index.ts

+          typeof returnedValue === 'object' &&
+          returnedValue !== null &&


Suggested change

typeof returnedValue === 'object' &&

returnedValue !== null &&

returnedValue != null &&

typeof returnedValue === 'object' &&

probably doesn't matter, but easier to bail out early

Good point. Like I mentioned in one of my previous comments I copied this verbatim from jest circus.

So there's probably a nice tiny performance enhancement waiting to be made there as well:
https://github.com/facebook/jest/blob/a20fd859673800c50f8f089cfb4a87faec119525/packages/jest-circus/src/utils.ts#L271-L275

Hmm, we actually have https://github.com/facebook/jest/blob/c3a000e31343dd9e37da159e5903376c4312a0fd/packages/jest-util/src/isPromise.ts - should probably use that

haha, the linked SO answer says not to use it 🙈

I opened #13314

Co-authored-by: Simen Bekkhus <[email protected]>

Remove redundant `async` Co-authored-by: Simen Bekkhus <[email protected]>

Change promise detection expression to bail out earlier for `null` and `undefined` Co-authored-by: Simen Bekkhus <[email protected]>

mrazauskas · 2022-09-23T12:55:11Z

packages/jest-mock/src/__tests__/index.test.ts

+      testFn();
+      expect(mock1).toHaveBeenCalled();
+
+      expect.assertions(3);


Just wondering: should expect.assertions be added to the examples in docs as well? I mean, if users must use expect inside the callback, it would be good to show that in the example. Or?

Since the callback function will always be executed immediately by withImplementation - no matter if it's async or not - I wouldn't think expect.assertions is necessary outside of jest's own test suite. 🤔

Ah, right. Got it (;

jeppester · 2022-09-23T13:15:41Z

I might be wrong but to me it seems that the failing job random and not related to the PR.
If you agree with that assumption, can I get you to rerun the job?

If that job succeeds I believe this PR is ready to merge. - that is unless you have more comments.

Btw. thank you for the thorough, kind, and useful reviews 👍, it's a great experience to contribute.

SimenB · 2022-09-23T13:17:02Z

Yeah, you can safely ignore that failure 👍

I'll land #13314, then merge main into this PR, then I'll land this one.

Thanks for the patience and great contribution! 🎉

SimenB · 2022-09-28T07:41:45Z

https://github.com/facebook/jest/releases/tag/v29.1.0

github-actions · 2022-10-30T00:11:32Z

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.

facebook-github-bot added the cla signed label Sep 19, 2022

jeppester force-pushed the feature/with-implementation branch from 3d4d3c1 to ecb89f1 Compare September 19, 2022 08:57

jeppester mentioned this pull request Sep 19, 2022

feat: implement mockRestoreInitialImplementation and restoreAllInitialMockImplementations #9270

Closed

feat(@jest/mock): Add withImplementation

c4cb677

For temporarily overriding mock implementations.

jeppester force-pushed the feature/with-implementation branch from ecb89f1 to c4cb677 Compare September 19, 2022 09:06

jeppester commented Sep 19, 2022

View reviewed changes

packages/jest-mock/src/index.ts Outdated Show resolved Hide resolved

jeppester commented Sep 19, 2022

View reviewed changes

packages/jest-mock/src/index.ts Outdated Show resolved Hide resolved

mrazauskas reviewed Sep 19, 2022

View reviewed changes

packages/jest-mock/README.md Show resolved Hide resolved

mrazauskas reviewed Sep 19, 2022

View reviewed changes

packages/jest-mock/src/__tests__/index.test.ts Outdated Show resolved Hide resolved

mrazauskas reviewed Sep 19, 2022

View reviewed changes

packages/jest-mock/src/index.ts Outdated Show resolved Hide resolved

mrazauskas reviewed Sep 19, 2022

View reviewed changes

packages/jest-mock/src/index.ts Outdated Show resolved Hide resolved

Add docs for mockFn.withImplementation

dd2bb3c

mrazauskas reviewed Sep 20, 2022

View reviewed changes

docs/MockFunctionAPI.md Outdated Show resolved Hide resolved

jeppester and others added 4 commits September 20, 2022 08:50

Use util.types.isPromise in jest-mock test

bc2643b

Fix incorrect usage of js tab

dcef50b

Co-authored-by: Tom Mrazauskas <[email protected]>

Use function overload for types for mock.withImplemenation

12fddb2

To avoid @ts-expect-error

Merge branch 'main' into feature/with-implementation

b74d73b

mrazauskas reviewed Sep 21, 2022

View reviewed changes

packages/jest-mock/__typetests__/mock-functions.test.ts Outdated Show resolved Hide resolved

Update packages/jest-mock/__typetests__/mock-functions.test.ts

bd99f78

Co-authored-by: Tom Mrazauskas <[email protected]>

SimenB approved these changes Sep 23, 2022

View reviewed changes

jeppester and others added 5 commits September 23, 2022 14:24

Update docs/MockFunctionAPI.md

93087bc

Co-authored-by: Simen Bekkhus <[email protected]>

Update packages/jest-mock/README.md

d252023

Co-authored-by: Simen Bekkhus <[email protected]>

Update packages/jest-mock/src/__tests__/index.test.ts

c776293

Remove redundant `async` Co-authored-by: Simen Bekkhus <[email protected]>

Update packages/jest-mock/src/index.ts

20300d8

Change promise detection expression to bail out earlier for `null` and `undefined` Co-authored-by: Simen Bekkhus <[email protected]>

Add expect.assertions to jest-mock withImplementation tests

67a4671

jeppester added 2 commits September 23, 2022 14:40

Alphabetize changelog features

1c0d3bd

Add missing ) in mock-functions type test

7acd442

mrazauskas reviewed Sep 23, 2022

View reviewed changes

SimenB added 2 commits September 23, 2022 15:35

Merge branch 'main' into feature/with-implementation

552989c

use shared isPromise helper

51757a6

SimenB merged commit 35e8b6a into jestjs:main Sep 23, 2022

jeppester deleted the feature/with-implementation branch September 26, 2022 06:37

mrazauskas mentioned this pull request Sep 28, 2022

[jest] add types of jest.now and withImplementation DefinitelyTyped/DefinitelyTyped#62451

Merged

8 tasks

matejcik mentioned this pull request Oct 6, 2022

Bump jsdom and jest matejcik/incremental-beees#41

Merged

github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(@jest/mock): Add withImplementation #13281

feat(@jest/mock): Add withImplementation #13281

jeppester commented Sep 19, 2022

mrazauskas commented Sep 19, 2022

jeppester commented Sep 20, 2022

jeppester commented Sep 22, 2022

mrazauskas commented Sep 22, 2022

SimenB commented Sep 23, 2022

SimenB left a comment

SimenB Sep 23, 2022

jeppester Sep 23, 2022

SimenB Sep 23, 2022

SimenB Sep 23, 2022

SimenB Sep 23, 2022

mrazauskas Sep 23, 2022

jeppester Sep 23, 2022 •

edited

Loading

mrazauskas Sep 23, 2022

jeppester commented Sep 23, 2022

SimenB commented Sep 23, 2022

SimenB commented Sep 28, 2022

github-actions bot commented Oct 30, 2022

		typeof returnedValue === 'object' &&
		returnedValue !== null &&

feat(@jest/mock): Add withImplementation #13281

feat(@jest/mock): Add withImplementation #13281

Conversation

jeppester commented Sep 19, 2022

Summary

Test plan

mrazauskas commented Sep 19, 2022

jeppester commented Sep 20, 2022

jeppester commented Sep 22, 2022

mrazauskas commented Sep 22, 2022

SimenB commented Sep 23, 2022

SimenB left a comment

Choose a reason for hiding this comment

SimenB Sep 23, 2022

Choose a reason for hiding this comment

jeppester Sep 23, 2022

Choose a reason for hiding this comment

SimenB Sep 23, 2022

Choose a reason for hiding this comment

SimenB Sep 23, 2022

Choose a reason for hiding this comment

SimenB Sep 23, 2022

Choose a reason for hiding this comment

mrazauskas Sep 23, 2022

Choose a reason for hiding this comment

jeppester Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

mrazauskas Sep 23, 2022

Choose a reason for hiding this comment

jeppester commented Sep 23, 2022

SimenB commented Sep 23, 2022

SimenB commented Sep 28, 2022

github-actions bot commented Oct 30, 2022

jeppester Sep 23, 2022 •

edited

Loading