testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.27k stars 467 forks source link

getByLabelText fails with unclear error if aria-labelledby has an invalid target #710

Closed msendlakowski closed 4 years ago

msendlakowski commented 4 years ago

Relevant code or config:

getByLabelText("a11y label text");

What you did:

Identifying an icon

What happened:

TypeError: Cannot read property 'getAttribute' of null

      39 | 
      40 |         test('then the overlay will render', async() => {
    > 41 |             expect(getByLabelText("a11y label text")).toBeTruthy();
         |                    ^
      42 |         });

Reproduction:

When the container does not have a node with id "c0-dialog-0-6-text"

<svg aria-hidden="true" class="icon icon--close" focusable="false" aria-labelledby="c0-dialog-0-6-text">
    <use href="#icon-close"></use>
</svg>

Problem description:

This generates a JS error rather than returning a user friendly test failure. In fact, it should probably just fail the test with no error.

Suggested solution:

I think an appropriate fix would be just to return an empty string from the getLabelContent function when "label" doesn't exist. https://github.com/testing-library/dom-testing-library/blob/master/src/queries/label-text.js#L54

kentcdodds commented 4 years ago

cc @delca85. I think this may be related to your changes. What do you think we should do here?

delca85 commented 4 years ago

Yes, it is totally dued by my changes.

Like @msendlakowski said, the code looks for a label with the id found in the aria-labelledby attribute in some element.

I think that the code flow should go on in a case like this, because this is maybe an incoherence found in the dom but not directly related to the inserted query and the element labeled by the desired value could still be found.

Maybe check if the element with the desired is is null before calling getLabelContent is what I should have done from the beginning.

If it sounds good to all of you I could work on it later today.

kentcdodds commented 4 years ago

Sounds great!

kentcdodds commented 4 years ago

:tada: This issue has been resolved in version 7.21.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket: