React-testing-library: Add getById and getByClass

Created on 30 May 2020  Â·  22Comments  Â·  Source: testing-library/react-testing-library

Querying by id and classes is the _Hello World_ of the frontend querying and the library doesn't allow that for now.

What about creating the getById and getByClass on the tests?

Restricting the frontend id querying is not improving, it is quite the opposite, what is really does is decrease the capacity of the library.

Placing a lot of data-testids on the code pollutes it, because it places unnecessary tags on html codes in terms of code functionality. The code becomes polluted with tags only related for testing.

I am in favor of TDDs, but adding things in the code (or worse: on html tags) only for testing is exaggerated. A good TDD will improve the code with more organization! A bad TDD will add things on code only related for testing purposes.

Some people can argue and defend the usage of data-testids on the code for whatever reason, but what I find uncomfortable is to disable for everybody the most basic querying on DOM elements.

Most helpful comment

@juandiegombr

Another issue is when we use third party libs to fill the HTML with elements dynamically and we have no control over what and how it will show... in addition, this third party library will eventually have new versions and change the way it display things on the page.

On this case, the only possible way I can find to get the element the third party library renders is by injecting the ugly data-testid, which pollutes a lot the code. To make the tests less likely to have to alter the tests over and over again when the library is upgraded, since we have no control. And I can show you why...

Since we don't have the getById, queryById and etc... we have two choices:

  1. Use some getByText or getByRole which can be completely useless if the third party library version is updated and changes the way the elements are shown on the page.
  2. Use the data-testid + getByTestId, which makes the code polluted, but will make the tests more consistent without requiring to do maintenance later on. It will decrease the likelihood of doing maintenance just to adapt to the third party library.

On the other hand, most libs that generates automatically html elements on the page already have a property called id which will be used internally by the library to create the elements. For example: https://www.telerik.com/kendo-react-ui/components/upload/api/UploadProps/#toc-id

The new getById on this case is the optimal solution in my opinion, because it will maintain compatibility with new versions of the third party library and it also doesn't pollute our code, since the id property is already being used internally by the library to build components, we just need to provide the id value and everything will work fine.

We will have a better control on the tests without polluting the code.

Each case is different and the getById can be the ideal solution for particular cases like this one.

Removing the getById from the library is just decreasing the capacity of the library to handle different scenarios.

All 22 comments

You have other ways to query your elements better than class or id without using data-testid.

You can get your elements by text and also give it a try to queries like getByRole or getByLabelText. That will force you to make your app more accessible. It is always something to care of!

Thank you for the quick response, guys!

@juandiegombr The problem with using getByRole and getByLabelText is that the role and the text can be the same for different html elements. In terms of functionality, sometimes we can display the same text message in two different places of the DOM and it is not a good approach to restrict the functionality because of the testing suite. On the other hand, on a well designed frontend DOM structure, the #id of the elements will be unique, and this will be the advantage of the getById.

@raoufswe The problem with using the approach with querySelector is that it leave behind the features of the getBy... matchers.

I was already aware of these breakthrough approaches to resolve this issue... but I really think it is not the ideal solution because of the issues that I mentioned above.

In addition, using the getById on the test suite will force the elements to have unique ids, which is a good design of html elements. It will intuitively force the code to be more organized. That is a good TDD in my opinion.

The HTML id attribute is used to specify a unique id for an HTML element (the value must be unique within the HTML document).
https://www.w3schools.com/html/html_id.asp

That's why I am in favor of creating a getById and/or getByClass.

Nothing stops you to write you own query helper for getById or getByClass. Personally, I haven't found the need for them.

Can you give an example where you have the same text twice? Most of the time I sorted this in different ways then cluttering up the HTML by adding id-attributes

@weyert

_Can you give an example where you have the same text twice?_

Facebook for example we have the same user name in different places of the same page.

Screen Shot 2020-05-30 at 23 02 45

As I said, sometimes we can display the same text message in two different places of the DOM and it is not a good approach to restrict the functionality because of the testing suite.

Precisely, if you really need them, you can add custom queries and using querySelector is very easy to do anyway so you can feel free to do that if you want.

