From 8613133643c5718e2cab3280d94760c1c34ce297 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Sun, 12 May 2024 20:14:24 +0000
Subject: [PATCH 01/12] wip: commit progress on test revamps

---
 site/src/hooks/useClipboard.temp.test.tsx | 172 ++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100644 site/src/hooks/useClipboard.temp.test.tsx

diff --git a/site/src/hooks/useClipboard.temp.test.tsx b/site/src/hooks/useClipboard.temp.test.tsx
new file mode 100644
index 0000000000000..8cdfb5cd04022
--- /dev/null
+++ b/site/src/hooks/useClipboard.temp.test.tsx
@@ -0,0 +1,172 @@
+import { act, renderHook } from "@testing-library/react";
+import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
+import { ThemeProvider } from "contexts/ThemeProvider";
+import {
+  type UseClipboardInput,
+  type UseClipboardResult,
+  useClipboard,
+} from "./useClipboard";
+
+type SetupMockClipboardResult = Readonly<{
+  mockClipboard: Clipboard;
+  getClipboardText: () => string;
+  setClipboardText: (newText: string) => void;
+  setSimulateFailure: (shouldFail: boolean) => void;
+}>;
+
+function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
+  let mockClipboardText = "";
+  let shouldSimulateFailure = false;
+
+  const mockClipboard: Clipboard = {
+    readText: async () => {
+      if (!isSecure) {
+        throw new Error(
+          "Not allowed to access clipboard outside of secure contexts",
+        );
+      }
+
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to read from clipboard");
+      }
+
+      return mockClipboardText;
+    },
+
+    writeText: async (newText) => {
+      if (!isSecure) {
+        throw new Error(
+          "Not allowed to access clipboard outside of secure contexts",
+        );
+      }
+
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to write to clipboard");
+      }
+
+      mockClipboardText = newText;
+    },
+
+    // Don't need these other methods for any of the tests; read and write are
+    // both synchronous and slower than the promise-based methods, so ideally
+    // we won't ever need to call them in the hook logic
+    addEventListener: jest.fn(),
+    removeEventListener: jest.fn(),
+    dispatchEvent: jest.fn(),
+    read: jest.fn(),
+    write: jest.fn(),
+  };
+
+  return {
+    mockClipboard,
+    getClipboardText: () => mockClipboardText,
+    setClipboardText: (newText) => {
+      mockClipboardText = newText;
+    },
+    setSimulateFailure: (newShouldFailValue) => {
+      shouldSimulateFailure = newShouldFailValue;
+    },
+  };
+}
+
+function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
+  return renderHook<UseClipboardResult, TInput>(
+    (props) => useClipboard(props),
+    {
+      initialProps: inputs,
+      wrapper: ({ children }) => (
+        // Need ThemeProvider because GlobalSnackbar uses theme
+        <ThemeProvider>
+          {children}
+          <GlobalSnackbar />
+        </ThemeProvider>
+      ),
+    },
+  );
+}
+
+const secureContextValues: readonly boolean[] = [true, false];
+const originalNavigator = window.navigator;
+const originalExecCommand = global.document.execCommand;
+
+// Not a big fan of describe.each most of the time, but since we need to test
+// the exact same test cases against different inputs, and we want them to run
+// as sequentially as possible to minimize flakes, they make sense here
+describe.each(secureContextValues)("useClipboard - secure: %j", (context) => {
+  const {
+    mockClipboard,
+    getClipboardText,
+    setClipboardText,
+    setSimulateFailure,
+  } = setupMockClipboard(context);
+
+  beforeEach(() => {
+    jest.useFakeTimers();
+    jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
+      ...originalNavigator,
+      clipboard: mockClipboard,
+    }));
+
+    global.document.execCommand = jest.fn(() => {
+      const dummyInput = document.querySelector("input[data-testid=dummy]");
+      const inputIsFocused =
+        dummyInput instanceof HTMLInputElement &&
+        document.activeElement === dummyInput;
+
+      let copySuccessful = false;
+      if (inputIsFocused) {
+        setClipboardText(dummyInput.value);
+        copySuccessful = true;
+      }
+
+      return copySuccessful;
+    });
+  });
+
+  afterEach(() => {
+    jest.runAllTimers();
+    jest.useRealTimers();
+    jest.resetAllMocks();
+    global.document.execCommand = originalExecCommand;
+  });
+
+  const assertClipboardTextUpdate = async (
+    result: ReturnType<typeof renderUseClipboard>["result"],
+    textToCheck: string,
+  ): Promise<void> => {
+    await act(() => result.current.copyToClipboard());
+    expect(result.current.showCopiedSuccess).toBe(true);
+
+    const clipboardText = getClipboardText();
+    expect(clipboardText).toEqual(textToCheck);
+  };
+
+  it("Copies the current text to the user's clipboard", async () => {
+    const textToCopy = "dogs";
+    const { result } = renderUseClipboard({ textToCopy });
+    await assertClipboardTextUpdate(result, textToCopy);
+  });
+
+  it("Should indicate to components not to show successful copy after a set period of time", async () => {
+    const textToCopy = "cats";
+    const { result } = renderUseClipboard({ textToCopy });
+    await assertClipboardTextUpdate(result, textToCopy);
+
+    await jest.runAllTimersAsync();
+    expect(result.current.showCopiedSuccess).toBe(false);
+  });
+
+  it("Should notify the user of an error using the provided callback", async () => {
+    const textToCopy = "birds";
+    const onError = jest.fn();
+    const { result } = renderUseClipboard({ textToCopy, onError });
+
+    setSimulateFailure(true);
+    await act(() => result.current.copyToClipboard());
+    expect(onError).toBeCalled();
+  });
+
+  it.skip("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
+    expect.hasAssertions();
+  });
+});

From 18cf563a9b3baebd75d0ecf4aad50f0a513e1b58 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Sun, 12 May 2024 20:55:28 +0000
Subject: [PATCH 02/12] fix: update existing tests to new format

