testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.27k stars 466 forks source link

Sub-optimal error message when using text matcher functions #732

Open murphyke opened 4 years ago

murphyke commented 4 years ago

When using a text matcher function with, e.g. screen.getByText, the TestingLibraryElementError message is not accurate and possibly not helpful, in that it says that the source code of the function is the "text" (pattern) being matched against.

I realize that this is low priority, because a) in some cases the matcher function will contain hard-coded comparison parameters, and b) if the comparison parameters happen to be hidden in outer-scope variables, the user can just look up the test case to see what the actual pattern or match parameters were.

The "not accurate" part would be relatively easy to fix in that the library could detect when the matcher is a function and use a more appropriate error message. (Aside: maybe it would be helpful if the current error message contained a link to a discussion of how to write a matcher function to match across elements).

The "possibly not helpful" part is true if the function doesn't contain literal comparison parameters (pattern, etc), so that the function source code doesn't reveal the pattern(s) used. The documentation could mention the possibility of using a custom toString method on such custom matcher functions.

Versions:

Relevant code or config:

function getByTextAcrossElements(stringOrRegexp) {
  return screen.getByText((content, element) => {
    // ...
  });
}

What you did:

expect(
  getByTextAcrossElements(
    new RegExp(`ICD-10 Diagnosis.*${finalExpected}`, 'i')
  )
).toBeTruthy();

What happened:

TestingLibraryElementError: Unable to find an element with the text: (content, element) => {
  ...
  }. 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.

Suggested solution

I discussed this above. In the case of my example above, using a custom toString might look like this:

export function getByTextAcrossElements(stringOrRegexp) {
  function _getByTextAcrossElements(content, element) {
    // ...
  };
  _getByTextAcrossElements.toString = () => `_getByTextAcrossElements, using pattern \`${stringOrRegexp}\``;
  return screen.getByText(_getByTextAcrossElements);
}
kentcdodds commented 4 years ago

Did you try your .toString solution? I'm pretty sure that will actually work without any changes :)

murphyke commented 4 years ago

Did you try your .toString solution? I'm pretty sure that will actually work without any changes :)

Yeah, it works fine and is a super easy way for end users to improve the situation immediately. The only "solution" on the lib's part would be finessing the error message if a function is being used and possibly adding a brief note to the docs where it discusses matcher functions.

kentcdodds commented 4 years ago

I think a note in the docs would make sense. Would you be willing to add a PR for that?

murphyke commented 4 years ago

I think a note in the docs would make sense. Would you be willing to add a PR for that?

Sure.

toneo- commented 4 years ago

I've just ran into this same issue! The error message is very confusing, I was starting to think (reluctantly) that the library was doing a search using the source code as text.

Could we have this adjusted for cases where a function is provided instead of a text string? e.g. "Unable to find an element using function matcher: \<source code>"

nickserv commented 3 years ago

I think it may look nicer to truncate the message to Unable to find an element with the text. Jest should show the relevant code with the stack trace, right?

toneo- commented 3 years ago

That makes sense, no need to show the source code if Jest does it already!

Ideally it shouldn't mention text at all where a function matcher is provided, since it leads the debugging dev down the wrong path. Unable to find an element using the provided function matcher. is more accurate. Wording can always be adjusted to taste as long as it doesn't imply that the source code is the text being matched against

itsik-avidan commented 3 years ago

Hi, I've suggested a PR for this issue