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

Best practice while querying translated strings #738

Open
pedrosimao opened this issue Jan 13, 2021 · 8 comments
Open

Best practice while querying translated strings #738

pedrosimao opened this issue Jan 13, 2021 · 8 comments

Comments

@pedrosimao
Copy link

I am having a discussion with my team about what is the best practice when it comes to querying translated texts.

As those texts can be modified by non-developers (translators) we don't want our tests to be broken on each translation change. But a part of my teammates thinks we should keep on using plain strings, so our tests resemble user behavior...
In our current code we are doing something like this to find a button:

const buttonToFind = await screen.findByText('My button text');

We are currently using react-intl, so I proposed a solution that would go like this to find the same button:

const buttonText = getNodeText(<FormattedMessage id="buttonText" defaultMessage="Hello Button" />);
const buttonToFind = await screen.findByText(buttonText);

I think this would make the test more resilient, without breaking the rule of making tests the most similar as possible to user behavior, after all, we would be querying the same text as the user while avoiding unmaintainable strings. What do you think?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Jan 13, 2021

Some options:

Approach Pros Cons
Use strings from the default language Test is easy to read, and asserts expected default output. If you have variables in your strings, you can test that they work properly with correct output. 1. Strings hardcoded into tests mean you have to update both tests and code for any copy changes. 2. If multiple elements have the same string/substring text, find-and-replace may be hard to use reliably.
Mock the translation library If your library is difficult to use in the test environment, you can mock it so it is easier. For example, you can add the message ID as a data-attribute to the text so you can query by that. Test code deviates from what runs in production. Tests may assert about message IDs but not enough about content, so errors are possible.
Use translation library in tests Decouples strings from tests, so you can update the message files in one place without worrying about breaking tests. Can run tests in another language or multiple languages. const buttonText = getNodeText(<FormattedMessage id="buttonText" defaultMessage="Hello Button" />); Overhead - it takes more lines of code to write the test, and you need to know the variables and message IDs to create the right strings. It's not obvious what the text actually is when you read the test code, making maintaining it harder.
Use translation library + inline snapshots Same as above, but by adding an inline snapshot of the string, you can read the test code and see what strings are in use, but easily update them with jest -u if the messages change. expect(buttonText).toMatchInlineSnapshot("'My button text'") Tests are longer because of the extra lines. You can wrap up some of the translation-related code into a helper function to make it a little more inline-able and avoid repeating yourself, but you still need to know the message IDs and variables inside the test.

@pedrosimao
Copy link
Author

pedrosimao commented Jan 13, 2021

@alexkrolick very interesting benchmark of all options available! Thanks for sharing!
It seems Kent Dodds kind of prefer the first option. (using strings)
My proposition on the other side seems not to break his rule of making the test as similar as possible to user interaction. Do you have thoughts on this particular point?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Jan 13, 2021

You're still using the same strings as the user to select the elements, it's just a matter of how you generate those strings and whether there is a single source of truth or not. Another consideration is you don't necessarily want test code to rely too much on app code, because if something goes wrong, the test may stop asserting on the right thing. That's why I suggested adding an inline snapshot of any text you generate using messageIDs, so that you know what strings are being used in the test, and as a way to debug unexpected results from the translation framework. I find a test that only consists of messageIDs is really hard to debug in 3 months when I forget what the IDs map to and have to keep opening up the message definitions.

@alexkrolick alexkrolick transferred this issue from testing-library/react-testing-library Jan 13, 2021
@alexkrolick alexkrolick reopened this Jan 13, 2021
@claudivanfilho
Copy link

Querying by the actual text has this clear CON:
"Strings hardcoded into tests mean you have to update both tests and code for any copy changes."
But it also has a good PRO which is:
"The less you mock, the most integrated and similar to the real experience of the user it becomes".
Ex: What if someone deletes the intl key from the intl file? The test querying by the actual text will fail as expected.

The second approach gives me reliability about the code that is being shipped and resembles more to what the user will see in the screen. So that's why I prefer querying by the actual text.

@mikeplis
Copy link

mikeplis commented Jul 8, 2022

The sample code in Use translation library in tests doesn't appear to be working.

getNodeText expects an HTMLElement, so trying to pass it a FormattedMessage results in this TypeScript error:

Argument of type 'Element' is not assignable to parameter of type 'HTMLElement'.
  Type 'ReactElement<any, any>' is missing the following properties from type 'HTMLElement': accessKey, accessKeyLabel, autocapitalize, dir, and 277 more.ts(2345)

Executing the code results in this error:

TypeError: node.matches is not a function

@fawazdinnunhan
Copy link

The sample code in Use translation library in tests doesn't appear to be working.

getNodeText expects an HTMLElement, so trying to pass it a FormattedMessage results in this TypeScript error:

Argument of type 'Element' is not assignable to parameter of type 'HTMLElement'.
  Type 'ReactElement<any, any>' is missing the following properties from type 'HTMLElement': accessKey, accessKeyLabel, autocapitalize, dir, and 277 more.ts(2345)

Executing the code results in this error:

TypeError: node.matches is not a function

I've run into the same issue, how is this working for you? @pedrosimao

@mikeplis
Copy link

Fwiw I got it working with a combination of screen.findByText and intl.formatMessage

@fawazdinnunhan
Copy link

Fwiw I got it working with a combination of screen.findByText and intl.formatMessage

Doesn't that require the useInt() react hook, which can only be used inside components? or so the react hook rules suggests.

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

No branches or pull requests

5 participants