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

Output is not helpful when ByText query fails #1375

Closed
stevehanson opened this issue Mar 27, 2023 · 4 comments · Fixed by #1378
Closed

Output is not helpful when ByText query fails #1375

stevehanson opened this issue Mar 27, 2023 · 4 comments · Fixed by #1378

Comments

@stevehanson
Copy link
Contributor

Thank you for this excellent library! I'm a longtime user of RNTL, and something I have struggled with is that when a ByText query fails, the output does not help me determine the cause of the failure:

expect(screen.getByText(/HELLOOO/)).toBeDefined()

> Unable to find an element with text: /HELLOOO/

When I encounter a failure like the above, my usual next step is to insert a screen.debug() or some console.log() statements and rerun the test until I can understand the cause of the failure.

If the test uses the async findByText query, then adding these debug or log statements manually can be a bit more difficult, and I usually do something like this:

await sleep(100) // wait 100ms to simulate what the findBy normally does before debugging
screen.debug()
expect(away screen.findByText(/HELLOOO/)).toBeDefined() // the line that was erroring

Previous workaround (toHaveTextContent matcher)

I recently found a workaround that fit my needs, using the jest-native toHaveTextContent matcher:

expect(screen.container).toHaveTextContent(/HELLOOO/)

> Expected element to have text content:
>       /HELLOOO/
>     Received:
>       HELLOWelcome to the AppSign InForgot Password

As can be seen above, the toHaveTextContent matcher outputs all of the text from the screen along with the error message, which can be quite helpful for diagnosing why the test is failing.

This workaround however no longer seems like a safe approach since in the latest version of RNTL screen.container is discouraged and has been renamed to UNSAFE_root.

Additionally, using the toHaveTextContent matcher is non-standard and something we have to train teams to use instead of the ByText queries. Ideally, we could solve this problem with the ByText queries themselves

Comparing to React Testing Library (web)

As can be seen in this Code Sandbox, React Testing Library helpfully outputs the full DOM when a byText query fails:

Unable to find an element with the text: /HELLOO/i. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

<body>
  <div>
    <h1>
      Hello 
      Jill
      !
    </h1>
  </div>
</body>

Proposal

A more helpful error message when a byText query fails would dramatically improve the testing experience with React Native. Ideally, we could achieve something like the error message that is shown on the web.

I haven't contributed to this project before, but I'm a longtime user and would be happy to help out with a contribution if maintainers agree that this would be valuable.

@pierrezimmermannbam
Copy link
Collaborator

Hi @stevehanson ! This looks like a very good idea! It is indeed quite difficult to see the state of the component when the query fails using findBy right now so I think this would be a good addition. Doing something similar to React Testing Library looks feasible, we could update the getMissingError from text queries and use format with mapProps option to log the elements without props.

A simplified version would look like this

const getMissingError = (text: TextMatch) =>
  `Unable to find an element with text: ${String(text)}
  
  ${format(screen.toJSON() || [], { mapProps: () => ({}) })}`;

This should be a good place to start and if other maintainers support this proposal I'd be more than happy if you opened a pr for it

@MattAgn @mdjastrzebski @AugustinLF what do you think about this proposal?

@AugustinLF
Copy link
Collaborator

We've had some discussions around that. We agree that it's definitely something that should be improved. I know that @mdjastrzebski had a PR to react-test-renderer to improve the output of rendered elements. For a base root screen log, I think it's fine, but we'll need it if you want better debugging help, like listing the components with the right role but wrong name.

@pierrezimmermannbam
Copy link
Collaborator

You mean this pr? to have toJSON method not only on root? Yes it would allow to do some cool stuff. In the meantime we could print the root with only relevant props to the query so only children for text queries and children, accessibilityRole & accessibilityLabel & accessibilityState for role queries and so on

@AugustinLF
Copy link
Collaborator

Yep exactly, we could also consider using the default user's debug option if they set it up globally.

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

Successfully merging a pull request may close this issue.

3 participants