---
 site/src/hooks/useClipboard.temp.test.tsx | 84 ++++++++++++++++-------
 site/src/hooks/useClipboard.ts            |  2 +-
 2 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/site/src/hooks/useClipboard.temp.test.tsx b/site/src/hooks/useClipboard.temp.test.tsx
index 8cdfb5cd04022..24a9434f857ee 100644
--- a/site/src/hooks/useClipboard.temp.test.tsx
+++ b/site/src/hooks/useClipboard.temp.test.tsx
@@ -4,13 +4,24 @@ import { ThemeProvider } from "contexts/ThemeProvider";
 import {
   type UseClipboardInput,
   type UseClipboardResult,
+  COPY_FAILED_MESSAGE,
   useClipboard,
 } from "./useClipboard";
 
+// execCommand is the workaround for copying text to the clipboard on HTTP-only
+// connections
+const originalExecCommand = global.document.execCommand;
+const originalNavigator = window.navigator;
+
+// Need to mock console.error because we deliberately need to trigger errors in
+// the code to assert that it can recover from them, but we also don't want them
+// logged if they're expected
+const originalConsoleError = console.error;
+
 type SetupMockClipboardResult = Readonly<{
   mockClipboard: Clipboard;
+  mockExecCommand: typeof originalExecCommand;
   getClipboardText: () => string;
-  setClipboardText: (newText: string) => void;
   setSimulateFailure: (shouldFail: boolean) => void;
 }>;
 
@@ -60,12 +71,31 @@ function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
   return {
     mockClipboard,
     getClipboardText: () => mockClipboardText,
-    setClipboardText: (newText) => {
-      mockClipboardText = newText;
-    },
     setSimulateFailure: (newShouldFailValue) => {
       shouldSimulateFailure = newShouldFailValue;
     },
+    mockExecCommand: (commandId) => {
+      if (commandId !== "copy") {
+        return false;
+      }
+
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to execute command 'copy'");
+      }
+
+      const dummyInput = document.querySelector("input[data-testid=dummy]");
+      const inputIsFocused =
+        dummyInput instanceof HTMLInputElement &&
+        document.activeElement === dummyInput;
+
+      let copySuccessful = false;
+      if (inputIsFocused) {
+        mockClipboardText = dummyInput.value;
+        copySuccessful = true;
+      }
+
+      return copySuccessful;
+    },
   };
 }
 
@@ -86,57 +116,61 @@ function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
 }
 
 const secureContextValues: readonly boolean[] = [true, false];
-const originalNavigator = window.navigator;
-const originalExecCommand = global.document.execCommand;
 
 // Not a big fan of describe.each most of the time, but since we need to test
 // the exact same test cases against different inputs, and we want them to run
 // as sequentially as possible to minimize flakes, they make sense here
-describe.each(secureContextValues)("useClipboard - secure: %j", (context) => {
+describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
   const {
     mockClipboard,
+    mockExecCommand,
     getClipboardText,
-    setClipboardText,
     setSimulateFailure,
-  } = setupMockClipboard(context);
+  } = setupMockClipboard(isSecure);
 
   beforeEach(() => {
     jest.useFakeTimers();
+    global.document.execCommand = mockExecCommand;
     jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
       ...originalNavigator,
       clipboard: mockClipboard,
     }));
 
-    global.document.execCommand = jest.fn(() => {
-      const dummyInput = document.querySelector("input[data-testid=dummy]");
-      const inputIsFocused =
-        dummyInput instanceof HTMLInputElement &&
-        document.activeElement === dummyInput;
+    console.error = (errorValue, ...rest) => {
+      const canIgnore =
+        errorValue instanceof Error &&
+        errorValue.message === COPY_FAILED_MESSAGE;
 
-      let copySuccessful = false;
-      if (inputIsFocused) {
-        setClipboardText(dummyInput.value);
-        copySuccessful = true;
+      if (!canIgnore) {
+        originalConsoleError(errorValue, ...rest);
       }
-
-      return copySuccessful;
-    });
+    };
   });
 
   afterEach(() => {
     jest.runAllTimers();
     jest.useRealTimers();
     jest.resetAllMocks();
+
+    console.error = originalConsoleError;
     global.document.execCommand = originalExecCommand;
   });
 
-  const assertClipboardTextUpdate = async (
+  const assertClipboardUpdateLifecycle = async (
     result: ReturnType<typeof renderUseClipboard>["result"],
     textToCheck: string,
   ): Promise<void> => {
     await act(() => result.current.copyToClipboard());
     expect(result.current.showCopiedSuccess).toBe(true);
 
+    // Because of timing trickery, any timeouts for flipping the copy status
+    // back to false will trigger before the test can complete. This will never
+    // be an issue in the real world, but it will kick up 'act' warnings in the
+    // console, which makes tests more annoying. Just wait for them to finish up
+    // to avoid anything from being logged, but note that the value of
+    // showCopiedSuccess will become false after this
+    await act(() => jest.runAllTimersAsync());
+
     const clipboardText = getClipboardText();
     expect(clipboardText).toEqual(textToCheck);
   };
@@ -144,15 +178,13 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (context) => {
   it("Copies the current text to the user's clipboard", async () => {
     const textToCopy = "dogs";
     const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardTextUpdate(result, textToCopy);
+    await assertClipboardUpdateLifecycle(result, textToCopy);
   });
 
   it("Should indicate to components not to show successful copy after a set period of time", async () => {
     const textToCopy = "cats";
     const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardTextUpdate(result, textToCopy);
-
-    await jest.runAllTimersAsync();
+    await assertClipboardUpdateLifecycle(result, textToCopy);
     expect(result.current.showCopiedSuccess).toBe(false);
   });
 
diff --git a/site/src/hooks/useClipboard.ts b/site/src/hooks/useClipboard.ts
index 83ec8283ed710..fa5bc5474e02a 100644
--- a/site/src/hooks/useClipboard.ts
+++ b/site/src/hooks/useClipboard.ts
@@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from "react";
 import { displayError } from "components/GlobalSnackbar/utils";
 
 const CLIPBOARD_TIMEOUT_MS = 1_000;
-const COPY_FAILED_MESSAGE = "Failed to copy text to clipboard";
+export const COPY_FAILED_MESSAGE = "Failed to copy text to clipboard";
 
 export type UseClipboardInput = Readonly<{
   textToCopy: string;

From 7c6a8e69453eb3df043b01c83a1ecdcf5f45efcf Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Sun, 12 May 2024 21:03:38 +0000
Subject: [PATCH 03/12] chore: add test case for global snackbar

---
 site/src/hooks/useClipboard.temp.test.tsx | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/site/src/hooks/useClipboard.temp.test.tsx b/site/src/hooks/useClipboard.temp.test.tsx
index 24a9434f857ee..77ed202141dfc 100644
--- a/site/src/hooks/useClipboard.temp.test.tsx
+++ b/site/src/hooks/useClipboard.temp.test.tsx
@@ -1,4 +1,4 @@
-import { act, renderHook } from "@testing-library/react";
+import { act, renderHook, screen } from "@testing-library/react";
 import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
 import { ThemeProvider } from "contexts/ThemeProvider";
 import {
@@ -166,8 +166,8 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     // Because of timing trickery, any timeouts for flipping the copy status
     // back to false will trigger before the test can complete. This will never
     // be an issue in the real world, but it will kick up 'act' warnings in the
-    // console, which makes tests more annoying. Just wait for them to finish up
-    // to avoid anything from being logged, but note that the value of
+    // console, which makes tests more annoying. Just waiting for them to finish
+    // up to avoid anything from being logged, but note that the value of
     // showCopiedSuccess will become false after this
     await act(() => jest.runAllTimersAsync());
 
@@ -198,7 +198,14 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     expect(onError).toBeCalled();
   });
 
-  it.skip("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
-    expect.hasAssertions();
+  it("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
+    const textToCopy = "crow";
+    const { result } = renderUseClipboard({ textToCopy });
+
+    setSimulateFailure(true);
+    await act(() => result.current.copyToClipboard());
+
+    const errorMessageNode = screen.queryByText(COPY_FAILED_MESSAGE);
+    expect(errorMessageNode).not.toBeNull();
   });
 });

