From 36a71531df1df4c009b8293512eab1853bc96ff4 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Wed, 6 Nov 2024 15:37:14 -0800 Subject: [PATCH 1/5] Update 08-testing.md for userEvent --- docs/development-guide/08-testing.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/development-guide/08-testing.md b/docs/development-guide/08-testing.md index 4a025ce90f4..61decd743b4 100644 --- a/docs/development-guide/08-testing.md +++ b/docs/development-guide/08-testing.md @@ -64,29 +64,31 @@ describe("My component", () => { Handling events such as clicks is a little more involved: ```js -import { fireEvent } from "@testing-library/react"; +import userEvent from '@testing-library/user-event'; import { renderWithTheme } from "src/utilities/testHelpers"; import Component from "./wherever"; const props = { onClick: vi.fn() }; describe("My component", () => { - it("should have some text", () => { + it("should have some text", async () => { const { getByText } = renderWithTheme(); const button = getByText("Submit"); - fireEvent.click(button); + await userEvent.click(button); expect(props.onClick).toHaveBeenCalled(); }); }); ``` -If, while using the Testing Library, your tests trigger a warning in the console from React ("Warning: An update to Component inside a test was not wrapped in act(...)"), first check out the library author's [blog post](https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning) about this. Depending on your situation, you probably will have to `wait` for something in your test: +Note that we recommend using `userEvent` rather than `fireEvent` where possible. This is a [React Testing Library best practice](https://testing-library.com/docs/user-event/intro#differences-from-fireevent), because `userEvent` more accurately simulates user interactions in a browser and makes the test more reliable in catching unintended event handler behavior. + +If, while using the Testing Library, your tests trigger a warning in the console from React ("Warning: An update to Component inside a test was not wrapped in act(...)"), first check out the library author's [blog post](https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning) about this. Depending on your situation, you probably will have to [`waitFor`](https://testing-library.com/docs/dom-testing-library/api-async/) for something in your test and ensure asynchronous side-effects have completed: ```js -import { fireEvent, wait } from '@testing-library/react'; +import { userEvent, waitFor } from '@testing-library/react'; ... -await wait(() => fireEvent.click(getByText('Delete'))); +await waitFor(() => userEvent.click(getByText('Delete'))); ... ``` From f4ef0dd211f16541d0e2518f61487869529168b5 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Wed, 6 Nov 2024 15:38:30 -0800 Subject: [PATCH 2/5] Fix typo --- docs/development-guide/08-testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development-guide/08-testing.md b/docs/development-guide/08-testing.md index 61decd743b4..df07bc47f1a 100644 --- a/docs/development-guide/08-testing.md +++ b/docs/development-guide/08-testing.md @@ -64,7 +64,7 @@ describe("My component", () => { Handling events such as clicks is a little more involved: ```js -import userEvent from '@testing-library/user-event'; +import { userEvent } from '@testing-library/user-event'; import { renderWithTheme } from "src/utilities/testHelpers"; import Component from "./wherever"; From 3ef0637a0805bc93b106becf7fb508214d43d5d9 Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Thu, 7 Nov 2024 20:33:15 -0800 Subject: [PATCH 3/5] Address feedback; also further clean up linting issues the doc --- docs/development-guide/08-testing.md | 41 ++++++++++++++++------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/docs/development-guide/08-testing.md b/docs/development-guide/08-testing.md index df07bc47f1a..a97bf29a06c 100644 --- a/docs/development-guide/08-testing.md +++ b/docs/development-guide/08-testing.md @@ -6,38 +6,38 @@ The unit tests for Cloud Manager are written in Typescript using the [Vitest](ht To run tests, first build the **api-v4** package: -``` +```shell yarn install:all && yarn workspace @linode/api-v4 build ``` Then you can start the tests: -``` +```shell yarn test ``` Or you can run the tests in watch mode with: -``` +```shell yarn test:watch ``` To run a specific file or files in a directory: -``` +```shell yarn test myFile.test.tsx yarn test src/some-folder ``` Vitest has built-in pattern matching, so you can also do things like run all tests whose filename contains "Linode" with: -``` +```shell yarn test linode ``` To run a test in debug mode, add a `debugger` breakpoint inside one of the test cases, then run: -``` +```shell yarn workspace linode-manager run test:debug ``` @@ -80,17 +80,9 @@ describe("My component", () => { }); ``` -Note that we recommend using `userEvent` rather than `fireEvent` where possible. This is a [React Testing Library best practice](https://testing-library.com/docs/user-event/intro#differences-from-fireevent), because `userEvent` more accurately simulates user interactions in a browser and makes the test more reliable in catching unintended event handler behavior. +We recommend using `userEvent` rather than `fireEvent` where possible. This is a [React Testing Library best practice](https://testing-library.com/docs/user-event/intro#differences-from-fireevent), because `userEvent` more accurately simulates user interactions in a browser and makes the test more reliable in catching unintended event handler behavior. -If, while using the Testing Library, your tests trigger a warning in the console from React ("Warning: An update to Component inside a test was not wrapped in act(...)"), first check out the library author's [blog post](https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning) about this. Depending on your situation, you probably will have to [`waitFor`](https://testing-library.com/docs/dom-testing-library/api-async/) for something in your test and ensure asynchronous side-effects have completed: - -```js -import { userEvent, waitFor } from '@testing-library/react'; - -... -await waitFor(() => userEvent.click(getByText('Delete'))); -... -``` +If, while using the Testing Library, your tests trigger a warning in the console from React ("Warning: An update to Component inside a test was not wrapped in act(...)"), first check out the library author's [blog post](https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning) about this. Depending on your situation, you probably will have to use [`findBy`](https://testing-library.com/docs/dom-testing-library/api-async/#findby-queries) or [`waitFor`](https://testing-library.com/docs/dom-testing-library/api-async/) for something in your test to ensure asynchronous side-effects have completed. ### Mocking @@ -110,7 +102,9 @@ vi.mock('@linode/api-v4/lib/kubernetes', async () => { Some components, such as our ActionMenu, don't lend themselves well to unit testing (they often have complex DOM structures from MUI and it's hard to target). We have mocks for most of these components in a `__mocks__` directory adjacent to their respective components. To make use of these, just tell Vitest to use the mock: +```js vi.mock('src/components/ActionMenu/ActionMenu'); +``` Any ``s rendered by the test will be simplified versions that are easier to work with. @@ -159,6 +153,7 @@ We use [Cypress](https://cypress.io) for end-to-end testing. Test files are foun * Select a reasonable expiry time (avoid "Never") and make sure that every permission is set to "Read/Write". 3. Set the `MANAGER_OAUTH` environment variable in your `.env` file using your new personal access token. * Example of `.env` addition: + ```shell # Manager OAuth token for Cypress tests: # (The real token will be a 64-digit string of hexadecimals). @@ -176,6 +171,7 @@ We use [Cypress](https://cypress.io) for end-to-end testing. Test files are foun Cloud Manager UI tests can be configured using environment variables, which can be defined in `packages/manager/.env` or specified when running Cypress. ##### Cypress Environment Variables + These environment variables are used by Cypress out-of-the-box to override the default configuration. Cypress exposes many other options that can be configured with environment variables, but the items listed below are particularly relevant for Cloud Manager testing. More information can be found at [docs.cypress.io](https://docs.cypress.io/guides/guides/environment-variables). | Environment Variable | Description | Example | Default | @@ -183,9 +179,11 @@ These environment variables are used by Cypress out-of-the-box to override the d | `CYPRESS_BASE_URL` | URL to Cloud Manager environment for tests | `https://cloud.linode.com` | `http://localhost:3000` | ##### Cloud Manager-specific Environment Variables + These environment variables are specific to Cloud Manager UI tests. They can be distinguished from out-of-the-box Cypress environment variables by their `CY_TEST_` prefix. ###### General + Environment variables related to the general operation of the Cloud Manager Cypress tests. | Environment Variable | Description | Example | Default | @@ -194,6 +192,7 @@ Environment variables related to the general operation of the Cloud Manager Cypr | `CY_TEST_TAGS` | Query identifying tests that should run by specifying allowed and disallowed tags. | `method:e2e` | Unset; all tests run by default | ###### Overriding Behavior + These environment variables can be used to override some behaviors of Cloud Manager's UI tests. This can be useful when testing Cloud Manager for nonstandard or work-in-progress functionality. | Environment Variable | Description | Example | Default | @@ -202,6 +201,7 @@ These environment variables can be used to override some behaviors of Cloud Mana | `CY_TEST_FEATURE_FLAGS` | JSON string containing feature flag data | `{}` | Unset; feature flag data is not overridden | ###### Run Splitting + These environment variables facilitate splitting the Cypress run between multiple runners without the use of any third party services. This can be useful for improving Cypress test performance in some circumstances. For additional performance gains, an optional test weights file can be specified using `CY_TEST_SPLIT_RUN_WEIGHTS` (see `CY_TEST_GENWEIGHTS` to generate test weights). | Environment Variable | Description | Example | Default | @@ -212,6 +212,7 @@ These environment variables facilitate splitting the Cypress run between multipl | `CY_TEST_SPLIT_RUN_WEIGHTS` | Path to test weights file | `./weights.json` | Unset; disabled by default | ###### Development, Logging, and Reporting + Environment variables related to Cypress logging and reporting, as well as report generation. | Environment Variable | Description | Example | Default | @@ -224,6 +225,7 @@ Environment variables related to Cypress logging and reporting, as well as repor | `CY_TEST_GENWEIGHTS` | Generate and output test weights to the given path | `./weights.json` | Unset; disabled by default | ###### Performance + Environment variables that can be used to improve test performance in some scenarios. | Environment Variable | Description | Example | Default | @@ -235,6 +237,7 @@ Environment variables that can be used to improve test performance in some scena 1. Look here for [Cypress Best Practices](https://docs.cypress.io/guides/references/best-practices) 2. Test Example: + ```tsx /* this test will not pass on cloud manager. it is only intended to show correct test structure, syntax, @@ -295,13 +298,15 @@ Environment variables that can be used to improve test performance in some scena }); }); ``` + 3. How to use intercepts: + ```tsx // stub response syntax: cy.intercept('POST', ‘/path’, {response}) or cy.intercept(‘/path’, (req) => { req.reply({response})}).as('something'); - // edit and end response syntax: + // edit and end response syntax: cy.intercept('GET', ‘/path’, (req) => { req.send({edit: something})}).as('something'); - // edit request syntax: + // edit request syntax: cy.intercept('POST', ‘/path’, (req) => { req.body.storyName = 'some name'; req.continue().as('something'); // use alias syntax: From 0cee04baa237a2b769cf5f294088114cda6c340e Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Fri, 8 Nov 2024 09:02:58 -0800 Subject: [PATCH 4/5] Fix a bad test that was not following good practices --- .../CopyTooltip/CopyTooltip.test.tsx | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/manager/src/components/CopyTooltip/CopyTooltip.test.tsx b/packages/manager/src/components/CopyTooltip/CopyTooltip.test.tsx index e18c0dc546a..d90b014d13c 100644 --- a/packages/manager/src/components/CopyTooltip/CopyTooltip.test.tsx +++ b/packages/manager/src/components/CopyTooltip/CopyTooltip.test.tsx @@ -1,4 +1,3 @@ -import { act, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import * as React from 'react'; @@ -11,32 +10,29 @@ import type { CopyTooltipProps } from './CopyTooltip'; const mockText = 'Hello world'; const defaultProps: CopyTooltipProps = { + onClickCallback: vi.fn(), text: mockText, }; describe('CopyTooltip', () => { it('should render the copy icon button and tooltip', async () => { - const { getByLabelText, getByRole } = renderWithTheme( + const { findByRole, getByLabelText } = renderWithTheme( ); const copyIconButton = getByLabelText(`Copy ${mockText} to clipboard`); - await act(() => userEvent.hover(copyIconButton)); + await userEvent.hover(copyIconButton); - await waitFor(() => { - const copyTooltip = getByRole('tooltip'); - expect(copyTooltip).toBeInTheDocument(); - expect(copyTooltip).toHaveTextContent('Copy'); - }); + const copyTooltip = await findByRole('tooltip'); + expect(copyTooltip).toBeInTheDocument(); + expect(copyTooltip).toHaveTextContent('Copy'); await userEvent.click(copyIconButton); - await waitFor(() => { - const copiedTooltip = getByRole('tooltip', {}); - expect(copiedTooltip).toBeInTheDocument(); - expect(copiedTooltip).toHaveTextContent('Copied!'); - }); + const copiedTooltip = await findByRole('tooltip'); + expect(copiedTooltip).toBeInTheDocument(); + expect(copiedTooltip).toHaveTextContent('Copied!'); }); it('should render text with the copyableText property', async () => { @@ -80,7 +76,7 @@ describe('CopyTooltip', () => { expect(getByText('•••••••••••')).toBeVisible(); expect(queryByText(mockText)).toBeNull(); - await act(() => userEvent.click(visibilityToggle)); + await userEvent.click(visibilityToggle); // Text should be unmasked expect(getByText('Hello world')).toBeVisible(); From 5d4231f4c7a66695cc10d93f166c1e6659b3504a Mon Sep 17 00:00:00 2001 From: mjac0bs Date: Fri, 8 Nov 2024 09:29:40 -0800 Subject: [PATCH 5/5] Added changeset: Update developer docs on unit testing user events --- .../manager/.changeset/pr-11221-changed-1731086980327.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 packages/manager/.changeset/pr-11221-changed-1731086980327.md diff --git a/packages/manager/.changeset/pr-11221-changed-1731086980327.md b/packages/manager/.changeset/pr-11221-changed-1731086980327.md new file mode 100644 index 00000000000..5e42e2805a3 --- /dev/null +++ b/packages/manager/.changeset/pr-11221-changed-1731086980327.md @@ -0,0 +1,5 @@ +--- +"@linode/manager": Changed +--- + +Update developer docs on unit testing user events ([#11221](https://github.com/linode/manager/pull/11221))