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(pretty-format): handles jsdom attributes properly #11189

Merged
merged 3 commits into from Mar 14, 2021

Conversation

pikou1995
Copy link
Contributor

@pikou1995 pikou1995 commented Mar 13, 2021

Summary

fixes #10770.

expect(received).toMatchObject(expected)

When received object mismatch with expected object, jest will copy received object withdeepCyclicCopyObject.

As for this issue, received object contains jsdom provided objects AbortController, and will encounter jsdom/lib/jsdom/living/attributes.

This module also contains hasAttribute, prettyFormat.plugins.DOMElement.test will throw an exception by calling hasAttribute.

Test plan

added handles jsdom attributes properly test.

@netlify
Copy link

netlify bot commented Mar 13, 2021

Deploy preview for jestjs ready!

Built without sensitive environment variables with commit d9eee82

https://deploy-preview-11189--jestjs.netlify.app

packages/pretty-format/src/plugins/DOMElement.ts Outdated Show resolved Hide resolved
@SimenB SimenB requested a review from jeysal March 13, 2021 12:39
Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #11189 (d9eee82) into master (f73d5f6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11189      +/-   ##
==========================================
- Coverage   64.14%   64.14%   -0.01%     
==========================================
  Files         307      307              
  Lines       13375    13379       +4     
  Branches     3260     3261       +1     
==========================================
+ Hits         8580     8582       +2     
- Misses       4089     4090       +1     
- Partials      706      707       +1     
Impacted Files Coverage Δ
packages/pretty-format/src/plugins/DOMElement.ts 97.22% <100.00%> (+0.34%) ⬆️
packages/expect/src/utils.ts 94.83% <0.00%> (-1.30%) ⬇️

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 f73d5f6...d9eee82. Read the comment docs.

Copy link
Contributor

@jeysal jeysal left a comment

Choose a reason for hiding this comment

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

I don't really understand how AbortController plays into encountering this and thus why the test added is exactly the way it is, but either way catch when calling unknown functions that could do anything is the right thing to do 👍

@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 10, 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.

Crash when pretty-printing AbortController / AbortSignal
5 participants