From 66a350207f99cab66b4125e4831c63ebb4e043a8 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Sun, 12 May 2024 21:06:24 +0000
Subject: [PATCH 04/12] refactor: consolidate files

---
 site/src/hooks/useClipboard.temp.test.tsx | 211 -----------------
 site/src/hooks/useClipboard.test.tsx      | 269 +++++++++++-----------
 2 files changed, 135 insertions(+), 345 deletions(-)
 delete mode 100644 site/src/hooks/useClipboard.temp.test.tsx

diff --git a/site/src/hooks/useClipboard.temp.test.tsx b/site/src/hooks/useClipboard.temp.test.tsx
deleted file mode 100644
index 77ed202141dfc..0000000000000
--- a/site/src/hooks/useClipboard.temp.test.tsx
+++ /dev/null
@@ -1,211 +0,0 @@
-import { act, renderHook, screen } from "@testing-library/react";
-import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
-import { ThemeProvider } from "contexts/ThemeProvider";
-import {
-  type UseClipboardInput,
-  type UseClipboardResult,
-  COPY_FAILED_MESSAGE,
-  useClipboard,
-} from "./useClipboard";
-
-// execCommand is the workaround for copying text to the clipboard on HTTP-only
-// connections
-const originalExecCommand = global.document.execCommand;
-const originalNavigator = window.navigator;
-
-// Need to mock console.error because we deliberately need to trigger errors in
-// the code to assert that it can recover from them, but we also don't want them
-// logged if they're expected
-const originalConsoleError = console.error;
-
-type SetupMockClipboardResult = Readonly<{
-  mockClipboard: Clipboard;
-  mockExecCommand: typeof originalExecCommand;
-  getClipboardText: () => string;
-  setSimulateFailure: (shouldFail: boolean) => void;
-}>;
-
-function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
-  let mockClipboardText = "";
-  let shouldSimulateFailure = false;
-
-  const mockClipboard: Clipboard = {
-    readText: async () => {
-      if (!isSecure) {
-        throw new Error(
-          "Not allowed to access clipboard outside of secure contexts",
-        );
-      }
-
-      if (shouldSimulateFailure) {
-        throw new Error("Failed to read from clipboard");
-      }
-
-      return mockClipboardText;
-    },
-
-    writeText: async (newText) => {
-      if (!isSecure) {
-        throw new Error(
-          "Not allowed to access clipboard outside of secure contexts",
-        );
-      }
-
-      if (shouldSimulateFailure) {
-        throw new Error("Failed to write to clipboard");
-      }
-
-      mockClipboardText = newText;
-    },
-
-    // Don't need these other methods for any of the tests; read and write are
-    // both synchronous and slower than the promise-based methods, so ideally
-    // we won't ever need to call them in the hook logic
-    addEventListener: jest.fn(),
-    removeEventListener: jest.fn(),
-    dispatchEvent: jest.fn(),
-    read: jest.fn(),
-    write: jest.fn(),
-  };
-
-  return {
-    mockClipboard,
-    getClipboardText: () => mockClipboardText,
-    setSimulateFailure: (newShouldFailValue) => {
-      shouldSimulateFailure = newShouldFailValue;
-    },
-    mockExecCommand: (commandId) => {
-      if (commandId !== "copy") {
-        return false;
-      }
-
-      if (shouldSimulateFailure) {
-        throw new Error("Failed to execute command 'copy'");
-      }
-
-      const dummyInput = document.querySelector("input[data-testid=dummy]");
-      const inputIsFocused =
-        dummyInput instanceof HTMLInputElement &&
-        document.activeElement === dummyInput;
-
-      let copySuccessful = false;
-      if (inputIsFocused) {
-        mockClipboardText = dummyInput.value;
-        copySuccessful = true;
-      }
-
-      return copySuccessful;
-    },
-  };
-}
-
-function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
-  return renderHook<UseClipboardResult, TInput>(
-    (props) => useClipboard(props),
-    {
-      initialProps: inputs,
-      wrapper: ({ children }) => (
-        // Need ThemeProvider because GlobalSnackbar uses theme
-        <ThemeProvider>
-          {children}
-          <GlobalSnackbar />
-        </ThemeProvider>
-      ),
-    },
-  );
-}
-
-const secureContextValues: readonly boolean[] = [true, false];
-
-// Not a big fan of describe.each most of the time, but since we need to test
-// the exact same test cases against different inputs, and we want them to run
-// as sequentially as possible to minimize flakes, they make sense here
-describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
-  const {
-    mockClipboard,
-    mockExecCommand,
-    getClipboardText,
-    setSimulateFailure,
-  } = setupMockClipboard(isSecure);
-
-  beforeEach(() => {
-    jest.useFakeTimers();
-    global.document.execCommand = mockExecCommand;
-    jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
-      ...originalNavigator,
-      clipboard: mockClipboard,
-    }));
-
-    console.error = (errorValue, ...rest) => {
-      const canIgnore =
-        errorValue instanceof Error &&
-        errorValue.message === COPY_FAILED_MESSAGE;
-
-      if (!canIgnore) {
-        originalConsoleError(errorValue, ...rest);
-      }
-    };
-  });
-
-  afterEach(() => {
-    jest.runAllTimers();
-    jest.useRealTimers();
-    jest.resetAllMocks();
-
-    console.error = originalConsoleError;
-    global.document.execCommand = originalExecCommand;
-  });
-
-  const assertClipboardUpdateLifecycle = async (
-    result: ReturnType<typeof renderUseClipboard>["result"],
-    textToCheck: string,
-  ): Promise<void> => {
-    await act(() => result.current.copyToClipboard());
-    expect(result.current.showCopiedSuccess).toBe(true);
-
-    // Because of timing trickery, any timeouts for flipping the copy status
-    // back to false will trigger before the test can complete. This will never
-    // be an issue in the real world, but it will kick up 'act' warnings in the
-    // console, which makes tests more annoying. Just waiting for them to finish
-    // up to avoid anything from being logged, but note that the value of
-    // showCopiedSuccess will become false after this
-    await act(() => jest.runAllTimersAsync());
-
-    const clipboardText = getClipboardText();
-    expect(clipboardText).toEqual(textToCheck);
-  };
-
-  it("Copies the current text to the user's clipboard", async () => {
-    const textToCopy = "dogs";
-    const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardUpdateLifecycle(result, textToCopy);
-  });
-
-  it("Should indicate to components not to show successful copy after a set period of time", async () => {
-    const textToCopy = "cats";
-    const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardUpdateLifecycle(result, textToCopy);
-    expect(result.current.showCopiedSuccess).toBe(false);
-  });
-
-  it("Should notify the user of an error using the provided callback", async () => {
-    const textToCopy = "birds";
-    const onError = jest.fn();
-    const { result } = renderUseClipboard({ textToCopy, onError });
-
-    setSimulateFailure(true);
-    await act(() => result.current.copyToClipboard());
-    expect(onError).toBeCalled();
-  });
-
-  it("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
-    const textToCopy = "crow";
-    const { result } = renderUseClipboard({ textToCopy });
-
-    setSimulateFailure(true);
-    await act(() => result.current.copyToClipboard());
-
-    const errorMessageNode = screen.queryByText(COPY_FAILED_MESSAGE);
-    expect(errorMessageNode).not.toBeNull();
-  });
-});
diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 5ddbed3f8cc12..12320e10acd38 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -1,129 +1,121 @@
-import { act, renderHook } from "@testing-library/react";
+/**
+ * @file The test setup for this file is a little funky because of how React
+ * Testing Library works.
+ *
+ * When you call user.setup to make a new user session, it will make a mock
+ * clipboard instance that will always succeed. It also can't be removed after
+ * it's been added. This actually makes testing useClipboard impossible to test
+ * properly because any call to user.setup immediately pollutes the tests with
+ * false negatives. Even if something should fail, it won't.
+ */
+import { act, renderHook, screen } from "@testing-library/react";
 import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
 import { ThemeProvider } from "contexts/ThemeProvider";
 import {
   type UseClipboardInput,
   type UseClipboardResult,
+  COPY_FAILED_MESSAGE,
   useClipboard,
 } from "./useClipboard";
 
