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

Fix inconsistent name of complex values in pretty-format #4001

Merged
merged 4 commits into from Jul 20, 2017

Conversation

pedrottimark
Copy link
Contributor

Summary

Give the return statements in printComplexValue a parallel structure.

  • For min: true option, the previous formatted value Array [] instead of [] for empty array (or arguments) was inconsistent with {} for empty object. The change doesn’t affect snapshots directly, but it does affect format of Expected/Received values in Jest output when a test fails.
  • When hitMaxDepth is true, the change to return name of typed array or constructed object instead of generic [Array] or [Object] doesn’t affect snapshots because default maxDepth is Infinity but does affect Jest output when a test fails, because maxDepth = 10 is default.

Test plan

Updated tests for min: true option in jest-matchers and jest-matcher-utils

@pedrottimark
Copy link
Contributor Author

This fails when I run tests locally after yarn clean-all && yarn but I didn’t dare to update it:

 FAIL  integration_tests/__tests__/show_config.test.js
  ● jest --showConfig › outputs config info and exits

-        "/mocked/root/path/jest/node_modules/babel-jest/build/index.js"
+        "/mocked/root/path/jest/integration_tests/verbose_reporter/node_modules/babel-jest/build/index.js"

@thymikee
Copy link
Collaborator

Love what you're doing to make pretty-format code better and easier to approach ❤️

@pedrottimark
Copy link
Contributor Author

Just remembered what is the breaking change for snapshots: an empty typed array becomes Unit32Array [] instead of Array []

@cpojer
Copy link
Member

cpojer commented Jul 10, 2017

Seems like Appveyor is unhappy about some snapshots?

@pedrottimark
Copy link
Contributor Author

Yes, I think about 15 added/removed because of Array [] to [] in their names.

@pedrottimark
Copy link
Contributor Author

pedrottimark commented Jul 10, 2017

I will look at Jest code to see if an unintended side effect of --ci option is default unminified instead of min: true option for Expected/Received values? EDIT: Does not seem relevant.

@pedrottimark
Copy link
Contributor Author

Could it be caching, because no source files change in jest-matchers or jest-matcher-utils

  • matchers.test.js.snap and spy_matchers.test.js.snap changed but not the corresponding .test.js files, because they call stringify in template literals for the snapshot tests than changed.
  • More surprising to me is index.test.js in jest-matcher-utils because I did edit the test file.

@cpojer cpojer merged commit e1f7167 into jestjs:master Jul 20, 2017
@pedrottimark pedrottimark deleted the fix-complex-name branch July 20, 2017 15:05
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Fix inconsistent name of complex values in pretty-format

* Fix accidental lint errors when editing test in  jest-matcher-utils

* Add test case for empty typed array

* Strengthened tests for maxDepth and min options
@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.

None yet

4 participants