mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: update tests for useClipboard to minimize risks of flakes (#13250)
* wip: commit progress on test revamps * fix: update existing tests to new format * chore: add test case for global snackbar * refactor: consolidate files * refactor: make http dependency more explicit * chore: add extra test case for exposed error value * docs: fix typos * fix: make sure clipboard is reset between test runs * docs: add more context to comments * refactor: update mock console.error logic to use jest.spyOn * docs: add more clarifying comments * refactor: split off type alias for clarity
This commit is contained in:
Michael Smith
@@ -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 });
|
||||
});
|
||||
// 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;
|
||||
|
||||
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;
|
||||
type SetupMockClipboardResult = Readonly<{
|
||||
mockClipboard: Clipboard;
|
||||
mockExecCommand: typeof global.document.execCommand;
|
||||
getClipboardText: () => string;
|
||||
setSimulateFailure: (shouldFail: boolean) => void;
|
||||
resetMockClipboardState: () => 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;
|
||||
},
|
||||
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<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 +130,92 @@ function renderUseClipboard(inputs: UseClipboardInput) {
|
||||
);
|
||||
}
|
||||
|
||||
type ScheduleConfig = Readonly<{ isHttps: boolean }>;
|
||||
type RenderResult = ReturnType<typeof renderUseClipboard>["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;
|
||||
}
|
||||
jest.spyOn(console, "error").mockImplementation((errorValue, ...rest) => {
|
||||
const canIgnore =
|
||||
errorValue instanceof Error &&
|
||||
errorValue.message === COPY_FAILED_MESSAGE;
|
||||
|
||||
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;
|
||||
});
|
||||
}
|
||||
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<typeof renderUseClipboard>["result"],
|
||||
const assertClipboardUpdateLifecycle = async (
|
||||
result: RenderResult,
|
||||
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 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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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).
|
||||
|
||||
Reference in New Issue
Block a user