Create @wordpress/test-utils package

[diegohaz]

Description

Closes #17249

This PR creates a new package called @wordpress/test-utils that provides utility methods to write integration tests with almost as much confidence as with end-to-end tests with the same developer experience as with unit tests (easy to write and run).

This is not meant to be a replacement for e2e tests though. This is more like a much better alternative to unit testing components with Enzyme.

Usage examples can be found in the tests: https://github.com/diegohaz/gutenberg/tree/add/test-utils/packages/test-utils/src/test

Why

In addition to what was discussed in #17249, I'm considering that, once it's stable, we'll recommend this library for testing projects that are already using @wordpress/* packages, like custom blocks and apps.

Per this comparison, react-testing-library seems to be the best option for testing single React components. That's why this package is wrapping many of their methods. But it's still not ideal compared to manual browser testing or end-to-end testing, specially because you can't reliably reproduce user interactions with their abstractions.

For example, even though this works in the browser, this would fail using any of the existing testing libraries out there that use jsdom:

import { render, fireEvent } from "@testing-library/react";

test("should call onMouseDown when button is clicked", () => {
  const onMouseDown = jest.fn();
  const { getByText } = render(
    <button onMouseDown={onMouseDown}>button</button>
  );
  const button = getByText("button");
  fireEvent.click(button);
  expect(onMouseDown).toHaveBeenCalled(); // ❌ failure
});

The test would have to know about the component implementation details and use fireEvent.mouseDown() instead. We would fall into a similar problem I described here:

I noticed that most unit tests on Toolbar broke even though the UI worked and outputted the same HTML as before.

The current solution for this is to write end-to-end tests instead. But they're harder to write and incredibly slow to run. Using jsdom we can --watch specific test files and have feedback almost instantaneously after saving our files.

The intention of this PR is to grab the best react-testing-library abstractions and fill its gaps with user interactions. This would work:

import { render, click } from "@wordpress/test-utils";

test("should call onMouseDown when button is clicked", () => {
  const onMouseDown = jest.fn();
  const { getByText } = render(
    <button onMouseDown={onMouseDown}>button</button>
  );
  const button = getByText("button");
  click(button);
  expect(onMouseDown).toHaveBeenCalled(); // ✅ success
});

How has this been tested?

• Did a lot of real browser testing to see how it handles user actions
• npm run test-unit

Types of changes

New feature

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR. .

[ItsJonQ]

@diegohaz I'm very excited about this 😍 .

I think testing React components/apps based on what the user sees and actual* DOM events is a big improvement.

(*As real as JSDOM is concerned)

We'll get much more integration and coverage across code when your components are executed and rendered, rather than the fragmented and often heavily mocked tests from something Enzyme.

Adding a collection of test utils (like your click, hover, and press, and type functions) will make it that much easier to write tests ✨

[aduth]

Since I observe many of these functions proxy through to underlying testing libraries (Testing Library), and while I can see there's quite a bit of added utility here, I'm curious in your own words why not to just use Testing Library directly, than to create our own custom abstraction atop an existing abstraction?

Since with an abstraction, we'll have additional maintenance overhead and lose of any benefit of shared knowledge of an existing tool (barrier to entry for new contributors), I'd want to be clear on the benefits it brings.

I can imagine some of the scenarios where these added behaviors could be useful, but since we never tried to see how far we could get with Testing Library alone, it's unclear whether these would be common pain points in that experience.

