getByRole's error message is incorrect on button with role "tab"

mattstobbs commented 4 years ago

DOM Testing Library version: 5.5.0
node version: 13.12.0
npm (or yarn) version: 1.22.4

Relevant code or config:

import React from 'react';
import { render, screen } from '@testing-library/react';

const Button = () => <button role="tab" aria-label="my-button" />;

test('button exists', () => {
  render(<Button />);

  screen.getByRole('button', { name: 'my-button' });
});

What happened:

The test (correctly) failed because it could not find an element with the role of button (the button has a role of "tab"). However, when the error message listed the accessible roles, it listed the button's role under "button":

TestingLibraryElementError: Unable to find an accessible element with the role "button" and name "my-button"

Here are the accessible roles:

  document:

  Name "":
  <body />

  --------------------------------------------------
  button:

  Name "my-button":
  <button
    aria-label="my-button"
    role="tab"
  />

Reproduction:

Code Sandbox

Problem description:

The current behaviour is confusing because the error message says there is no element with the role of "button" and then shows that there is an element with the role of "button".

Suggested solution:

Buttons with other roles (e.g. tab) should be shown under their correct roles in the error message.

I'm happy to put up a PR for this but would need pointing in the right direction as I'm unfamiliar with the code 😁

eps1lon commented 4 years ago

Thanks for the report. That is indeed weird.

You should start with adding a test to src/tests/role.js. The code for ByRole lives in src/queries/role.js.

mattstobbs commented 4 years ago

It looks like there are actually 2 issues here.

The first is the library aria-query, which we're using to get a map of the elements to their possible roles. Under elementRoles, the button element only has the role "button". This would need to be extended to include other roles, such as tab.

The second is that the matches function correctly determines that <button role="tab /> has the role of tab, but currentNode.matches determines the node has the role of button. Do you know where that function is defined?

mattstobbs commented 4 years ago

Just dug a bit more into this. currentNode.matches returns true for the selector "button" because it is a button. The way this is handled in the aria-query library is to have more specific selectors first, so they will match first (i.e. the button would match with button[role="tab"] before it matches with button).

Therefore, the solution would be to put up an issue and PR in the aria-query library, and once that is merged, update the version used by dom-testing-library. If you're happy with that, @eps1lon, I can go ahead and do that.

eps1lon commented 4 years ago

Great investigation! We do sort by specificity: https://github.com/testing-library/dom-testing-library/blob/429de04d023f2c49af2cb7b10d6daa537d417999/src/role-helpers.js#L118 I think we just need to refine the logic so that an explicit "role" has the highest specificity: https://github.com/testing-library/dom-testing-library/blob/429de04d023f2c49af2cb7b10d6daa537d417999/src/role-helpers.js#L92-L94

mattstobbs commented 4 years ago

That would make sense. It looks like that logic is still manipulating the map from aria-query, so it still wouldn't include button[role="tab"] because that library doesn't include it?

eps1lon commented 4 years ago

aria-query definitely includes tab.

mattstobbs commented 4 years ago

When I console.log(elementRoleList), which is the output of

https://github.com/testing-library/dom-testing-library/blob/429de04d023f2c49af2cb7b10d6daa537d417999/src/role-helpers.js#L5

I get the following list: elementRoleList.txt

which doesn't include tab, or a selector called button[role="tab"]. Am I missing something?