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

feat(waitFor): add onTimeout which adds DOM output to timeout errors #671

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

kentcdodds
Copy link
Member

@kentcdodds kentcdodds commented Jun 24, 2020

What: add onTimeout which adds DOM output to timeout errors

Why: Closes #559 (nicer error messages)

How: add the parameter and have it use prettyDOM to attach the nice message to the error

Checklist:

@codesandbox-ci
Copy link

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 2e12261:

Sandbox Source
lively-dust-v8h0x Configuration

@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #671 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #671   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          593       595    +2     
  Branches       148       148           
=========================================
+ Hits           593       595    +2     
Impacted Files Coverage Δ
src/wait-for.js 100.00% <100.00%> (ø)

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 86205e5...2e12261. Read the comment docs.

@@ -112,7 +112,7 @@ describe('asynchronous queries throw on invalid container type', () => {
['findAllByTestId', findAllByTestId],
])('%s', (_queryName, query) => {
const queryOptions = {}
const waitOptions = {timeout: 1}
const waitOptions = {timeout: 1, onTimeout: e => e}
Copy link
Member Author

Choose a reason for hiding this comment

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

I added onTimeout: e => e to several places in the tests because otherwise the snapshots got formatted really weird and would fail due to formatting issues 🤷‍♂️

So I added a special test to handle this case specifically and the rest just do nothing to change the error.

@@ -25,6 +26,10 @@ function waitFor(
showOriginalStackTrace = getConfig().showOriginalStackTrace,
stackTraceError,
interval = 50,
onTimeout = error => {
error.message = `${error.message}\n\n${prettyDOM(container)}`
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason we do this rather than simply calling screen.debug() here:

  1. It could be that the container isn't bound to the same document as screen, so we should use the container given
  2. If we do screen.debug() then the DOM output will appear above the error which I think would be confusing.

@kentcdodds kentcdodds merged commit 6bc8e2e into master Jun 24, 2020
@kentcdodds kentcdodds deleted the pr/print-screen-in-wait-for branch June 24, 2020 21:13
@kentcdodds
Copy link
Member Author

🎉 This PR is included in version 7.18.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

automatically call screen.debug() if there's a timeout in waitFor
1 participant