Throw better error message by findByRole query

[vinaymahamuni]

Describe the feature you'd like:

Currently if findByRole query doesn't find the element for given role and name, it throws error with message TestingLibraryElementError: Unable to find role="button"

Test to verify above behavior

it("Doesn't tell where it failed", async () => {
  render(<button>Click me</button>)
  
  await screen.findByRole('button', { name: 'submit' })
  await screen.findByRole('button', { name: 'Click me' })
})

In above test, I don't get feedback for which name the findByRole query failed.

For same scenario, if getByRole query is used, it gives the name as well
TestingLibraryElementError: Unable to find an accessible element with the role "button" and name "submit"

This helps in debugging. Specially when it fails on CI.

Suggested implementation:

I looked the code. This behavior happen for all the queries which internally use runWithExpensiveErrorDiagnosticsDisabled function to run callback. In src/queries/role.js there is getMissingError function. In that there is one condition applied which tries to avoid expensive error diagnostics. This condition applies to findByRole function as well. I agree with decision. But I think that adding only namehint to the error string won't hit the performance.

Describe alternatives you've considered:

Currently, I try to figure out where error might have occur using line number printed in stacktrace. But that is very difficult as I am using typescript + snowpack+ web test runner which runs test on browser and I don't get easy access to generated js files

Teachability, Documentation, Adoption, Migration Strategy:

we can easily extract out function returning namehint and reuse it in first if condition

const getNameHint = (name) => {
  let nameHint = ''
  if (name === undefined) {
    nameHint = ''
  } else if (typeof name === 'string') {
    nameHint = ` and name "${name}"`
  } else {
    nameHint = ` and name \`${name}\``
  }
  return nameHint
}

const getMissingError = (
  container,
  role,
  {hidden = getConfig().defaultHidden, name} = {},
) => {
  if (getConfig()._disableExpensiveErrorDiagnostics) {
    return `Unable to find role="${role}${getNameHint(name)}"`
  }
  ...
  ...
  ...
  ...
  ...
  ...
  ...
}

[MatanBobi]

Hi @vinaymahamuni, thanks for opening this one!
I think the reason this is happening is because findBy* functions are using waitFor under the hood to wait for the result.
waitFor runs the query with a flag to get a minimal error.
I'm not too familiar with why are we doing that just for waitFor. Maybe @eps1lon will know?

[eps1lon]

@MatanBobi The why is not questioned by the issue. For its rationale, check the git-blame: #590

As to the issue about the missing name hint: I'm all for it. Thanks for catching this 👍🏻

[MatanBobi]

@MatanBobi The why is not questioned by the issue. For its rationale, check the git-blame: #590

@eps1lon I wasn't questioning the why, just wanted to understand the use case to see that a fix for this one won't cause a regression, sorry if it seemed like I was questioning it.

Regarding this issue, I'm up for doing it but I'm not sure how we can without running the expensive part of understanding the correct error message. Any ideas?

[eps1lon]

Regarding this issue, I'm up for doing it but I'm not sure how we can without running the expensive part of understanding the correct error message. Any ideas?

The issuer just asked for adding the expected name. That seems fine to me and doesn't require any expensive diagnostics as far as I can tell.

sorry if it seemed like I was questioning it.

No need to appologize.