chore: revamp tests

Parkreiner · Parkreiner · commit 923d080dc105 · 2023-11-03T14:46:42.000Z
diff --git a/site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.test.tsx b/site/src/pages/CreateWorkspacePage/useWorkspaceDuplication.test.tsx
@@ -1,47 +1,36 @@
-import { waitFor, screen } from "@testing-library/react";
-import userEvent from "@testing-library/user-event";
-import { createMemoryRouter } from "react-router-dom";
-import { renderWithRouter } from "testHelpers/renderHelpers";
-
+import { waitFor } from "@testing-library/react";
 import * as M from "../../testHelpers/entities";
 import { type Workspace } from "api/typesGenerated";
 import { useWorkspaceDuplication } from "./useWorkspaceDuplication";
 import { MockWorkspace } from "testHelpers/entities";
 import CreateWorkspacePage from "./CreateWorkspacePage";
+import { renderHookWithAuth } from "testHelpers/renderHelpers";
 
-// Tried really hard to get these tests working with RTL's renderHook, but I
-// kept running into weird function mismatches, mostly stemming from the fact
-// that React Router's RouteProvider does not accept children, meaning that you
-// can't inject values into it with renderHook's wrapper
-function WorkspaceMock({ workspace }: { workspace?: Workspace }) {
-  const { duplicateWorkspace, isDuplicationReady } =
-    useWorkspaceDuplication(workspace);
-
-  return (
-    <button onClick={duplicateWorkspace} disabled={!isDuplicationReady}>
-      Click me!
-    </button>
-  );
-}
-
-function renderInMemory(workspace?: Workspace) {
-  const router = createMemoryRouter([
-    { path: "/", element: <WorkspaceMock workspace={workspace} /> },
+function render(workspace?: Workspace) {
+  return renderHookWithAuth(
+    ({ workspace }: { workspace?: Workspace }) => {
+      return useWorkspaceDuplication(workspace);
+    },
     {
-      path: "/templates/:template/workspace",
-      element: <CreateWorkspacePage />,
+      initialProps: { workspace },
+      extraRoutes: [
+        {
+          path: "/templates/:template/workspace",
+          element: <CreateWorkspacePage />,
+        },
+      ],
     },
-  ]);
-
-  return renderWithRouter(router);
+  );
 }
 
+type RenderResult = Awaited<ReturnType<typeof render>>;
+
 async function performNavigation(
-  button: HTMLElement,
-  router: ReturnType<typeof createMemoryRouter>,
+  result: RenderResult["result"],
+  router: RenderResult["router"],
 ) {
-  await waitFor(() => expect(button).not.toBeDisabled());
-  await userEvent.click(button);
+  await waitFor(() => expect(result.current.isDuplicationReady).toBe(true));
+  result.current.duplicateWorkspace();
 
   return waitFor(() => {
     expect(router.state.location.pathname).toEqual(
@@ -52,34 +41,30 @@ async function performNavigation(
 
 describe(`${useWorkspaceDuplication.name}`, () => {
   it("Will never be ready when there is no workspace passed in", async () => {
-    const { rootComponent, rerender } = renderInMemory(undefined);
-    const button = await screen.findByRole("button");
-    expect(button).toBeDisabled();
+    const { result, rerender } = await render(undefined);
+    expect(result.current.isDuplicationReady).toBe(false);
 
     for (let i = 0; i < 10; i++) {
-      rerender(rootComponent);
-      expect(button).toBeDisabled();
+      rerender({ workspace: undefined });
+      expect(result.current.isDuplicationReady).toBe(false);
     }
   });
 
   it("Will become ready when workspace is provided and build params are successfully fetched", async () => {
-    renderInMemory(MockWorkspace);
-    const button = await screen.findByRole("button");
+    const { result } = await render(MockWorkspace);
 
-    expect(button).toBeDisabled();
-    await waitFor(() => expect(button).not.toBeDisabled());
+    expect(result.current.isDuplicationReady).toBe(false);
+    await waitFor(() => expect(result.current.isDuplicationReady).toBe(true));
   });
 
-  it("duplicateWorkspace navigates the user to the workspace creation page", async () => {
-    const { router } = renderInMemory(MockWorkspace);
-    const button = await screen.findByRole("button");
-    await performNavigation(button, router);
+  it("Is able to navigate the user to the workspace creation page", async () => {
+    const { result, router } = await render(MockWorkspace);
+    await performNavigation(result, router);
   });
 
   test("Navigating populates the URL search params with the workspace's build params", async () => {
-    const { router } = renderInMemory(MockWorkspace);
-    const button = await screen.findByRole("button");
-    await performNavigation(button, router);
+    const { result, router } = await render(MockWorkspace);
+    await performNavigation(result, router);
 
     const parsedParams = new URLSearchParams(router.state.location.search);
     const mockBuildParams = [
@@ -97,9 +82,8 @@ describe(`${useWorkspaceDuplication.name}`, () => {
   });
 
   test("Navigating appends other necessary metadata to the search params", async () => {
-    const { router } = renderInMemory(MockWorkspace);
-    const button = await screen.findByRole("button");
-    await performNavigation(button, router);
+    const { result, router } = await render(MockWorkspace);
+    await performNavigation(result, router);
 
     const parsedParams = new URLSearchParams(router.state.location.search);
     const extraMetadataEntries = [
diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx
@@ -1,4 +1,9 @@
-import { render as tlRender, screen, waitFor } from "@testing-library/react";
+import {
+  render as tlRender,
+  screen,
+  waitFor,
+  renderHook,
+} from "@testing-library/react";
 import { AppProviders, ThemeProviders } from "App";
 import { DashboardLayout } from "components/Dashboard/DashboardLayout";
 import { TemplateSettingsLayout } from "pages/TemplateSettingsPage/TemplateSettingsLayout";
@@ -10,15 +15,13 @@ import {
 } from "react-router-dom";
 import { RequireAuth } from "../components/RequireAuth/RequireAuth";
 import { MockUser } from "./entities";
-import { ReactNode } from "react";
+import { ReactNode, useState } from "react";
 import { QueryClient } from "react-query";
 
-export const renderWithRouter = (
-  router: ReturnType<typeof createMemoryRouter>,
-) => {
-  // Create one query client for each render isolate it avoid other
-  // tests to be affected
-  const queryClient = new QueryClient({
+function createTestQueryClient() {
+  // Helps create one query client for each test case, to make sure that tests
+  // are isolated and can't affect each other
+  return new QueryClient({
     defaultOptions: {
       queries: {
         retry: false,
@@ -28,16 +31,19 @@ export const renderWithRouter = (
       },
     },
   });
+}
 
-  const rootComponent = (
-    <AppProviders queryClient={queryClient}>
-      <RouterProvider router={router} />
-    </AppProviders>
-  );
+export const renderWithRouter = (
+  router: ReturnType<typeof createMemoryRouter>,
+) => {
+  const queryClient = createTestQueryClient();
 
   return {
-    ...tlRender(rootComponent),
-    rootComponent,
+    ...tlRender(
+      <AppProviders queryClient={queryClient}>
+        <RouterProvider router={router} />
+      </AppProviders>,
+    ),
     router,
   };
 };
@@ -56,7 +62,7 @@ export const render = (element: ReactNode) => {
   );
 };
 
-type RenderWithAuthOptions = {
+export type RenderWithAuthOptions = {
   // The current URL, /workspaces/123
   route?: string;
   // The route path, /workspaces/:workspaceId
@@ -98,6 +104,82 @@ export function renderWithAuth(
   };
 }
 
+type RenderHookWithAuthOptions<Props> = Partial<
+  Readonly<
+    Omit<RenderWithAuthOptions, "children"> & {
+      initialProps: Props;
+    }
+  >
+>;
+
+/**
+ * Custom version of renderHook that is aware of all our App providers.
+ *
+ * Had to do some nasty, cursed things in the implementation to make sure that
+ * the tests using this function remained simple.
+ *
+ * @see {@link https://github.com/coder/coder/pull/10362#discussion_r1380852725}
+ */
+export async function renderHookWithAuth<Result, Props>(
+  render: (initialProps: Props) => Result,
+  {
+    initialProps,
+    path = "/",
+    extraRoutes = [],
+  }: RenderHookWithAuthOptions<Props> = {},
+) {
+  const queryClient = createTestQueryClient();
+
+  // Easy to miss – there's an evil definite assignment via the !
+  let escapedRouter!: ReturnType<typeof createMemoryRouter>;
+
+  const { result, rerender, unmount } = renderHook(render, {
+    initialProps,
+    wrapper: ({ children }) => {
+      /**
+       * Unfortunately, there isn't a way to define the router outside the
+       * wrapper while keeping it aware of children, meaning that we need to
+       * define the router as readonly state in the component instance. This
+       * ensures the value remains stable across all re-renders
+       */
+      // eslint-disable-next-line react-hooks/rules-of-hooks -- This is actually processed as a component; the linter just isn't aware of that
+      const [readonlyStatefulRouter] = useState(() => {
+        return createMemoryRouter([
+          { path, element: <>{children}</> },
+          ...extraRoutes,
+        ]);
+      });
+
+      /**
+       * Leaks the wrapper component's state outside React's render cycles.
+       */
+      escapedRouter = readonlyStatefulRouter;
+
+      return (
+        <AppProviders queryClient={queryClient}>
+          <RouterProvider router={readonlyStatefulRouter} />
+        </AppProviders>
+      );
+    },
+  });
+
+  /**
+   * This is necessary to get around some providers in AppProviders having
+   * conditional rendering and not always rendering their children immediately.
+   *
+   * The hook result won't actually exist until the children defined via wrapper
+   * render in full.
+   */
+  await waitFor(() => expect(result.current).not.toBe(null));
+
+  return {
+    result,
+    rerender,
+    unmount,
+    router: escapedRouter,
+  } as const;
+}
+
 export function renderWithTemplateSettingsLayout(
   element: JSX.Element,
   {
