callstack / react-native-testing-library

🦉 Simple and complete React Native testing utilities that encourage good testing practices.
https://callstack.github.io/react-native-testing-library/
MIT License
3.01k stars 264 forks source link

toHaveTextContent matching rules are not aligned with jest-dom #1613

Open AugustinLF opened 1 month ago

AugustinLF commented 1 month ago

Hey hey, long time not posted here^^'

Describe the bug

toHaveTextContent() does exact matching by default, while the same helper exposed by jest-dom does partial matching.

test('toHaveTextContent() does exact match by default', () => {
  render(<Text testID="text">Hello World</Text>);

  const text = screen.getByTestId('text');
  expect(text).toHaveTextContent('Hello World');
  // this will error with jest-dom when using RTL.
  expect(text).not.toHaveTextContent('Hello');
  expect(text).not.toHaveTextContent('World');
});

Expected behavior

I would expect both APIs to behave similarly. Furthermore, the jest-dom matcher doesn't support the exact option, which makes it a bit hard to run both web and native tests with the same code.

I'll happily open a conversion in jest-dom's repo, but we should be sure of what we want before hand. Have you had discussions with them on this topic?

mdjastrzebski commented 1 month ago

@AugustinLF good to see you back mate! 🙌

Regarding the change I have couple of thoughts: 1). Making the default behavior match Jest DOM would be a breaking change, and since Jest matchers are now in RNTL that would be a major version for RNTL as a whole. 2) I find the Jest DOM matcher name confusing, as toHaveTextContent kinda suggest content == text condition. A more suitable name would be toContainText or similar. In such case the non-exact default behavior would make sense. 3) The semantic of this matcher in JestDOM is different from getByText which checks the whole text, and has exact option. In contrast the RNLT matcher has the same semantic and options as getByText.

As for your use case, if you use exactly the same code to run RTL and RNTL tests, then consider creating own Jest matcher, that would conditionally call JestDOM or RNTL matchers with proper options. That seems like a more bulletproof solution.

Alternatively you can pass regex instead of string, and in such case both implementations should work the same.

Wdyt?

AugustinLF commented 1 month ago

You're raising good points. I don't think the fact that such a change would require a major bump is a blocker (we can definitely wait to do such a change when planning to release the next major version), but as highlighted in your answer, the question is indeed "do we think this API change would be beneficial".

Personally I think that the API exposed by RNTL is the better one (aligning with getByText only makes sense IMO). But there is a value nonetheless in having similar API between both libraries, I'd guess that there is a non negligible part of our user base that does use both libraries. They might not have like me a setup where they "write once, test everywhere", but having to keep in mind which behaviour belongs to which platform is definitely not great. Especially when it comes to utils like these, where the difference is not driven by a platform difference.

But then I'll raise that on the jest-dom repo, and se what they think of it.

In the meanwhile, would you be up for a PR that allows exporting the matchers, so I can (similarly to jest-dom) use part of them?

import matchers from '@testing-library/react-native/matchers'

const { toHaveTextContent, ...rest } = matchers

expect.extend({
  ...rest,
  toHaveTextContent: myCustomMatcher,
})

That way I could go indeed go over each matchers, and add what makes sense for both platforms?