(Note: I read through #17249 but I'm also retroactively catching up on this discussion, so apologies if I've overlooked this mentioned previously)

[aduth]

Should we be modifying the global this way here? Is there even any guarantee that the DOM globals exist? In our own project, we have these globals set up, but this is managed by the Jest preset. Are other projects which consume from test-utils expected to set this up themselves? Should this be documented?

[diegohaz]

Good question! This library will require projects to setup an environment that at least simulates the browser, like jsdom, which seems pretty common on React projects (I'm not sure about custom blocks projects though). But I'll include that in the readme.

This mock is needed here because jsdom can't do layout and therefore there will be no value for getClientRects, clientWidth or clientHeight. The isVisible function in @wordpress/dom would always return false.

I'm going to experiment mocking that only when the related focus methods are called and restoring it right after that. It seems to be a safer approach.

[diegohaz]

Thanks for the review @aduth! 😊

Updated the PR description with this.

Basically, I'm considering that, once it's stable, we'll recommend this library for testing projects that are already using @wordpress/* packages, like custom blocks and apps.

Per this comparison, react-testing-library seems to be the best option for testing single React components. But it's still not ideal compared to manual browser testing or end-to-end testing, specially because you can't reliably reproduce user interactions with their abstractions.

For example, even though this works in the browser, this would fail using any of the existing testing libraries out there that use jsdom:

import { render, fireEvent } from "@testing-library/react";

test("should call onMouseDown when button is clicked", () => {
  const onMouseDown = jest.fn();
  const { getByText } = render(
    <button onMouseDown={onMouseDown}>button</button>
  );
  const button = getByText("button");
  fireEvent.click(button);
  expect(onMouseDown).toHaveBeenCalled(); // ❌ failure
});

The test would have to know about the component implementation details and use fireEvent.mouseDown() instead. We would fall into a similar problem I described here:

I noticed that most unit tests on Toolbar broke even though the UI worked and outputted the same HTML as before.

The current solution for this is to write end-to-end tests instead. But they're harder to write and incredibly slow to run. Using jsdom we can --watch specific test files and have feedback almost instantaneously after saving our files.

The intention of this PR is to grab the best react-testing-library abstractions and fill its gaps with user interactions. This would work:

import { render } from "@testing-library/react";
import { click } from "@wordpress/test-utils";

test("should call onMouseDown when button is clicked", () => {
  const onMouseDown = jest.fn();
  const { getByText } = render(
    <button onMouseDown={onMouseDown}>button</button>
  );
  const button = getByText("button");
  click(button);
  expect(onMouseDown).toHaveBeenCalled(); // ✅ success
});

Testing Library has a package called user-event which I'm planning to contribute to. But right now it doesn't accurately reproduce user interactions, it's not well integrated with React, and I'm afraid that it becomes unmaintained.

One option is to make @testing-library/react a peer dependency and only expose the user interaction methods. The tradeoffs are that it's more work for people consuming @wordpress/* packages and it would be harder for us to get rid of Testing Library in the future if needed.

[gziolo]

I appreciate the efforts. This is an outstanding proposal. It's evident that you put a lot of work into crafting this package. It resembles a lot the public API of Puppeteer as noted in details in my comment :)

The current solution for this is to write end-to-end tests instead. But they're harder to write and incredibly slow to run. Using jsdom we can --watch specific test files and have feedback almost instantaneously after saving our files.

Let me respond to your statement. Historically, we had more PRs opened from new contributors proposing e2e tests rather than those proposing unit tests, which goes against your point that writing e2e tests is harder. I'm sure it's very hard to set everything up and ensure they are reliable. However, there are many reasons why e2e tests are difficult to maintain like network requests, the overall complexity of the app under test, etc.

I agree that having the watch mode working out of the box for unit tests is a big advantage. I'm wondering if you could replicate the same conditions when using Jest + Puppeteer by importing the same render method used in the tested page in the actual test file and thus having this feedback wired with the watch mode.

Overall, I think you are promoting the same API I would love to see when testing UI components in isolation. I'm still not convinced that mocking DOM with jsdom and building additional abstractions over DOM to make it behave like a regular browser is the best approach. Anyway, I like that you provided a proof of concept which can lead the discussion in the right direction.

[gziolo]

I must admit that it looks very familiar to the Puppeteer API. I did quick search in their documentation to find matching methods and nearly all of them are covered:

• act – React testing helper
• blur
• click - https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-pageclickselector-options
• fire-event
• focus – https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-pagefocusselector
• hover – https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-pagehoverselector
• press – https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-elementhandlepresskey-options
• render – React testing helper
• type – https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-pagetypeselector-text-options
• wait - https://pptr.dev/#?product=Puppeteer&version=v2.0.0&show=api-pagewaitforselectororfunctionortimeout-options-args

This makes my comment left on the parent issue even more tempting to explore:
#17249 (comment)

[getdave]

Thanks for taking this up @diegohaz. In my opinion, this is much needed and a great contribution. Thank you!

As I expressed in my original Issue, Enzyme often encourages less optimal testing practices - although it is possible to test well, it's much easier not to!

I agree with your approach which aligns nicely with that which I suggested in the Issue. Using an abstraction over RTL avoids us reinventing the wheel and provides the benefits of a well-tested library. If it ever falls out of favour or becomes unmaintained, we can simply port over the methods we require and continue to maintain this test-utils package ourselves.

Since I observe many of these functions proxy through to underlying testing libraries (Testing Library), and while I can see there's quite a bit of added utility here, I'm curious in your own words why not to just use Testing Library directly, than to create our own custom abstraction atop an existing abstraction?

@aduth My argument here is simplicity and utility. I've now implement x2 PRs where I've tried to follow testing "best practices" using the raw React Test Utils. Whilst it is perfectly possible to test well using this approach, it isn't easy. In general, it requires writing a lot of custom code which makes the process laborious (and somewhat tedious) - hardly a good recipe for encouraging folks to write tests.

As an example, say you want to select a label by its text content (this is preferred over DOM attributes such as class as it is what the user sees). This is an extremely common requirement which I found myself requiring time and time again.

With RTL this is as easy as using the getByText() method (eg: getByText('Hello World')). Without this abstraction, you need to write your own implementation. At first, this seems simple. Perhaps something such as:

document.querySelector('*').find(x => x.innerText === 'Hello World');

However, this is naive and will return false positives as innerText returns the text content of the the entire DOM tree (including descendant nodes) rather than that only restricted to the individual node x. This quickly becomes a rabbit hole of adding additional complexity and condition cases. After a while, you come to the realisation that you're basically rewriting RTL!

I'm sure someone will suggest a simple workaround to the above example, however I believe the wider point it illustrates is still valid.

If we want contributors to write tests (and we surely do) then we need to make it as simple as possible. You shouldn't need to know how to implement a text content matcher in order to test a simple component using best practices.

In summary, the principal reasons why I'm in favour of wrapping RTL are:

• simple and easy to use API - approachable
• helps to avoid testing component implementation details (internal API) without having to read the documentation on testing best practices - the methods themselves "force" best practices on you.
• well known in the community - good support in the public domain
• well supported
• well tested
• recommended by the creators of React
• recommended by some of the most prominent people in the React testing space
• easy (with some caveats) to migrate away from in future if we commit to maintain a simple subset of the main API
• faster than e2e tests with less overhead

vs e2e tests

I should also add that I'm not suggested we stop writing e2e tests. I believe we should recognise that they solve a different purpose.

RTL should be used to quickly add test coverage to a group of related components, but not an entire application. We will always need e2e tests but in my experience having written a fair number of e2e tests in Gutenberg (although admittedly not as many as the others contributing to this discussion!) they do not provide a fast feedback loop (~30s even on my 1yr old MBP isn't quick). This slows down development and reduces the likelihood that I'll bother to write tests at all.

We should also note that there is also (entirely valid) resistance to adding e2e tests for small components (or groups thereof). There is considerable overhead with each new e2e test and therefore only key interactions are deemed important enough to warrant being covered in this level of detail.

This is precisely why we currently have "unit" tests for components. A simple, effective and fast way to provide some level of confidence in our code without the overhead of a full e2e test.

This proposal doesn't remove the requirement for e2e tests, it merely improves the existing implementation of our component "unit" (Functional) tests.

Thanks again @diegohaz.

[getdave]

...also, I wonder whether we there would be value in seeing a refactor some existing component tests to use this new package. Probably in a separate PR branched from this PR.

[diegohaz]

Thanks for the thoughtful comment @getdave! I started rewriting some existing tests in my own fork. Here's a PR for Toolbar: diegohaz#1

It includes tests for the roving tabindex behavior using the press module provided by this PR.

[getdave]

@diegohaz Can I do anything to help you to get this over the line?

[diegohaz]

@getdave The code here is just waiting for an approval (and possibly more refactors on existing tests? Just let me know) 😆

There are some conflicts now. I'll resolve them asap.

[talldan]

This is a pretty incredible PR, @diegohaz! Kudos to you for the amount of work that went into it.

I've added the Needs Decision label, as I wasn't sure if an approach had been agreed on in terms of testing components, I know there's been some discussion in the past about using an end-to-end testing framework for that purpose.

I personally like the idea of these tests being quick to run, providing a fast feedback loop, and exposing a more human-understandable API than some other react testing libraries.

Looking at the code, the most intricate part is that it simulates a lot of the fundamental browser interactions—e.g. chains of events that get triggered from a click/hover/etc.—and it looks like lots of attention to detail went into the implementation. I could also see that this part will need quite a bit of real world usage to establish that it works as expected, and it's also where maintainers of the package will need the most knowledge. Documenting and testing how this works will be important, so it's great that there are tests in this PR.

I started reviewing, but it'll take a bit of time to go through everything.

[talldan]

I wonder if conditions like this should fail noisily given they indicate undesired behaviour (or dead test code).

[diegohaz]

The reason it's not throwing is so you can call blur() just to blur the active element, even if there isn't one (and document.activeElement is document.body, so isBodyElement(element) would return true).

This is used by focus, for example:

gutenberg/packages/test-utils/src/focus.js

Lines 15 to 27 in e317fe4

	export default function focus( element ) {
	if ( getActiveElement( element ) === element || ! isFocusable( element ) ) {
	return;
	}
	blur();
	act( () => {
	element.focus();
	} );
	fireEvent.focusIn( element );
	}

But I think we can throw if you explicitly pass an element to blur(element).

[talldan]

Is there any benefit to passing an element into this function? It seems like it has to be the activeElement anyway (see line 22).

[diegohaz]

My thoughts for adding that parameter were:

• Consistency: all the other functions receive element as a parameter.
• Readability: you may want to explicitly pass the element to this function so your test code is more clear.

But I'm totally fine in removing it. :)

[ItsJonQ]

@diegohaz Clever ✌️😇

[diegohaz]

I've created a simpler alternative that just adds @testing-library/react, without the utilities proposed by this PR: #20428

[diegohaz]

Also, as an alternative to this PR, we could use @testing-library/user-event, which implements similar utilities.

We don't have to maintain it ourselves as we would have to do with this PR. But the project is still not mature enough, and since it's not being maintained by the core react-testing-library maintainers, I'm afraid that it becomes unmaintained at some point.

[gziolo]

I came to this PR to mention @testing-library/user-event based on the recent article with official recommendations: https://kentcdodds.com/blog/common-mistakes-with-react-testing-library#not-using-testing-libraryuser-event:

@testing-library/user-event is a package that's built on top of fireEvent, but it provides several methods that resemble the user interactions more closely. In the example above, fireEvent.change will simply trigger a single change event on the input. However the type call, will trigger keyDown, keyPress, and keyUp events for each character as well. It's much closer to the user's actual interactions. This has the benefit of working well with libraries that you may use which don't actually listen for the change event.

We're still working on @testing-library/user-event to ensure that it delivers what it promises: firing all the same events the user would fire when performing a specific action. I don't think we're quite there yet and this is why it's not baked-into @testing-library/dom (though it may be at some point in the future). However, I'm confident enough in it to recommend you give it a look and use it's utilities over fireEvent.

Advice: Use @testing-library/user-event over fireEvent where possible.

Should we close this PR and integrate this library instead? I see new commits landed in the last days so it means it's still actively maintained.

[gziolo]

We also discussed it this week here: #22352 (comment).

[diegohaz]

Yeah! I agree that we should close this and use @testing-library/user-event instead. It's also a default dependency on apps created with create-react-app.

One thing to keep in mind is that it may be necessary to wrap the user events within act (it's probably not needed, but some people are reporting it on testing-library/user-event#128).

[gziolo]

Kudos for the work on this PR regardless of the fact that it was closed :)