jest-dom icon indicating copy to clipboard operation
jest-dom copied to clipboard

Support assertions for each DOM query

Open benquarmby opened this issue 2 years ago • 8 comments

Describe the feature you'd like:

TL;DR:

expect(element).toContainOneByRole("heading", {
    name: "Attention"
});

Please consider adding jest assertions for each of the eight query types provided by @testing-library/dom. Assertions based directly on DOM queries would make this advice much easier to follow.

Advice: If you want to assert that something exists, make that assertion explicit.

Problem Statement

In many tests, asserting on both the existence and absence of elements is key to proving correct behavior. While it's true that getBy* and friends already throw excellent diagnostic information when something can't be found:

  • Relying on the throwing side effect for the purpose of assertion does not make the intent of a test clear.
  • The errors these queries throw don't help when asserting on the absence of an element.
  • Wrapping queries in fake assertions to make the intent clear results in spurious test feedback.

Fabricated Example

An app needs to show and hide a warning within a dialog as validity changes. This is what the test might look like today.

const dialog = screen.getByRole("dialog");
const input = within(dialog).getByLabelText("Foo");
userEvent.type(input, "INVALID");

// Don't forget to use `getBy*` here, or we'll see a confusing null error.
expect(within(dialog).getByRole("heading", {name: "Attention"})).toBeInTheDocument();

userEvent.type(input, "VALID");

// Don't forget to use `queryBy*` here or it will fail before it can check for absence.
expect(within(dialog).queryByRole("heading", {name: "Attention"})).not.toBeInTheDocument();

Drawbacks

Having assertions that are tightly integrated with DOM queries would require a dependency on @testing-library/dom which does not exist today. That said, the library is called @testing-library/jest-dom, which implies the two are connected.

Suggested implementation:

Proposal

Using "by role" as an example:

  • Add toContainOneByRole which passes when there is precisely one element. The negated version not.toContainOneByRole passes when there are zero or more than one elements. This has some similarities with getByRole.
  • Add toContainAnyByRole which passes when there are one or more elements. The negated version not.toContainAnyByRole passes when there are zero elements. This has some similarities with getAllByRole.

The assertion arguments would have exact symmetry with each query function. For example:

toContainOneByRole(role: ByRoleMatcher, options?: ByRoleOptions): void;
toContainAnyByRole(role: ByRoleMatcher, options?: ByRoleOptions): void;

Repeat for the other seven query types in @testing-library/dom.

Sample Implementation

A hastily put together implementation can be found in this gist: https://gist.github.com/benquarmby/7510252ab701669c2eaf3c0156dd680c. There are holes, but it works for demonstration purposes.

Describe alternatives you've considered:

  • Making a separate library, either official as @testing-library/jest-dom-query or unofficial.
  • Copy and pasting the custom matchers around.

Teachability, Documentation, Adoption, Migration Strategy:

Using the previous fabricated example, usage could look like this:

const dialog = screen.getByRole("dialog");
const input = within(dialog).getByLabelText("Foo");
userEvent.type(input, "INVALID");

expect(dialog).toContainOneByRole("heading", {name: "Attention"});

userEvent.type(input, "VALID");

expect(dialog).not.toContainAnyByRole("heading", {name: "Attention"});

benquarmby avatar Nov 09 '23 19:11 benquarmby

I would be open to this. Sounds good.

Hopefully someone can take it to contribute this new feature before I or other maintainer do it, which may take a while.

gnapse avatar Nov 14 '23 20:11 gnapse

Hopefully someone can take it to contribute this new feature before I or other maintainer do it, which may take a while.

I'm willing to make a contribution. My biggest concern is what I mentioned in the drawbacks section. These assertions would require a dependency on @testing-library/dom. Would you be open to having that as a production or peer dependency?

benquarmby avatar Nov 14 '23 20:11 benquarmby

Hmmm, good point.

@testing-library/core-maintainers thoughts?

gnapse avatar Nov 14 '23 20:11 gnapse

peer would be the proper way to handle this

kentcdodds avatar Nov 14 '23 21:11 kentcdodds

peer would be the proper way to handle this

Agreed. That's the way my local test implementation works.

We can assume most consumers of @testing-library/jest-dom probably already have some version of @testing-library/dom. That said, adding a new peer dependency is technically a breaking change; The new assertions will break without it.

I can get started on a PR if everyone is OK with:

  • adding a peer dependency.
  • accepting a breaking change.

benquarmby avatar Nov 14 '23 21:11 benquarmby

I'm ok with that. But let's give it a day or two, to see what others have to say.

gnapse avatar Nov 15 '23 00:11 gnapse

@benquarmby I say go for it.

gnapse avatar Nov 24 '23 10:11 gnapse

Made some progress here: https://github.com/testing-library/jest-dom/compare/main...benquarmby:jest-dom:jest-dom-query

Please ignore the VSCode settings. They're just for my personal convenience and will be removed. Still a way to go before it's ready for review.

benquarmby avatar Jan 16 '24 18:01 benquarmby