We're not going to add more direct support for querying by ID or class name than we already have so I'm going to close this issue.

Just saw your message come in. For something like that you can scope your query using within. That's what the user subconsciously does anyway.

I was searching here also for the solution for this problem.

I'm against to introduce hacks in the app code to write the tests. Because it will pollute the codebase with useless code that are only used on the tests.

This will also increase the bundle pack size in a few bytes, what will affect the user page load in some degree. (I know that we can use another hack to remove the react-testing-library hacks during the the assets precompilation).

Is good that we have ways to write our own finders to bypass this restriction, however since this approach is the standard for this library, this will leads many projects to go with the standard approach and by consequence adding all these problem discussed here.

Facebook for example we have the same user name in different places of the same page.
Screen Shot 2020-05-30 at 23 02 45

Personally, I never test the whole page and try to match the same text in such manner in unit tests. The only case where it really occurs is when using Testing Library in Cypress. Which is then easily solved by using within-query.

I don't see how adding id-attribute for texts like in the above example is any stranger than using test-id which is clearly the reason why the id's are being added to ease the testing. Personally, I am not a huge fan of test-id throughout my code so I am using it as last resort really. Luckily, I have been able to avoid using those in most cases.

I don't see how adding id-attribute for texts like in the above example is any stranger than using test-id

Adding ids to the elements is not only designed for testing things, it will improve the html elements, enabling them for more succinct and readable querying later on. That's why I said that adding getById will intuitively force the code to be more organized. That is a good TDD in my opinion.

Adding data-testid will pollute the code because the idea behind this is just to add attributes on html elements just for testing purposes. Querying by data-testid later on on the code will just make the code more confusing, because it is not the purpose of the data-testid.

That's why I really believe that: id > data-testid

Also, I used the Facebook as an example just to demonstrate that a well-known, well-designed and consolidated website can contain the same text in different parts of it, imagine the smaller sites then.

I use testing-library in integration tests for whole pages. I usually have a few elements with the same text but there's always forms to get them without querying by class or by Id. Sometimes there is not a one line selector but It doesn't make it more difficult to read, instead it has other benefits as making more explicit and semantic your tests.

As I said in my comment before, If we have accessibility in mind, that two elements with the text of Victor should be in different ARIA landmarks and you can get those areas by role and the get the specific element using within.

@juandiegombr

Another issue is when we use third party libs to fill the HTML with elements dynamically and we have no control over what and how it will show... in addition, this third party library will eventually have new versions and change the way it display things on the page.

On this case, the only possible way I can find to get the element the third party library renders is by injecting the ugly data-testid, which pollutes a lot the code. To make the tests less likely to have to alter the tests over and over again when the library is upgraded, since we have no control. And I can show you why...

Since we don't have the getById, queryById and etc... we have two choices:

  1. Use some getByText or getByRole which can be completely useless if the third party library version is updated and changes the way the elements are shown on the page.
  2. Use the data-testid + getByTestId, which makes the code polluted, but will make the tests more consistent without requiring to do maintenance later on. It will decrease the likelihood of doing maintenance just to adapt to the third party library.

On the other hand, most libs that generates automatically html elements on the page already have a property called id which will be used internally by the library to create the elements. For example: https://www.telerik.com/kendo-react-ui/components/upload/api/UploadProps/#toc-id

The new getById on this case is the optimal solution in my opinion, because it will maintain compatibility with new versions of the third party library and it also doesn't pollute our code, since the id property is already being used internally by the library to build components, we just need to provide the id value and everything will work fine.

We will have a better control on the tests without polluting the code.

Each case is different and the getById can be the ideal solution for particular cases like this one.

Removing the getById from the library is just decreasing the capacity of the library to handle different scenarios.

I took a time to investigate and discovered how to accomplish that

import { configure } from '@testing-library/dom'
configure({ testIdAttribute: 'id' })

That's all I needed.

you can't use waitFor with container.querySelector() 👎

@r3wt I upgraded the version of the react-testing-library and looks like it is not possible to use this approach with configure that I discovered anymore.

Could you enable it again guys? @kentcdodds

Thank you very much!

