findBy* retry-ability when multiple elements found #83
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only relevant for the findBy* queries, due to it throwing an error if multiple elements are found.
This update changed the failure message for matching multiple elements. Unfortunately I can't see away around it if we are using retry-ability.
alexkrolick
reviewed
Sep 4, 2019
| // Only catch exceptions for multiple elements found, which we will continue to retry | ||
| if (/Found multiple elements/.test(err.message)) { | ||
| // Cannot delete after, because consoleProps() is called on the log object before I get the chance | ||
| consoleProps.CaughtError = err; |
There was a problem hiding this comment.
Could you (conditionally) clear the value in the try block?
alexkrolick
reviewed
Sep 4, 2019
| @@ -118,12 +123,12 @@ describe('find* dom-testing-library commands', () => { | |||
| }) | |||
|
|
|||
| it('findByText finding multiple items should error', () => { | |||
| const errorMessage = `Found multiple elements with the text: /^Button Text/i\n\n(If this is intentional, then use the \`*AllBy*\` variant of the query (like \`queryAllByText\`, \`getAllByText\`, or \`findAllByText\`)).` | |||
| const errorMessage = `Timed out retrying: Expected to find element: 'findByText(/^Button Text/i)', but never found it.` | |||
There was a problem hiding this comment.
Yeah this is kind of confusing... Let's hold while we figure it out
|
Hi folks 👋 @Jmaharman, do you still have plans to work on this or should we have someone else pick up where you left off? No pressure, just wondering how to proceed :) |
NicholasBoll
added a commit
to NicholasBoll/cypress-testing-library
that referenced
this pull request
Jan 30, 2020
* Add element selector infomation for debugging (outlines element when you click on command) (fixes testing-library#103) * Add @testing-library/dom errors (from `get*` queries) to failure messages - these are more helpful than the generic `find*('input') does not exist` messages (fixes testing-library#103) * Add retryability to `findBy*` when multiple elements are found (fixes testing-library#83) * Add option to disable logging of all commands * `query*` and `find*` have a consistent code path and error messaging (fixes testing-library#103) * Remove usage of Cypress commands in queries (fixes testing-library#103)
NicholasBoll
added a commit
to NicholasBoll/cypress-testing-library
that referenced
this pull request
Jan 30, 2020
* Add element selector information for debugging (outlines element when you click on command) (fixes testing-library#103) * Add @testing-library/dom errors (from `get*` queries) to failure messages - these are more helpful than the generic `find*('input') does not exist` messages (fixes testing-library#103) * Add retryability to `findBy*` when multiple elements are found (fixes testing-library#83) * Add option to disable logging of all commands * `query*` and `find*` have a consistent code path and error messaging (fixes testing-library#103) * Remove usage of Cypress commands in queries (fixes testing-library#103)
5 tasks
|
Sorry, I don't have the time to jump into it. If someone else wants to have
a run it at then they are more than welcome to.
…On Tue, 28 Jan 2020 at 18:15, Kent C. Dodds ***@***.***> wrote:
Hi folks 👋 @Jmaharman <https://github.com/Jmaharman>, do you still have
plans to work on this or should we have someone else pick up where you left
off? No pressure, just wondering how to proceed :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#83?email_source=notifications&email_token=AABUSP7ZGNT5PZRXGRRIONTRABY5TA5CNFSM4ITOQS22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKELGYI#issuecomment-579384161>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABUSP2SWAFO2O7NAJAXHKLRABY5TANCNFSM4ITOQS2Q>
.
|
|
No problem @Jmaharman, thanks anyway! I think this will actually be handled by #108. |
kentcdodds
pushed a commit
that referenced
this pull request
Feb 2, 2020
* feat: add more helpful debugging information to queries * Add element selector information for debugging (outlines element when you click on command) (fixes #103) * Add @testing-library/dom errors (from `get*` queries) to failure messages - these are more helpful than the generic `find*('input') does not exist` messages (fixes #103) * Add retryability to `findBy*` when multiple elements are found (fixes #83) * Add option to disable logging of all commands * `query*` and `find*` have a consistent code path and error messaging (fixes #103) * Remove usage of Cypress commands in queries (fixes #103) * feat: add ability for queries to inherit previous subject * Fixes #109 without breaking change caused by #100 * feat: add parent/child log type detection * chore: implement feedback * docs: update readme to complete my thought process * slightly simplify config fn * Update README.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> Closes #103, Closes #109, Closes #110
|
🎉 This issue has been resolved in version 5.2.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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
Add retry-ability to findBy* when multiple elements are matched.
Why:
findBy* queries would fail early if they match multiple elements, when it could automatically retry.
*How:
This catches the multiple matches exception, and retries until the request times out.
Checklist:
[ ] Documentation
[x] Tests
[x] Ready to be merged
Side Effects
Unfortunately this change comes with side effects, in that the error message when this scenario occurs says that the element cannot be found. See the last commit for the new error message being expected in the test.
I can't see a way around that for now, and as such I'm not sure if we should include this change.
I thought I'd commit this PR so that others could see it, and consider another way of doing it.