-describe(useClipboard.name, () => {
-  describe("HTTP (non-secure) connections", () => {
-    scheduleClipboardTests({ isHttps: false });
-  });
-
-  describe("HTTPS (secure/default) connections", () => {
-    scheduleClipboardTests({ isHttps: true });
-  });
-});
-
-/**
- * @file This is a very weird test setup.
- *
- * There are two main things that it's fighting against to insure that the
- * clipboard functionality is working as expected:
- * 1. userEvent.setup's default global behavior
- * 2. The fact that we need to reuse the same set of test cases for two separate
- *    contexts (secure and insecure), each with their own version of global
- *    state.
- *
- * The goal of this file is to provide a shared set of test behavior that can
- * be imported into two separate test files (one for HTTP, one for HTTPS),
- * without any risk of global state conflicts.
- *
- * ---
- * For (1), normally you could call userEvent.setup to enable clipboard mocking,
- * but userEvent doesn't expose a teardown function. It also modifies the global
- * scope for the whole test file, so enabling just one userEvent session will
- * make a mock clipboard exist for all other tests, even though you didn't tell
- * them to set up a session. The mock also assumes that the clipboard API will
- * always be available, which is not true on HTTP-only connections
- *
- * Since these tests need to split hairs and differentiate between HTTP and
- * HTTPS connections, setting up a single userEvent is disastrous. It will make
- * all the tests pass, even if they shouldn't. Have to avoid that by creating a
- * custom clipboard mock.
- *
- * ---
- * For (2), we're fighting against Jest's default behavior, which is to treat
- * the test file as the main boundary for test environments, with each test case
- * able to run in parallel. That works if you have one single global state, but
- * we need two separate versions of the global state, while repeating the exact
- * same test cases for each one.
- *
- * If both tests were to be placed in the same file, Jest would not isolate them
- * and would let their setup steps interfere with each other. This leads to one
- * of two things:
- * 1. One of the global mocks overrides the other, making it so that one
- *    connection type always fails
- * 2. The two just happen not to conflict each other, through some convoluted
- *    order of operations involving closure, but you have no idea why the code
- *    is working, and it's impossible to debug.
- */
-type MockClipboardEscapeHatches = Readonly<{
-  getMockText: () => string;
-  setMockText: (newText: string) => void;
-  simulateFailure: boolean;
-  setSimulateFailure: (failureMode: boolean) => void;
+// execCommand is the workaround for copying text to the clipboard on HTTP-only
+// connections
+const originalExecCommand = global.document.execCommand;
+const originalNavigator = window.navigator;
+
+// Need to mock console.error because we deliberately need to trigger errors in
+// the code to assert that it can recover from them, but we also don't want them
+// logged if they're expected
+const originalConsoleError = console.error;
+
+type SetupMockClipboardResult = Readonly<{
+  mockClipboard: Clipboard;
+  mockExecCommand: typeof originalExecCommand;
+  getClipboardText: () => string;
+  setSimulateFailure: (shouldFail: boolean) => void;
 }>;
 
-type MockClipboard = Readonly<Clipboard & MockClipboardEscapeHatches>;
-function makeMockClipboard(isSecureContext: boolean): MockClipboard {
-  let mockClipboardValue = "";
-  let shouldFail = false;
-
-  return {
-    get simulateFailure() {
-      return shouldFail;
-    },
-    setSimulateFailure: (value) => {
-      shouldFail = value;
-    },
+function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
+  let mockClipboardText = "";
+  let shouldSimulateFailure = false;
 
+  const mockClipboard: Clipboard = {
     readText: async () => {
-      if (shouldFail) {
-        throw new Error("Clipboard deliberately failed");
-      }
-
-      if (!isSecureContext) {
+      if (!isSecure) {
         throw new Error(
-          "Trying to read from clipboard outside secure context!",
+          "Not allowed to access clipboard outside of secure contexts",
         );
       }
 
-      return mockClipboardValue;
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to read from clipboard");
+      }
+
+      return mockClipboardText;
     },
+
     writeText: async (newText) => {
-      if (shouldFail) {
-        throw new Error("Clipboard deliberately failed");
+      if (!isSecure) {
+        throw new Error(
+          "Not allowed to access clipboard outside of secure contexts",
+        );
       }
 
-      if (!isSecureContext) {
-        throw new Error("Trying to write to clipboard outside secure context!");
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to write to clipboard");
       }
 
-      mockClipboardValue = newText;
-    },
-
-    getMockText: () => mockClipboardValue,
-    setMockText: (newText) => {
-      mockClipboardValue = newText;
+      mockClipboardText = newText;
     },
 
+    // Don't need these other methods for any of the tests; read and write are
+    // both synchronous and slower than the promise-based methods, so ideally
+    // we won't ever need to call them in the hook logic
     addEventListener: jest.fn(),
     removeEventListener: jest.fn(),
     dispatchEvent: jest.fn(),
     read: jest.fn(),
     write: jest.fn(),
   };
+
+  return {
+    mockClipboard,
+    getClipboardText: () => mockClipboardText,
+    setSimulateFailure: (newShouldFailValue) => {
+      shouldSimulateFailure = newShouldFailValue;
+    },
+    mockExecCommand: (commandId) => {
+      if (commandId !== "copy") {
+        return false;
+      }
+
+      if (shouldSimulateFailure) {
+        throw new Error("Failed to execute command 'copy'");
+      }
+
+      const dummyInput = document.querySelector("input[data-testid=dummy]");
+      const inputIsFocused =
+        dummyInput instanceof HTMLInputElement &&
+        document.activeElement === dummyInput;
+
+      let copySuccessful = false;
+      if (inputIsFocused) {
+        mockClipboardText = dummyInput.value;
+        copySuccessful = true;
+      }
+
+      return copySuccessful;
+    },
+  };
 }
 
-function renderUseClipboard(inputs: UseClipboardInput) {
-  return renderHook<UseClipboardResult, UseClipboardInput>(
+function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
+  return renderHook<UseClipboardResult, TInput>(
     (props) => useClipboard(props),
     {
       initialProps: inputs,
       wrapper: ({ children }) => (
+        // Need ThemeProvider because GlobalSnackbar uses theme
         <ThemeProvider>
           {children}
           <GlobalSnackbar />
@@ -133,79 +125,77 @@ function renderUseClipboard(inputs: UseClipboardInput) {
   );
 }
 
-type ScheduleConfig = Readonly<{ isHttps: boolean }>;
+const secureContextValues: readonly boolean[] = [true, false];
 
-export function scheduleClipboardTests({ isHttps }: ScheduleConfig) {
-  const mockClipboardInstance = makeMockClipboard(isHttps);
-  const originalNavigator = window.navigator;
+// Not a big fan of describe.each most of the time, but since we need to test
+// the exact same test cases against different inputs, and we want them to run
+// as sequentially as possible to minimize flakes, they make sense here
+describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
+  const {
+    mockClipboard,
+    mockExecCommand,
+    getClipboardText,
+    setSimulateFailure,
+  } = setupMockClipboard(isSecure);
 
   beforeEach(() => {
     jest.useFakeTimers();
+    global.document.execCommand = mockExecCommand;
     jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
       ...originalNavigator,
-      clipboard: mockClipboardInstance,
+      clipboard: mockClipboard,
     }));
 
-    if (!isHttps) {
-      // Not the biggest fan of exposing implementation details like this, but
-      // making any kind of mock for execCommand is really gnarly in general
-      global.document.execCommand = jest.fn(() => {
-        if (mockClipboardInstance.simulateFailure) {
-          return false;
-        }
-
-        const dummyInput = document.querySelector("input[data-testid=dummy]");
-        const inputIsFocused =
-          dummyInput instanceof HTMLInputElement &&
-          document.activeElement === dummyInput;
-
-        let copySuccessful = false;
-        if (inputIsFocused) {
-          mockClipboardInstance.setMockText(dummyInput.value);
-          copySuccessful = true;
-        }
-
-        return copySuccessful;
-      });
-    }
+    console.error = (errorValue, ...rest) => {
+      const canIgnore =
+        errorValue instanceof Error &&
+        errorValue.message === COPY_FAILED_MESSAGE;
+
+      if (!canIgnore) {
+        originalConsoleError(errorValue, ...rest);
+      }
+    };
   });
 
   afterEach(() => {
+    jest.runAllTimers();
     jest.useRealTimers();
-    mockClipboardInstance.setMockText("");
-    mockClipboardInstance.setSimulateFailure(false);
+    jest.resetAllMocks();
+
+    console.error = originalConsoleError;
+    global.document.execCommand = originalExecCommand;
   });
 
-  const assertClipboardTextUpdate = async (
+  const assertClipboardUpdateLifecycle = async (
     result: ReturnType<typeof renderUseClipboard>["result"],
     textToCheck: string,
   ): Promise<void> => {
     await act(() => result.current.copyToClipboard());
     expect(result.current.showCopiedSuccess).toBe(true);
 
-    const clipboardText = mockClipboardInstance.getMockText();
+    // Because of timing trickery, any timeouts for flipping the copy status
+    // back to false will trigger before the test can complete. This will never
+    // be an issue in the real world, but it will kick up 'act' warnings in the
+    // console, which makes tests more annoying. Just waiting for them to finish
+    // up to avoid anything from being logged, but note that the value of
+    // showCopiedSuccess will become false after this
+    await act(() => jest.runAllTimersAsync());
+
+    const clipboardText = getClipboardText();
     expect(clipboardText).toEqual(textToCheck);
   };
 
-  /**
-   * Start of test cases
-   */
   it("Copies the current text to the user's clipboard", async () => {
     const textToCopy = "dogs";
     const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardTextUpdate(result, textToCopy);
+    await assertClipboardUpdateLifecycle(result, textToCopy);
   });
 
   it("Should indicate to components not to show successful copy after a set period of time", async () => {
     const textToCopy = "cats";
     const { result } = renderUseClipboard({ textToCopy });
-    await assertClipboardTextUpdate(result, textToCopy);
-
-    setTimeout(() => {
-      expect(result.current.showCopiedSuccess).toBe(false);
-    }, 10_000);
-
-    await jest.runAllTimersAsync();
+    await assertClipboardUpdateLifecycle(result, textToCopy);
+    expect(result.current.showCopiedSuccess).toBe(false);
   });
 
   it("Should notify the user of an error using the provided callback", async () => {
@@ -213,8 +203,19 @@ export function scheduleClipboardTests({ isHttps }: ScheduleConfig) {
     const onError = jest.fn();
     const { result } = renderUseClipboard({ textToCopy, onError });
 
-    mockClipboardInstance.setSimulateFailure(true);
+    setSimulateFailure(true);
     await act(() => result.current.copyToClipboard());
     expect(onError).toBeCalled();
   });
-}
+
+  it("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
+    const textToCopy = "crow";
+    const { result } = renderUseClipboard({ textToCopy });
+
+    setSimulateFailure(true);
+    await act(() => result.current.copyToClipboard());
+
+    const errorMessageNode = screen.queryByText(COPY_FAILED_MESSAGE);
+    expect(errorMessageNode).not.toBeNull();
+  });
+});

From 8dfe4229b159523f29db3c0dd3feaf48b067690e Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Sun, 12 May 2024 21:19:34 +0000
Subject: [PATCH 05/12] refactor: make http dependency more explicit

---
 site/src/hooks/useClipboard.test.tsx | 6 +++++-
 site/src/hooks/useClipboard.ts       | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 12320e10acd38..1ef3dc5efc169 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -16,6 +16,7 @@ import {
   type UseClipboardResult,
   COPY_FAILED_MESSAGE,
   useClipboard,
+  HTTP_FALLBACK_DATA_ID,
 } from "./useClipboard";
 
 // execCommand is the workaround for copying text to the clipboard on HTTP-only
@@ -93,7 +94,10 @@ function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
         throw new Error("Failed to execute command 'copy'");
       }
 
-      const dummyInput = document.querySelector("input[data-testid=dummy]");
+      const dummyInput = document.querySelector(
+        `input[data-testid=${HTTP_FALLBACK_DATA_ID}]`,
+      );
+
       const inputIsFocused =
         dummyInput instanceof HTMLInputElement &&
         document.activeElement === dummyInput;
diff --git a/site/src/hooks/useClipboard.ts b/site/src/hooks/useClipboard.ts
index fa5bc5474e02a..6228c3778766d 100644
--- a/site/src/hooks/useClipboard.ts
+++ b/site/src/hooks/useClipboard.ts
@@ -3,6 +3,7 @@ import { displayError } from "components/GlobalSnackbar/utils";
 
 const CLIPBOARD_TIMEOUT_MS = 1_000;
 export const COPY_FAILED_MESSAGE = "Failed to copy text to clipboard";
+export const HTTP_FALLBACK_DATA_ID = "http-fallback";
 
 export type UseClipboardInput = Readonly<{
   textToCopy: string;
@@ -99,7 +100,7 @@ function simulateClipboardWrite(textToCopy: string): boolean {
   const dummyInput = document.createElement("input");
 
   // Have to add test ID to dummy element for mocking purposes in tests
-  dummyInput.setAttribute("data-testid", "dummy");
+  dummyInput.setAttribute("data-testid", HTTP_FALLBACK_DATA_ID);
 
   // Using visually-hidden styling to ensure that inserting the element doesn't
   // cause any content reflows on the page (removes any risk of UI flickers).

From caf12220408fa2f7af62d7bc4607b1b4a09c39cd Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Mon, 13 May 2024 13:11:22 +0000
Subject: [PATCH 06/12] chore: add extra test case for exposed error value

---
 site/src/hooks/useClipboard.test.tsx | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 1ef3dc5efc169..5504f0543b781 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -212,14 +212,32 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     expect(onError).toBeCalled();
   });
 
-  it("Should dispatch a new toast message to the global snackbar if no callback is provided", async () => {
+  it("Should dispatch a new toast message to the global snackbar when errors happen if no error callback is provided to the hook", async () => {
     const textToCopy = "crow";
     const { result } = renderUseClipboard({ textToCopy });
 
+    /**
+     * @todo Look into why deferring error-based state updates to the global
+     * snackbar still kicks up act warnings, even after using act for the main
+     * source of the state transition
+     */
     setSimulateFailure(true);
     await act(() => result.current.copyToClipboard());
 
     const errorMessageNode = screen.queryByText(COPY_FAILED_MESSAGE);
     expect(errorMessageNode).not.toBeNull();
   });
+
+  it("Should expose the error value for render logic when a copy fails", async () => {
+    // Using empty error callback to silence any possible act warnings from
+    // Snackbar state transitions
+    const onError = jest.fn();
+    const textToCopy = "hamster";
+    const { result } = renderUseClipboard({ textToCopy, onError });
+
+    setSimulateFailure(true);
+    await act(() => result.current.copyToClipboard());
+
+    expect(result.current.error).toBeInstanceOf(Error);
+  });
 });

From 65910c17fe0e1e999200544f0c382bf075ac8019 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Mon, 13 May 2024 13:31:46 +0000
Subject: [PATCH 07/12] docs: fix typos

---
 site/src/hooks/useClipboard.test.tsx | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 5504f0543b781..ca713139a4dc5 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -4,9 +4,10 @@
  *
  * When you call user.setup to make a new user session, it will make a mock
  * clipboard instance that will always succeed. It also can't be removed after
- * it's been added. This actually makes testing useClipboard impossible to test
- * properly because any call to user.setup immediately pollutes the tests with
- * false negatives. Even if something should fail, it won't.
+ * it's been added, and it will persist across test cases. This actually makes
+ * testing useClipboard properly impossible because any call to user.setup
+ * immediately pollutes the tests with false negatives. Even if something should
+ * fail, it won't.
  */
 import { act, renderHook, screen } from "@testing-library/react";
 import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar";
@@ -218,8 +219,9 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
 
     /**
      * @todo Look into why deferring error-based state updates to the global
-     * snackbar still kicks up act warnings, even after using act for the main
-     * source of the state transition
+     * snackbar still kicks up act warnings, even after wrapping copyToClipboard
+     * in act. copyToClipboard should be the main source of the state
+     * transitions,
      */
     setSimulateFailure(true);
     await act(() => result.current.copyToClipboard());
@@ -228,12 +230,12 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     expect(errorMessageNode).not.toBeNull();
   });
 
