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

chore: Remove repetition of matcherName and options in matchers #8224

Merged

Conversation

pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Mar 26, 2019

Summary

Follow example of snapshot matchers in #8132

This pattern reduces chance of mistakes in remaining improvements to matchers

For example, when in this pull request I cut one of the existing literal objects for options in toBe and forgot to replace isNot: true with isNot: this.isNot until it broke some e2e tests

Also:

  • replaced diffString with difference because it isn’t always a string
  • replaced null with '' for secondArgument in toHaveProperty and removed 3 occurrences as MatcherHintOptions
  • un-factored-out matcherHint call in toStrictEqual to call function only when test fails

Test plan

Updated snapshots in matchers.test.ts for not in regular black instead of dim:

  • 23 for .not.toHaveProperty
  • 22 for .not.toMatchObject

The matcher name in hint remains dim as clue that their reports are not yet improved.

Removed 'proxies matchers to expect' test according to #7970 (comment)

  • packages/jest-jasmine2/src/__tests__/matchers.test.ts
  • packages/jest-jasmine2/src/__tests__/__snapshots__/matchers.test.ts.snap

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

I like it!

@codecov-io
Copy link

codecov-io commented Mar 26, 2019

Codecov Report

Merging #8224 into master will increase coverage by 0.08%.
The diff coverage is 96.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8224      +/-   ##
==========================================
+ Coverage   62.29%   62.38%   +0.08%     
==========================================
  Files         265      265              
  Lines       10551    10576      +25     
  Branches     2563     2564       +1     
==========================================
+ Hits         6573     6598      +25     
  Misses       3388     3388              
  Partials      590      590
Impacted Files Coverage Δ
packages/expect/src/matchers.ts 96.02% <96.96%> (+0.39%) ⬆️

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 4b3483e...95a1e57. Read the comment docs.

packages/expect/src/matchers.ts Show resolved Hide resolved
@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