-
Notifications
You must be signed in to change notification settings - Fork 470
Add a debug method to screen. #429
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
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d64f32b:
|
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 22 22
Lines 374 377 +3
Branches 87 88 +1
=====================================
+ Hits 374 377 +3
Continue to review full report at Codecov.
|
I am not familiar with adding types, could I get some direction on how to do that? Thank you, I am thinking of something like this
What could I do to add the debug type? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Just a few little things in the tests. Thanks!
src/__tests__/screen.js
Outdated
renderIntoDocument( | ||
`<button>test</button><span>multi-test</span><div>multi-test</div>`, | ||
) | ||
expect(screen.debug).toBeInstanceOf(Function) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we don't need this assertion because we make it implicitly by calling it. Implicit assertions aren't always the best, but when it's something as simple as what type something is I don't think it's all that useful to have.
</div> | ||
</body>" | ||
`) | ||
// log single element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a console.log.mockClear()
between these so that each group of assertions can be more independent from the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does make it a-lot more clear. Cool, I appreciate this suggestion.
@@ -7,14 +7,19 @@ import * as defaultQueries from './queries' | |||
/** | |||
* @param {HTMLElement} element container | |||
* @param {FuncMap} queries object of functions | |||
* @param {Object} initialValue for reducer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting idea. This probably isn't how I would have done it, but I'm actually pretty cool with it.
@kentcdodds suggestions for the tests were added. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it. Thank you!
Thank you! I appreciate it! |
🎉 This PR is included in version 6.12.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
A debug method is being added to the screen object.
Why:
Facilitate debugging tests without having to always modify imports.
How:
Added the debug method to the screen object, following the pattern for the debug method from render in react-testing-library.
Checklist:
docs site
DefinitelyTyped
Would close
#417
Docs PR:
testing-library/testing-library-docs#362