-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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 documentation related to auto-mocking #8099
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8099 +/- ##
=======================================
Coverage 62.44% 62.44%
=======================================
Files 264 264
Lines 10371 10371
Branches 2515 2515
=======================================
Hits 6476 6476
Misses 3318 3318
Partials 577 577 Continue to review full report at Codecov.
|
docs/JestObjectAPI.md
Outdated
|
||
#### `Function` | ||
|
||
A new function will be created. The new function will have no formal parameters and when called will return `undefined`. This functionality also applies to `async functions`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should mocked async
functions return Promise<undefined>
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested this and it seems like mocked async functions don't return Promise<undefined>
instead they return undefined
just like synchronous functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, I meant that maybe we should change this. Good that the current behavior gets documented though!
docs/JestObjectAPI.md
Outdated
|
||
Example: | ||
|
||
<!-- prettier-ignore --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it's just adding the parense. new class Bar{}
and new class Bar{}()
behaves the same.
Should it be new
ed?
I opened up prettier/prettier#5964, not sure if it's a bug or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I added another comment that may or may not be related to the same issue. For now I just removed the js
attribute from the markdown code block and everything is formatted correctly and syntax hi-lighting seems to be working in the site preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
docs/JestObjectAPI.md
Outdated
|
||
#### `String` | ||
|
||
A new copy of the original stirng is mocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new copy of the original stirng is mocked. | |
A new copy of the original string is mocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this!
docs/JestObjectAPI.md
Outdated
|
||
#### `Object` | ||
|
||
Objects are deeply cloned. Their interfaces are maintained and their values are mocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Objects are deeply cloned. Their interfaces are maintained and their values are mocked. | |
Objects are deeply cloned. Their keys are maintained and their values are mocked. |
docs/JestObjectAPI.md
Outdated
|
||
#### `Class` | ||
|
||
A new class will be created. The interface of the original class is maintained however all of the class member functions will be mocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about class properties, if there's a property that is an array will it be mocked with the array strategy mentioned below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I can add this case to the example code / description.
docs/JestObjectAPI.md
Outdated
|
||
#### `Function` | ||
|
||
A new [mock function](https://jestjs.io/docs/en/mock-functions.html) will be created. The new function will have no formal parameters and when called will return `undefined`. This functionality also applies to `async functions`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it harm the meaning to write in present tense and active voice as much as possible? For example:
Create a new mock function which has no formal parameters and returns undefined
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right about using present tense. I also don't think present tense effects the meaning or makes it unclear. Since the rest of the docs are written in present tense I think I should update this section to match. I'll work on switching everything to present tense.
docs/JestObjectAPI.md
Outdated
|
||
#### `String` | ||
|
||
A new copy of the original string is mocked. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This made me stop to think (which is not necessarily bad :)
By a new copy of primitive type like string or number, is the implicit assumption as value of a property?
If yes, then is it possible to summarize the behavior, which I think confirms what testers expect as one heading for all primitive types (with a few additional tests, if needed) including boolean, symbol, null, undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to summarize all primitives under one heading since the mocking functionality is the same. Instead of a string and number header I can create a primitives header and maybe add the following tests to the docs: https://github.com/facebook/jest/blob/445e6cb9f5fddf87174e510a602cf8bc11a840b1/packages/jest-mock/src/__tests__/index.test.ts#L33-L36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution to fill this gap and willingness to follow through on review comments.
I will leave for Simen to confirm the changes that he requested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic, thanks!
CHANGELOG.md
Outdated
@@ -13,6 +13,7 @@ | |||
|
|||
### Chore & Maintenance | |||
|
|||
- `[*]` Add documentation and tests related to auto-mocking ([#8086](https://github.com/facebook/jest/pull/8099)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this up under master?
(Might have to merge in master or rebase first)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Synced with upstream master and moved the changelog line under the master heading. Thanks for the feedback and help with this @SimenB !
Thank you! |
* upstream/master: (391 commits) more precise circus asyncError types (jestjs#8150) Add typeahead watch plugin (jestjs#6449) fix: getTimerCount not taking immediates and ticks into account (jestjs#8139) website: add an additional filter predicate to backers (jestjs#7286) [🔥] Revised README (jestjs#8076) [jest-each] Fix test function type (jestjs#8145) chore: improve bug template labels for easier maintenance (jestjs#8141) Add documentation related to auto-mocking (jestjs#8099) Add support for bigint to pretty-format (jestjs#8138) Revert "Add fuzzing based tests in Jest (jestjs#8012)" chore: remove console.log chore: Improve description of optional arguments in ExpectAPI.md (jestjs#8126) Add fuzzing based tests in Jest (jestjs#8012) Move @types/node to the root package.json (jestjs#8129) chore: use property initializer syntax (jestjs#8117) chore: delete flow types from the repo (jestjs#8061) Move changelog entry to the proper version (jestjs#8115) Release 24.5.0 Expose throwOnModuleCollision (jestjs#8113) add matchers to expect type (jestjs#8093) ...
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. |
Summary
The purpose of this PR is to add more documentation related to how Jest auto-mocks modules. It aims to clarify how Jest mocks and transforms values with
jest.genMockFromModule
. Resolves #6812Test plan
Add a few tests to verify the assertions made by the documentation in
packages/jest-mock/src/__tests__/index.test.ts
.