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

compare objects with Symbol keys #3437

Merged
merged 2 commits into from
Jun 5, 2017

Conversation

cpenarrieta
Copy link
Contributor

Summary
Fixes #3347
I'm using Object.getOwnPropertySymbols to get the Symbol keys

Test plan
yarn test

@codecov-io
Copy link

Codecov Report

Merging #3437 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3437      +/-   ##
==========================================
- Coverage   64.02%   63.99%   -0.04%     
==========================================
  Files         177      177              
  Lines        6580     6574       -6     
  Branches        5        5              
==========================================
- Hits         4213     4207       -6     
  Misses       2365     2365              
  Partials        2        2
Impacted Files Coverage Δ
packages/jest-matchers/src/jasmine-utils.js 78.89% <100%> (ø) ⬆️
packages/jest-config/src/normalize.js 89.54% <0%> (-0.4%) ⬇️
packages/jest-config/src/index.js 26.08% <0%> (ø) ⬆️

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 62d29f2...878d017. Read the comment docs.

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.

I think this is good to go, but we need to refactor these jasmine utils later. There's a lot of code we can remove.

@cpenarrieta
Copy link
Contributor Author

@thymikee any update with this PR? do you prefer to refactor the jasmine utils first? I so, I can help with some guidance on what needs to be refactored.
Thanks

@thymikee
Copy link
Collaborator

thymikee commented Jun 5, 2017

Jasmine is already being rewritten into something better by @DmitriiAbramov so no need for that :). Utils will probably stay the same, but refactor can be postponed. Just wait for him or @cpojer to get their eyes on this.

@cpojer cpojer merged commit b69406b into jestjs:master Jun 5, 2017
@cpenarrieta cpenarrieta deleted the issue-3347-symbols-ib-objects branch June 5, 2017 16:01
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* compare objects with Symbol keys

* Update symbol-in-objects-test.js
@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.

Symbols in objects and toEqual
5 participants