Hi @Victorcorcos the behaviour is not changed, could you please add more details about your problem?
If you can create a case on codesandbox should be fine

Thanks

With all the respect to the library authors/maintainers/contributors, as a first time user of this specific testing library I really feel like this project tries to enforce me to write code the way the testing library wants rather how I want. I do not object to projects promoting good coding standards, that is desirable.

My markup was like this:

 <li>
      <a className="link" href={link.url}>
          {link.title}
      </a>
    </li>

Following the prompts of this project, I used this line in my tests:

expect(screen.getByText("some text").href).toContain("/some-href")

And due to a spec change, it the component became like this:

 <li>
      <a className="link" href={link.url}>
        {link.icon && (
          <img className="link__icon" src={link.icon} alt="" />
        )}
        <span className="link__text">{link.title}</span>
      </a>
    </li>

Now my test code is broken because I needed to wrap the text in a span element. Had i used a class selector, I wouldn't need to change my tests at all.

According to this project I should go back to my markup and add a bunch of data-testids (which I am not inherently against) and fix all my test cases to use getByTestId. Not good times. How will I avoid this in the future? I will put a data-testid on everything, increase the amount of markup I write on my component and get rid of the getByRole/getByText selectors in my code (so what's the purpose of these selecotrs anyway then?)

Is there a better way to structure my code to avoid this pain? Maybe there is. I can't think of it, sorry, maybe I am not as good at it as others. And I assume I can't rely on the help of volunteers on this forum to help me along the way as I build my app. So i have three options:

1) Write custom code to accomplish this simple task and maintain it myself, essentially hacking around the library and having code that may not work in the future, since these aren't supported solutions (as one person demonstrated before)
2) Accept a coding style in a context that doesn't make sense according to my judgment just to be able to test my code easily
3) Replace my testing library.

You see, I don't really wish to do any of those, all of these three options have a negative impact on my productivity. What I will probably end up doing is suck it up, deliver my current project and never touch this library again, which is sad because I think it has some serious benefits over some others.

Edit: And all of that just because it doesn't fit with the philosophy of the project. Because nowhere did I see that this is not implemented due to code complexities or lack of resources.

Just fyi to the maintainers, testing table elements is horror in this library. could all be remedied by providing css/id/class selectors

How do you test a component that does not have text without being able to use class name etc? I.e. something that renders like this?

 <button
    aria-label="close dialog"
    class="wv-close--large"
    />

@Coler, for that case you use getByRole('button', {name: 'close dialog'});, this will use the aria label to find it

On Mon, 16 Nov 2020, 9:42 pm Coler, notifications@github.com wrote:

How do you test a component that does not have text without being able to
use class name etc? I.e. something that renders like this?

aria-label="close dialog"
class="wv-close--large"
/>

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/testing-library/react-testing-library/issues/683#issuecomment-728346236,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AASLMLXZAVYLVP6BALOICGLSQGMD5ANCNFSM4NOR3EHQ
.

@coler-j use getByLabelText('close dialog'), which supports both label and aria-label matching. we are using this alot in our apps at work, and it works well with material-design (unlike inputs rofl)

@brendan-donegan @r3wt thanks guys, I found that a bit after commenting.

Unfortunately stuck on v 9.5 where it doesn't seem to work. Working on getting my org updated.

Thanks!

You have other ways to query your elements better than class or id without using data-testid.

You can get your elements by text and also give it a try to queries like getByRole or getByLabelText. That will force you to make your app more accessible. It is always something to care of!

This library shouldn`t force anything but provide as many helpers as possible, I'm also here stuck figuring out if I add the useless test-ids since there's no better option.

The library gives access to normal DOM selectors so can do this

    const { container } = render(<Skeleton height={10} paragraph={4} />);
    expect(container.getElementsByClassName('skeleton-paragraph-row').length).toBe(4);
Was this page helpful?
0 / 5 - 0 ratings

Related issues

alejandronanez picture alejandronanez  Â·  4Comments

nagacoder picture nagacoder  Â·  3Comments

addamove picture addamove  Â·  3Comments

jalvarado91 picture jalvarado91  Â·  3Comments

AirborneEagle picture AirborneEagle  Â·  3Comments