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

Factor out common code for collections in pretty-format #4184

Merged
merged 3 commits into from
Aug 2, 2017

Conversation

pedrottimark
Copy link
Contributor

Summary

From lemon to lemonade:

  1. confused assumption:
    • that maxDepth option didn’t work for asymmetric matchers
    • didn’t read carefully to see that they delegate array or object to core which handles depth
  2. embarrassing error in Replace print with serialize in AsymmetricMatcher plugin #4173:
    • incorrectly edited new tests to fail when they should have passed
    • added code to increment depth twice at a level to make the tests pass when it should fail
  3. delayed discovery that in new tests, my careful comments contradicted the maxDepth option
  4. course correction:
    • instead of constructing new or ArrayContaining or ObjectContaining instances
    • factor out 4 helper functions out so built-in plugins can call them (this PR calls 2 and the next PR for Immutable plugins will call the other 2)
    • add printer callback function as argument
    • also repent of decision to separate printProp from printProps
  5. API adjustment:
    • add config to arguments so printer callback function doesn’t need a closure
    • move printer to last argument of serialize method

Test plan

Update 2 tests with options.maxDepth = 2 so that depth of 3 is greater, duh

@pedrottimark
Copy link
Contributor Author

The basic test is a control for precision. Factoring doesn’t have a significant effect on run time.

type plugins 4184 factored 4114 unfactored ratio
basic 0 5715 5736 0.996
complex 0 232547 236053 0.985
react_test_component 9 692198 708478 0.977
react_element 9 802735 818914 0.980

@codecov-io
Copy link

Codecov Report

Merging #4184 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4184      +/-   ##
==========================================
+ Coverage   60.26%   60.38%   +0.12%     
==========================================
  Files         197      198       +1     
  Lines        6787     6748      -39     
  Branches        6        6              
==========================================
- Hits         4090     4075      -15     
+ Misses       2694     2670      -24     
  Partials        3        3
Impacted Files Coverage Δ
...ackages/pretty-format/src/plugins/react_element.js 100% <ø> (ø) ⬆️
.../pretty-format/src/plugins/react_test_component.js 100% <ø> (ø) ⬆️
packages/pretty-format/src/plugins/lib/markup.js 100% <100%> (ø) ⬆️
packages/pretty-format/src/index.js 97.7% <100%> (-0.78%) ⬇️
packages/pretty-format/src/collections.js 100% <100%> (ø)
...es/pretty-format/src/plugins/asymmetric_matcher.js 100% <100%> (+10%) ⬆️
packages/jest-mock/src/index.js 92.1% <0%> (+7.43%) ⬆️

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 99d9cff...09c312f. Read the comment docs.

@cpojer cpojer merged commit 4f5ccd3 into jestjs:master Aug 2, 2017
@pedrottimark pedrottimark deleted the pretty-format-collections branch August 3, 2017 13:51
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Factor out common code for collections in pretty-format

* Fix pretty lint error

* Improve comments and put printObjectProperties back together
@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 13, 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.

4 participants