diff --git a/site/src/hooks/useClipboard.test.tsx b/site/src/hooks/useClipboard.test.tsx index 5ddbed3f8cc12..b8296efb26eb0 100644 --- a/site/src/hooks/useClipboard.test.tsx +++ b/site/src/hooks/useClipboard.test.tsx @@ -1,129 +1,126 @@ -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, 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"; import { ThemeProvider } from "contexts/ThemeProvider"; import { type UseClipboardInput, type UseClipboardResult, + COPY_FAILED_MESSAGE, useClipboard, + HTTP_FALLBACK_DATA_ID, } from "./useClipboard"; -describe(useClipboard.name, () => { - describe("HTTP (non-secure) connections", () => { - scheduleClipboardTests({ isHttps: false }); - }); - - describe("HTTPS (secure/default) connections", () => { - scheduleClipboardTests({ isHttps: true }); - }); -}); +// 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; -/** - * @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; +type SetupMockClipboardResult = Readonly<{ + mockClipboard: Clipboard; + mockExecCommand: typeof global.document.execCommand; + getClipboardText: () => string; + setSimulateFailure: (shouldFail: boolean) => void; + resetMockClipboardState: () => void; }>; -type MockClipboard = Readonly; -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; + }, + resetMockClipboardState: () => { + shouldSimulateFailure = false; + mockClipboardText = ""; + }, + mockExecCommand: (commandId) => { + if (commandId !== "copy") { + return false; + } + + if (shouldSimulateFailure) { + throw new Error("Failed to execute command 'copy'"); + } + + const dummyInput = document.querySelector( + `input[data-testid=${HTTP_FALLBACK_DATA_ID}]`, + ); + + 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( +function renderUseClipboard(inputs: TInput) { + return renderHook( (props) => useClipboard(props), { initialProps: inputs, wrapper: ({ children }) => ( + // Need ThemeProvider because GlobalSnackbar uses theme {children} @@ -133,79 +130,92 @@ function renderUseClipboard(inputs: UseClipboardInput) { ); } -type ScheduleConfig = Readonly<{ isHttps: boolean }>; +type RenderResult = ReturnType["result"]; -export function scheduleClipboardTests({ isHttps }: ScheduleConfig) { - const mockClipboardInstance = makeMockClipboard(isHttps); - const originalNavigator = window.navigator; +// 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, + resetMockClipboardState, + } = setupMockClipboard(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(() => ({ ...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; - }); - } + jest.spyOn(console, "error").mockImplementation((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(); + 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(); }); - const assertClipboardTextUpdate = async ( - result: ReturnType["result"], + const assertClipboardUpdateLifecycle = async ( + result: RenderResult, textToCheck: string, ): Promise => { 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 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(); 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 +223,39 @@ 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 when errors happen while 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 wrapping copyToClipboard + * in act. copyToClipboard should be the main source of the state + * transitions, but it looks like extra state changes are still getting + * flushed through the GlobalSnackbar component afterwards + */ + setSimulateFailure(true); + await act(() => result.current.copyToClipboard()); + + const errorMessageNode = screen.queryByText(COPY_FAILED_MESSAGE); + expect(errorMessageNode).not.toBeNull(); + }); + + 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: jest.fn() }); + + setSimulateFailure(true); + await act(() => result.current.copyToClipboard()); + + expect(result.current.error).toBeInstanceOf(Error); + }); +}); diff --git a/site/src/hooks/useClipboard.ts b/site/src/hooks/useClipboard.ts index 83ec8283ed710..6228c3778766d 100644 --- a/site/src/hooks/useClipboard.ts +++ b/site/src/hooks/useClipboard.ts @@ -2,7 +2,8 @@ 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 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).