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 documentation related to auto-mocking #8099

Merged
merged 14 commits into from Mar 18, 2019

Conversation

@mackness
Copy link
Contributor

mackness commented Mar 10, 2019

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 #6812

Test plan

Add a few tests to verify the assertions made by the documentation in packages/jest-mock/src/__tests__/index.test.ts.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 10, 2019

Codecov Report

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

Impacted file tree graph

@@           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.

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

docs/JestObjectAPI.md Outdated Show resolved Hide resolved

#### `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`.

This comment has been minimized.

Copy link
@SimenB

SimenB Mar 10, 2019

Collaborator

should mocked async functions return Promise<undefined> instead?

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

I just tested this and it seems like mocked async functions don't return Promise<undefined> instead they return undefined just like synchronous functions.

This comment has been minimized.

Copy link
@SimenB

SimenB Mar 12, 2019

Collaborator

yep, I meant that maybe we should change this. Good that the current behavior gets documented though!


Example:

<!-- prettier-ignore -->

This comment has been minimized.

Copy link
@SimenB

SimenB Mar 10, 2019

Collaborator

?

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

So for some reason prettier is adding these unwanted parens after the closing class bracket

image

This comment has been minimized.

Copy link
@mackness

mackness Mar 12, 2019

Author Contributor

If I remove js in the opening code block in the markdown file prettier does not insert the weird parenthesis. 🤔

image

This comment has been minimized.

Copy link
@SimenB

SimenB Mar 12, 2019

Collaborator

Oh, it's just adding the parense. new class Bar{} and new class Bar{}() behaves the same.

Should it be newed?

I opened up prettier/prettier#5964, not sure if it's a bug or not

This comment has been minimized.

Copy link
@mackness

mackness Mar 12, 2019

Author Contributor

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

Copy link
Collaborator

SimenB left a comment

This is awesome! I especially like the added tests to assert behavior for the different types. Thanks for those.

I left some inline q's.

Can you also update the versioned docs and the changelog?

image

Copy link
Member

rickhanlonii left a comment

Thanks for doing this!


#### `String`

A new copy of the original stirng is mocked.

This comment has been minimized.

Copy link
@rickhanlonii

rickhanlonii Mar 11, 2019

Member
Suggested change
A new copy of the original stirng is mocked.
A new copy of the original string is mocked.

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

Thanks for catching this!


#### `Object`

Objects are deeply cloned. Their interfaces are maintained and their values are mocked.

This comment has been minimized.

Copy link
@rickhanlonii

rickhanlonii Mar 11, 2019

Member
Suggested change
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.

#### `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.

This comment has been minimized.

Copy link
@rickhanlonii

rickhanlonii Mar 11, 2019

Member

What about class properties, if there's a property that is an array will it be mocked with the array strategy mentioned below?

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

Good point, I can add this case to the example code / description.

docs/JestObjectAPI.md Show resolved Hide resolved
docs/JestObjectAPI.md Outdated Show resolved Hide resolved
Mack Solomon

#### `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`.

This comment has been minimized.

Copy link
@pedrottimark

pedrottimark Mar 11, 2019

Collaborator

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.

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

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.


#### `String`

A new copy of the original string is mocked.

This comment has been minimized.

Copy link
@pedrottimark

pedrottimark Mar 11, 2019

Collaborator

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?

This comment has been minimized.

Copy link
@mackness

mackness Mar 11, 2019

Author Contributor

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:

expect(metadata.value).toEqual(Symbol.for('bowties.are.cool'));
expect(moduleMocker.getMetadata('banana').value).toEqual('banana');
expect(moduleMocker.getMetadata(27).value).toEqual(27);
expect(moduleMocker.getMetadata(false).value).toEqual(false);

docs/JestObjectAPI.md Outdated Show resolved Hide resolved
docs/JestObjectAPI.md Outdated Show resolved Hide resolved
Copy link
Collaborator

pedrottimark left a comment

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.

@SimenB
SimenB approved these changes Mar 17, 2019
Copy link
Collaborator

SimenB left a comment

This is fantastic, thanks!

@@ -13,6 +13,7 @@

### Chore & Maintenance

- `[*]` Add documentation and tests related to auto-mocking ([#8086](https://github.com/facebook/jest/pull/8099))

This comment has been minimized.

Copy link
@SimenB

SimenB Mar 17, 2019

Collaborator

Can you move this up under master?

(Might have to merge in master or rebase first)

This comment has been minimized.

Copy link
@mackness

mackness Mar 18, 2019

Author Contributor

Synced with upstream master and moved the changelog line under the master heading. Thanks for the feedback and help with this @SimenB !

@SimenB SimenB merged commit 90c6002 into facebook:master Mar 18, 2019
12 checks passed
12 checks passed
ci/circleci: lint-and-typecheck Your tests passed on CircleCI!
Details
ci/circleci: test-browser Your tests passed on CircleCI!
Details
ci/circleci: test-jest-circus Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
ci/circleci: test-node-6 Your tests passed on CircleCI!
Details
ci/circleci: test-node-8 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
facebook.jest #20190317.12 succeeded
Details
@SimenB

This comment has been minimized.

Copy link
Collaborator

SimenB commented Mar 18, 2019

Thank you!

thymikee added a commit to Brantron/jest that referenced this pull request Mar 19, 2019
* upstream/master: (391 commits)
  more precise circus asyncError types (facebook#8150)
  Add typeahead watch plugin (facebook#6449)
  fix: getTimerCount not taking immediates and ticks into account (facebook#8139)
  website: add an additional filter predicate to backers (facebook#7286)
  [🔥] Revised README (facebook#8076)
  [jest-each] Fix test function type (facebook#8145)
  chore: improve bug template labels for easier maintenance (facebook#8141)
  Add documentation related to auto-mocking (facebook#8099)
  Add support for bigint to pretty-format (facebook#8138)
  Revert "Add fuzzing based tests in Jest (facebook#8012)"
  chore: remove console.log
  chore: Improve description of optional arguments in ExpectAPI.md (facebook#8126)
  Add fuzzing based tests in Jest (facebook#8012)
  Move @types/node to the root package.json (facebook#8129)
  chore: use property initializer syntax (facebook#8117)
  chore: delete flow types from the repo (facebook#8061)
  Move changelog entry to the proper version (facebook#8115)
  Release 24.5.0
  Expose throwOnModuleCollision (facebook#8113)
  add matchers to expect type (facebook#8093)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.