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

docs: remove expect.assertions(1) in rejects example of TutorialAsync.md #9149

Merged
merged 6 commits into from
Nov 20, 2019
Merged

docs: remove expect.assertions(1) in rejects example of TutorialAsync.md #9149

merged 6 commits into from
Nov 20, 2019

Conversation

Jongmun-Park
Copy link
Contributor

Hi.
a few days ago, i registered issue #9143
please, read that issue.
i suggest how about removing expect.assertions(1); code in rejects example of TutorialAsync.md
becauese i was not sure whether I should used assertions.

thank u.

@facebook-github-bot
Copy link
Contributor

Hi Jongmun-Park! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@codecov-io
Copy link

codecov-io commented Nov 8, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9149   +/-   ##
=======================================
  Coverage   64.77%   64.77%           
=======================================
  Files         277      277           
  Lines       11724    11724           
  Branches     2879     2878    -1     
=======================================
  Hits         7594     7594           
  Misses       3512     3512           
  Partials      618      618

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 bbfef53...6a1a958. Read the comment docs.

@jeysal
Copy link
Contributor

jeysal commented Nov 10, 2019

Hi, thanks for the PR! We had another one doing this where we suggested adding a clarifying comment instead of removing the line, if you want to read through that and maybe change this PR to do it that way that'd be great :)

@Jongmun-Park
Copy link
Contributor Author

Thanks @jeysal
i updated the docs as you recommended like this.
expect.assertions(number) is not required but recommended to verify that a certain number of assertions are called during a test.
how about this?

@jeysal
Copy link
Contributor

jeysal commented Nov 13, 2019

Yes, perhaps explaining more in detail that it is otherwise easy to forget to await a .resolves assertion.

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@thymikee thymikee merged commit ee66670 into jestjs:master Nov 20, 2019
ayush000 added a commit to ayush000/jest that referenced this pull request Nov 21, 2019
* master:
  chore: upgrade to fsevents 2 (jestjs#9215)
  docs: remove expect.assertions(1) in rejects example of Tutoria… (jestjs#9149)
  chore: bump to istanbul alphas (jestjs#9192)
  Fix typo in JestPlatform.md (jestjs#9212)
  jest-snapshot: Ignore indentation for most serialized objects (jestjs#9203)
  fix(jest-types): tighten Config types and set more defaults (jestjs#9200)
  jest-snapshot: Improve colors when snapshots are updatable (jestjs#9132)
  jest-snapshot: Omit irrelevant received properties when property matchers fail (jestjs#9198)
  chore: make changedFiles option optional in `shouldInstrument` (jestjs#9197)
  fix(pretty-format): correctly detect memo (jestjs#9196)
  chore: regenerate lockfiles in e2e tests (jestjs#9193)
  chore: bump handlebars
@Jongmun-Park Jongmun-Park deleted the fix/TutorialAsync.md branch November 22, 2019 01:08
@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 11, 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.

5 participants