testing-playground icon indicating copy to clipboard operation
testing-playground copied to clipboard

Some questions about recommended queries

Open timkindberg opened this issue 5 years ago • 14 comments

Seems like overkill to use regex for recommended queries. Is that really what we want to propagate as the recommended pattern?

So for example the playground recommends:

screen.getByRole('button', { name: /signup/i })

But why not this simpler option?

screen.getByRole('button', { name: 'signup' })

Additionally maybe it's just me but I think:

screen.getByLabelText('Username')

is written more from a "user perspective" than:

screen.getByRole('textbox', { name: /username/i })

We just have to be careful, because folks are going to take your recommendations as gospel.

timkindberg avatar Jun 15 '20 14:06 timkindberg

All queries are recommended like it's described in "Which query should I use?"

MichaelDeBoey avatar Jun 15 '20 14:06 MichaelDeBoey

@MichaelDeBoey Ah ok, I see. @kentcdodds what are your thoughts on what I'm asking?? Do you find that the regex with the /i tends to be fine?

timkindberg avatar Jun 15 '20 17:06 timkindberg

The reason why it is used the regex instead of the string is because when you find an element is not important the case. So with regex you can search for:

  1. screen.getByRole('button', { name: /signup/i })
  2. screen.getByRole('button', { name: /Signup/i })
  3. screen.getByRole('button', { name: /SIGNUP/i })

with the same result

marcosvega91 avatar Jun 15 '20 17:06 marcosvega91

Basically what @marcosvega91 says 😊

At the end, a case sensitive query is fine. But we do recommend to use a case insensitive query because in 99% of all cases, it doesn't make sense to let your test fail due to changes in casing.

In other words, most of the times, the functioning of a button with a label Send will not change when the label changes to send.

I'm even willing so say that if the functioning of code depends on the casing of visual elements, that something might be wrong.

Case sensitive queries, only make sense when the test is explicitly meant to test just that; test('button has uppercase label'). We usually use the queries to locate elements, not to verify the labels. For that you might want to use expect, because it makes the intent of the query/test more clear.

Hope this explains it a bit. If not, please let me know.

smeijer avatar Jun 15 '20 17:06 smeijer

I get that, but it seems like we are getting just a tad farther away from "How a user would look for it".

I guess it's a bit safer for if/when the label casing might change, but if we want the test to break if the word is changed from "Signup" to "Create Account" I think it should also break from "Signup" to "signup".

I realize this is super pedantic. :) But this pedantry is why react-testing-library became popular.

It's just we are going down this slippery slope, where I used to think getByText('Signup') was the best. It was simple and clean and great.

Now I'm finding by Accessibility roles (using a guide on the internet of roles) and I'm passing a second arg object with a case-insensitve regex. It's made the whole thing a tad less easy to casually introduce to coworkers I think.

This is not complaining BTW! Not looking for a refund Kent ;) I love all the concepts and the library. I'm just wondering if it's something that's been considered, if the API needs a little course correction.

Maybe if the roles truly are the best way to do things, they should be codified into the API?? getButton('Signup'), getTextfield('Username')? I'm betting it's just easier to maintain getByRole though instead of a bunch of specific get methods.

Just some thoughts. Thanks everyone for your answers.

timkindberg avatar Jun 15 '20 17:06 timkindberg

This is not complaining BTW! Not looking for a refund Kent ;)

lol

I justify the reasoning for the priority order in both the docs @MichaelDeBoey mentioned as well as in Common mistakes with React Testing Library. We need to think about things from the terms of confidence.

Here's a quick example:

<div>
  <label for="username">Username</label>
  <input id="username" type="text" />
</div>

This will work with both:

screen.getByLabelText(/username/i)
screen.getByRole('textbox', {name: /username/i})

But what happens if I typo this (or more likely I'm using an abstraction that renders the input and something goes wrong):

<div>
  <label for="username">Username</label>
  <div id="username" type="text" />
</div>

The *LabelText query still works, but the UI is totally broken. This is a little contrived because a good test would probably type into the input and it would fail for that reason, but in general the idea is the same. The role gives us additional confidence that things will work as expected.

As for the regex, I personally prefer the regex with ignore case because the user doesn't care whether it's SUBMIT, submit, Submit, or even SuBmIt. If you want to do a string that's fine, but I'm happier recommending the regex.

I'm not going to make this decision though, if enough people feel differently then feel free to make a PR and other maintainers can merge it :)

kentcdodds avatar Jun 15 '20 18:06 kentcdodds

Hi, sorry to comment on a closed issue.. So the app I'm working on is in Japanese, so it makes no sense to have the text as a regex (Japanese doesn't have lowercase/uppercase). Now I'm copypasting and then changing back the regex to a string, or typing by myself from the start... I would really like to be able to copypaste. Would a PR that makes the regex optional be welcome? Of course I would be leaving the regex as the default.

asile12 avatar Jul 13 '21 09:07 asile12

Hi @asile12,

I'm happy to accept a pull request that adds an optional setting and defaults to the current behavior.

smeijer avatar Jul 13 '21 11:07 smeijer

I know this is old and closed, but I'll leave my opinion here. I always use case sensitive queries. Here why: UX cares about casing. Designs specify casing. Since designers care about casing, my tests do too.

coryhouse avatar Apr 26 '22 21:04 coryhouse

@coryhouse I can see you point, but IMO it's an overkill to pay so much care for the designers, which we'd want not only to test text cases, but also to test every CSS property that affects padding, margin, fonts, colors, animations etc.

clytras avatar Mar 31 '23 18:03 clytras

Same spirit here: our users want to see "Hello, John Doe!", not "hello, jOhn doe!". I want to be sure I output the right text to my users.

The casing of sentences and words can change the meaning: turkey vs Turkey, Polish vs polish, march vs March...

(also, it's harder to read and modify regex like /it's safe with us\. we hate spam!/i than "It's safe with us. We hate spam!" in my test code)

Is it possible to reconsider and re-open this issue?

tkrotoff avatar Oct 07 '25 14:10 tkrotoff

Reopened, it's been 5 years, and I too agree case sensitive testing is in fact better. I'd gladly accept a PR that makes this change.

smeijer avatar Oct 08 '25 15:10 smeijer

My team recently ran into an issue where a change in the combination of parent/child components caused the a11y name of two elements to be concatenated in a way that was undesired. I'm leaning towards strings over regex, to reduce the risk of that occurring again, and to have better confidence in the testing of our UI.

Just to confirm @smeijer, when you mentioned being open to a PR to fix the case sensitivity, you’re referring to updating the suggestion logic to prefer strings ('Sign up') instead of regex (/Sign up/), or just simply suggesting case sensitive regex?

From what I can see, the suggestion logic lives in @testing-library/dom-testing-library here, the change would need to happen there I would think? If so, does that mean we should first go ahead and move the issue over there and get feedback since that has more widespread downstream effects?

mikedidomizio avatar Oct 20 '25 16:10 mikedidomizio

@mikedidomizio , I'm open to prefer strings. But if we can't do that without changes to dom-testing-lib, then we should open an issue there. The impact on that end is far larger though (as it'd be breaking change), so if we can change the recommendation here directly, that'd be far easier (even if it'd be a bit more work).

smeijer avatar Oct 20 '25 18:10 smeijer