-  it("Should expose the error value for render logic when a copy fails", async () => {
-    // Using empty error callback to silence any possible act warnings from
-    // Snackbar state transitions
-    const onError = jest.fn();
+  it("Should expose the error as a value when a copy fails", async () => {
+    // Using empty onError callback to silence any possible act warnings from
+    // Snackbar state transitions that you might get if the hook uses the
+    // default
     const textToCopy = "hamster";
-    const { result } = renderUseClipboard({ textToCopy, onError });
+    const { result } = renderUseClipboard({ textToCopy, onError: jest.fn() });
 
     setSimulateFailure(true);
     await act(() => result.current.copyToClipboard());

From 4399630e36d5ce4d9c412925790123b1e0e416d1 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Mon, 13 May 2024 16:50:49 +0000
Subject: [PATCH 08/12] fix: make sure clipboard is reset between test runs

---
 site/src/hooks/useClipboard.test.tsx | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index ca713139a4dc5..98978c9f926be 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -35,6 +35,7 @@ type SetupMockClipboardResult = Readonly<{
   mockExecCommand: typeof originalExecCommand;
   getClipboardText: () => string;
   setSimulateFailure: (shouldFail: boolean) => void;
+  resetClipboardMocks: () => void;
 }>;
 
 function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
@@ -86,6 +87,10 @@ function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
     setSimulateFailure: (newShouldFailValue) => {
       shouldSimulateFailure = newShouldFailValue;
     },
+    resetClipboardMocks: () => {
+      shouldSimulateFailure = false;
+      mockClipboardText = "";
+    },
     mockExecCommand: (commandId) => {
       if (commandId !== "copy") {
         return false;
@@ -141,6 +146,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     mockExecCommand,
     getClipboardText,
     setSimulateFailure,
+    resetClipboardMocks,
   } = setupMockClipboard(isSecure);
 
   beforeEach(() => {
@@ -167,6 +173,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     jest.useRealTimers();
     jest.resetAllMocks();
 
+    resetClipboardMocks();
     console.error = originalConsoleError;
     global.document.execCommand = originalExecCommand;
   });
@@ -179,11 +186,12 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     expect(result.current.showCopiedSuccess).toBe(true);
 
     // Because of timing trickery, any timeouts for flipping the copy status
-    // back to false will trigger before the test can complete. This will never
-    // be an issue in the real world, but it will kick up 'act' warnings in the
-    // console, which makes tests more annoying. Just waiting for them to finish
-    // up to avoid anything from being logged, but note that the value of
-    // showCopiedSuccess will become false after this
+    // back to false will usually trigger before any test cases calling this
+    // assert function can complete. This will never be an issue in the real
+    // world, but it will kick up 'act' warnings in the console, which makes
+    // tests more annoying. Getting around that by waiting for all timeouts to
+    // wrap up, but note that the value of showCopiedSuccess will become false
+    // after runAllTimersAsync finishes
     await act(() => jest.runAllTimersAsync());
 
     const clipboardText = getClipboardText();

From 186f6c89ee905c7c81ba85194bc5efcd8cdb774c Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Mon, 13 May 2024 16:55:45 +0000
Subject: [PATCH 09/12] docs: add more context to comments

---
 site/src/hooks/useClipboard.test.tsx | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 98978c9f926be..6ab8df79d32cb 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -229,7 +229,8 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
      * @todo Look into why deferring error-based state updates to the global
      * snackbar still kicks up act warnings, even after wrapping copyToClipboard
      * in act. copyToClipboard should be the main source of the state
-     * transitions,
+     * transitions, but it looks like extra state changes are still getting
+     * flushed through the GlobalSnackbar component afterwards
      */
     setSimulateFailure(true);
     await act(() => result.current.copyToClipboard());

From a5214fc23d36ac8133632b7547f4249dc5d09edb Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Wed, 15 May 2024 13:30:29 +0000
Subject: [PATCH 10/12] refactor: update mock console.error logic to use
 jest.spyOn

---
 site/src/hooks/useClipboard.test.tsx | 29 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index 6ab8df79d32cb..b22e5978f8375 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -20,11 +20,6 @@ import {
   HTTP_FALLBACK_DATA_ID,
 } from "./useClipboard";
 
-// execCommand is the workaround for copying text to the clipboard on HTTP-only
-// connections
-const originalExecCommand = global.document.execCommand;
-const originalNavigator = window.navigator;
-
 // Need to mock console.error because we deliberately need to trigger errors in
 // the code to assert that it can recover from them, but we also don't want them
 // logged if they're expected
@@ -32,10 +27,10 @@ const originalConsoleError = console.error;
 
 type SetupMockClipboardResult = Readonly<{
   mockClipboard: Clipboard;
-  mockExecCommand: typeof originalExecCommand;
+  mockExecCommand: typeof global.document.execCommand;
   getClipboardText: () => string;
   setSimulateFailure: (shouldFail: boolean) => void;
-  resetClipboardMocks: () => void;
+  resetMockClipboardState: () => void;
 }>;
 
 function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
@@ -87,7 +82,7 @@ function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
     setSimulateFailure: (newShouldFailValue) => {
       shouldSimulateFailure = newShouldFailValue;
     },
-    resetClipboardMocks: () => {
+    resetMockClipboardState: () => {
       shouldSimulateFailure = false;
       mockClipboardText = "";
     },
@@ -135,29 +130,34 @@ function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
   );
 }
 
-const secureContextValues: readonly boolean[] = [true, false];
+// execCommand is the workaround for copying text to the clipboard on HTTP-only
+// connections
+const originalExecCommand = global.document.execCommand;
+const originalNavigator = window.navigator;
 
 // Not a big fan of describe.each most of the time, but since we need to test
 // the exact same test cases against different inputs, and we want them to run
 // as sequentially as possible to minimize flakes, they make sense here
+const secureContextValues: readonly boolean[] = [true, false];
 describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
   const {
     mockClipboard,
     mockExecCommand,
     getClipboardText,
     setSimulateFailure,
-    resetClipboardMocks,
+    resetMockClipboardState,
   } = setupMockClipboard(isSecure);
 
   beforeEach(() => {
     jest.useFakeTimers();
     global.document.execCommand = mockExecCommand;
+
     jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
       ...originalNavigator,
       clipboard: mockClipboard,
     }));
 
-    console.error = (errorValue, ...rest) => {
+    jest.spyOn(console, "error").mockImplementation((errorValue, ...rest) => {
       const canIgnore =
         errorValue instanceof Error &&
         errorValue.message === COPY_FAILED_MESSAGE;
@@ -165,7 +165,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
       if (!canIgnore) {
         originalConsoleError(errorValue, ...rest);
       }
-    };
+    });
   });
 
   afterEach(() => {
@@ -173,8 +173,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     jest.useRealTimers();
     jest.resetAllMocks();
 
-    resetClipboardMocks();
-    console.error = originalConsoleError;
+    resetMockClipboardState();
     global.document.execCommand = originalExecCommand;
   });
 
@@ -221,7 +220,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     expect(onError).toBeCalled();
   });
 
-  it("Should dispatch a new toast message to the global snackbar when errors happen if no error callback is provided to the hook", async () => {
+  it("Should dispatch a new toast message to the global snackbar when errors happen while no error callback is provided to the hook", async () => {
     const textToCopy = "crow";
     const { result } = renderUseClipboard({ textToCopy });
 

From f396df404a21383a40ed5ad3040f0d02e4c42d55 Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Wed, 15 May 2024 13:42:04 +0000
Subject: [PATCH 11/12] docs: add more clarifying comments

---
 site/src/hooks/useClipboard.test.tsx | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index b22e5978f8375..afcf999415e16 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -150,6 +150,10 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
 
   beforeEach(() => {
     jest.useFakeTimers();
+
+    // Can't use jest.spyOn here because there's no guarantee that the mock
+    // browser environment actually implements execCommand. Trying to spy on an
+    // undefined value will throw an error
     global.document.execCommand = mockExecCommand;
 
     jest.spyOn(window, "navigator", "get").mockImplementation(() => ({
@@ -172,9 +176,11 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
     jest.runAllTimers();
     jest.useRealTimers();
     jest.resetAllMocks();
+    global.document.execCommand = originalExecCommand;
 
+    // Still have to reset the mock clipboard state because the same mock values
+    // are reused for each test case in a given describe.each iteration
     resetMockClipboardState();
-    global.document.execCommand = originalExecCommand;
   });
 
   const assertClipboardUpdateLifecycle = async (

From 58beffc2c71a08092ec4926d994644a972ef495b Mon Sep 17 00:00:00 2001
From: Parkreiner <throwawayclover@gmail.com>
Date: Wed, 15 May 2024 13:44:02 +0000
Subject: [PATCH 12/12] refactor: split off type alias for clarity

---
 site/src/hooks/useClipboard.test.tsx | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx
index afcf999415e16..b8296efb26eb0 100644
--- a/site/src/hooks/useClipboard.test.tsx
+++ b/site/src/hooks/useClipboard.test.tsx
@@ -130,6 +130,8 @@ function renderUseClipboard<TInput extends UseClipboardInput>(inputs: TInput) {
   );
 }
 
+type RenderResult = ReturnType<typeof renderUseClipboard>["result"];
+
 // execCommand is the workaround for copying text to the clipboard on HTTP-only
 // connections
 const originalExecCommand = global.document.execCommand;
@@ -184,7 +186,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
   });
 
   const assertClipboardUpdateLifecycle = async (
-    result: ReturnType<typeof renderUseClipboard>["result"],
+    result: RenderResult,
     textToCheck: string,
   ): Promise<void> => {
     await act(() => result.current.copyToClipboard());