test: remove waits from slow tests

This commit is contained in:
gsxdsm
2026-04-12 12:52:32 -07:00
parent 231c984a27
commit 52f1f77369
5 changed files with 177 additions and 117 deletions

View File

@@ -47,6 +47,18 @@ const defaultModels = [
{ provider: "openai", id: "gpt-4o", name: "GPT-4o", reasoning: false, contextWindow: 128000 },
];
async function clickContinueToModelStep() {
await waitFor(() => {
expect(screen.getByText("Continue →")).toBeTruthy();
});
fireEvent.click(screen.getByText("Continue →"));
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
});
}
beforeEach(() => {
vi.clearAllMocks();
mockFetchAuthStatus.mockResolvedValue({ providers: defaultAuthProviders });
@@ -205,7 +217,7 @@ describe("ModelOnboardingModal", () => {
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
});
});
it("shows Save button as disabled when API key input is empty", async () => {
@@ -264,15 +276,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={vi.fn()} addToast={vi.fn()} />);
await waitFor(() => {
expect(screen.getByText("Continue →")).toBeTruthy();
});
// Wait for auto-advance or click Continue
// The auto-advance happens after 600ms, but we can also click
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
});
it("shows model dropdown on the model step", async () => {
@@ -284,9 +288,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={vi.fn()} addToast={vi.fn()} />);
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
expect(screen.getByTestId("mock-model-dropdown")).toBeTruthy();
});
@@ -300,9 +302,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={vi.fn()} addToast={vi.fn()} />);
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
const dropdown = screen.getByTestId("mock-model-dropdown");
fireEvent.change(dropdown, { target: { value: "anthropic/claude-sonnet-4-5" } });
@@ -321,9 +321,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={vi.fn()} addToast={vi.fn()} />);
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
fireEvent.click(screen.getByText("← Back"));
@@ -344,10 +342,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={onComplete} addToast={vi.fn()} />);
// Wait for auto-advance to model step
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
// Select a model
const dropdown = screen.getByTestId("mock-model-dropdown");
@@ -390,10 +385,7 @@ describe("ModelOnboardingModal", () => {
render(<ModelOnboardingModal onComplete={onComplete} addToast={vi.fn()} />);
// Wait for auto-advance
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
// Click Complete Setup without selecting a model
fireEvent.click(screen.getByText("Complete Setup"));
@@ -486,13 +478,11 @@ describe("ModelOnboardingModal", () => {
{ id: "anthropic", name: "Anthropic", authenticated: true, type: "oauth" },
],
});
mockFetchModels.mockResolvedValueOnce({ models: [], favoriteProviders: [], favoriteModels: [] });
mockFetchModels.mockResolvedValue({ models: [], favoriteProviders: [], favoriteModels: [] });
render(<ModelOnboardingModal onComplete={vi.fn()} addToast={vi.fn()} />);
await waitFor(() => {
expect(screen.getByText("Choose Default Model")).toBeTruthy();
}, { timeout: 3000 });
await clickContinueToModelStep();
expect(screen.getByText(/No models available/)).toBeTruthy();
});

View File

@@ -590,92 +590,114 @@ describe("TerminalModal", () => {
// --- initialCommand / script launch behavior ---
describe("initialCommand execution", () => {
async function flushInitialCommandDelay() {
await act(async () => {
await vi.advanceTimersByTimeAsync(500);
});
}
it("sends initialCommand to terminal when connected", async () => {
vi.useFakeTimers();
mockUseTerminal.mockReturnValue(
createMockTerminalState({ connectionStatus: "connected" })
);
render(<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />);
try {
render(<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("npm run build\n");
});
} finally {
vi.useRealTimers();
}
});
it("does not send the same initialCommand twice on re-renders", async () => {
vi.useFakeTimers();
mockUseTerminal.mockReturnValue(
createMockTerminalState({ connectionStatus: "connected" })
);
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
try {
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("npm run build\n");
});
const callCount = mockSendInput.mock.calls.length;
const callCount = mockSendInput.mock.calls.length;
// Re-render with same props
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
// Re-render with same props
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
// Should not send the command again
expect(mockSendInput).toHaveBeenCalledTimes(callCount);
await flushInitialCommandDelay();
// Should not send the command again
expect(mockSendInput).toHaveBeenCalledTimes(callCount);
} finally {
vi.useRealTimers();
}
});
it("sends a new initialCommand when it changes while terminal is open", async () => {
vi.useFakeTimers();
mockUseTerminal.mockReturnValue(
createMockTerminalState({ connectionStatus: "connected" })
);
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
try {
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("npm run build\n");
});
// Change the command (e.g., user runs a different script)
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
);
// Change the command (e.g., user runs a different script)
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="pnpm test" />
);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("pnpm test\n");
});
} finally {
vi.useRealTimers();
}
});
it("resends command after modal close and reopen", async () => {
vi.useFakeTimers();
mockUseTerminal.mockReturnValue(
createMockTerminalState({ connectionStatus: "connected" })
);
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
try {
const { rerender } = render(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("npm run build\n");
});
// Close the modal
rerender(
<TerminalModal isOpen={false} onClose={mockOnClose} initialCommand="npm run build" />
);
// Close the modal
rerender(
<TerminalModal isOpen={false} onClose={mockOnClose} initialCommand="npm run build" />
);
// Reopen with the same command
mockSendInput.mockClear();
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
// Reopen with the same command
mockSendInput.mockClear();
rerender(
<TerminalModal isOpen={true} onClose={mockOnClose} initialCommand="npm run build" />
);
await waitFor(() => {
await flushInitialCommandDelay();
expect(mockSendInput).toHaveBeenCalledWith("npm run build\n");
});
} finally {
vi.useRealTimers();
}
});
});

View File

@@ -19,6 +19,7 @@ describe("useBatchBadgeFetch", () => {
afterEach(() => {
vi.clearAllMocks();
vi.useRealTimers();
});
it("calls API with correct task IDs", async () => {
@@ -60,8 +61,12 @@ describe("useBatchBadgeFetch", () => {
const hook2 = renderHook(() => useBatchBadgeFetch());
// Start both fetches concurrently (but don't await yet)
const fetchPromise1 = hook1.result.current.fetchBatch(["FN-001"]);
const fetchPromise2 = hook2.result.current.fetchBatch(["FN-001"]);
let fetchPromise1!: Promise<void>;
let fetchPromise2!: Promise<void>;
act(() => {
fetchPromise1 = hook1.result.current.fetchBatch(["FN-001"]);
fetchPromise2 = hook2.result.current.fetchBatch(["FN-001"]);
});
// Resolve the shared promise
resolvePromise!(mockResult);
@@ -108,33 +113,42 @@ describe("useBatchBadgeFetch", () => {
});
it("makes new API call after 5 second cache expires", async () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
const mockResult: BatchStatusResult = {
"FN-001": { issueInfo: undefined, prInfo: undefined, stale: true },
};
mockFetchBatchStatus.mockResolvedValue(mockResult);
const { result } = renderHook(() => useBatchBadgeFetch());
try {
const { result } = renderHook(() => useBatchBadgeFetch());
// First fetch
await act(async () => {
await result.current.fetchBatch(["FN-001"]);
});
// First fetch
await act(async () => {
await result.current.fetchBatch(["FN-001"]);
});
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(1);
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(1);
// Wait for cache to expire (5 seconds + 1ms buffer)
await new Promise((resolve) => setTimeout(resolve, 5100));
// Move past the cache expiration window without waiting in real time.
vi.setSystemTime(new Date("2026-01-01T00:00:05.001Z"));
// Second fetch after cache expired
await act(async () => {
await result.current.fetchBatch(["FN-001"]);
});
// Second fetch after cache expired
await act(async () => {
await result.current.fetchBatch(["FN-001"]);
});
// Should make a new API call
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(2);
}, 10000);
// Should make a new API call
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(2);
} finally {
vi.useRealTimers();
}
});
it("retries 429 errors with exponential backoff", async () => {
vi.useFakeTimers();
const rateLimitError = new Error("429 Rate limit exceeded");
const mockResult: BatchStatusResult = {
"FN-001": { issueInfo: undefined, prInfo: undefined, stale: true },
@@ -146,19 +160,29 @@ describe("useBatchBadgeFetch", () => {
.mockRejectedValueOnce(rateLimitError)
.mockResolvedValueOnce(mockResult);
const { result } = renderHook(() => useBatchBadgeFetch());
try {
const { result } = renderHook(() => useBatchBadgeFetch());
// Start the fetch
await act(async () => {
await result.current.fetchBatch(["FN-001"]);
});
// Start the fetch, then advance through the retry backoff immediately.
let fetchPromise: Promise<void>;
act(() => {
fetchPromise = result.current.fetchBatch(["FN-001"]);
});
// Wait for retries (exponential backoff: 1s + 2s = 3s total)
await new Promise((resolve) => setTimeout(resolve, 4000));
await act(async () => {
await vi.advanceTimersByTimeAsync(1000);
await vi.advanceTimersByTimeAsync(2000);
});
await act(async () => {
await fetchPromise;
});
// Should have made multiple calls due to retries
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(3);
}, 10000);
// Should have made multiple calls due to retries
expect(mockFetchBatchStatus).toHaveBeenCalledTimes(3);
} finally {
vi.useRealTimers();
}
});
it("does not retry non-429 errors", async () => {
const otherError = new Error("Network error");

View File

@@ -1110,8 +1110,9 @@ describe("GitHubClient", () => {
});
it("respects Retry-After header on 429", async () => {
vi.useFakeTimers();
const headers = new Headers();
headers.set("Retry-After", "1"); // Use 1 second for test speed
headers.set("Retry-After", "1");
fetchSpy
.mockResolvedValueOnce({
@@ -1127,22 +1128,26 @@ describe("GitHubClient", () => {
json: () => Promise.resolve({ id: 1 }),
} as Response);
const startTime = Date.now();
const result = await client.fetchThrottled(
const resultPromise = client.fetchThrottled(
"https://api.github.com/repos/owner/repo/issues/1",
{},
{ delayMs: 100, maxRetries: 3 }
);
const elapsed = Date.now() - startTime;
await vi.advanceTimersByTimeAsync(999);
expect(fetchSpy).toHaveBeenCalledTimes(1);
await vi.advanceTimersByTimeAsync(1);
const result = await resultPromise;
expect(result.success).toBe(true);
// Should wait at least 1 second (Retry-After value), not just the exponential backoff
expect(elapsed).toBeGreaterThanOrEqual(900); // Allow some tolerance
}, 10000); // Increase timeout for this test
expect(fetchSpy).toHaveBeenCalledTimes(2);
});
it("returns error with retryAfter after max retries exceeded", async () => {
vi.useFakeTimers();
const headers = new Headers();
headers.set("Retry-After", "1"); // Use 1 second for test speed
headers.set("Retry-After", "1");
// All attempts return 429
fetchSpy.mockResolvedValue({
@@ -1153,17 +1158,20 @@ describe("GitHubClient", () => {
json: () => Promise.resolve({ message: "Rate limited" }),
} as Response);
const result = await client.fetchThrottled(
const resultPromise = client.fetchThrottled(
"https://api.github.com/repos/owner/repo/issues/1",
{},
{ delayMs: 1, maxRetries: 2 }
);
await vi.advanceTimersByTimeAsync(2000);
const result = await resultPromise;
expect(result.success).toBe(false);
expect(result.error).toContain("rate limit exceeded");
expect(result.retryAfter).toBe(1);
expect(fetchSpy).toHaveBeenCalledTimes(3); // initial + 2 retries
}, 10000); // Increase timeout for this test
});
it("retries on network errors with exponential backoff", async () => {
fetchSpy
@@ -1200,6 +1208,9 @@ describe("GitHubClient", () => {
});
it("enforces delay between sequential requests", async () => {
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-01-01T00:00:01.000Z"));
fetchSpy.mockResolvedValue({
ok: true,
status: 200,
@@ -1213,17 +1224,19 @@ describe("GitHubClient", () => {
{ delayMs: 100 }
);
const startTime = Date.now();
// Second request should be delayed
await client.fetchThrottled(
const resultPromise = client.fetchThrottled(
"https://api.github.com/repos/owner/repo/issues/2",
{},
{ delayMs: 100 }
);
const elapsed = Date.now() - startTime;
// Should have waited at least 100ms between requests
expect(elapsed).toBeGreaterThanOrEqual(90); // Allow some tolerance
expect(fetchSpy).toHaveBeenCalledTimes(1);
await vi.advanceTimersByTimeAsync(99);
expect(fetchSpy).toHaveBeenCalledTimes(1);
await vi.advanceTimersByTimeAsync(1);
await resultPromise;
expect(fetchSpy).toHaveBeenCalledTimes(2);
});
it("uses custom delayMs option", async () => {

View File

@@ -171,6 +171,8 @@ describe("RemoteNodeClient", () => {
});
it("retries on network errors", async () => {
vi.useFakeTimers();
const fetchMock = vi
.fn()
.mockRejectedValueOnce(new TypeError("network down"))
@@ -184,11 +186,14 @@ describe("RemoteNodeClient", () => {
const client = new RemoteNodeClient({ baseUrl: BASE_URL, apiKey: API_KEY });
await expect(client.health()).resolves.toEqual({
const request = client.health();
const expectation = expect(request).resolves.toEqual({
status: "ok",
version: "1.0.0",
uptime: 123,
});
await vi.advanceTimersByTimeAsync(1000);
await expectation;
expect(fetchMock).toHaveBeenCalledTimes(2);
});
@@ -209,6 +214,8 @@ describe("RemoteNodeClient", () => {
});
it("retries on 5xx responses", async () => {
vi.useFakeTimers();
const fetchMock = vi
.fn()
.mockResolvedValueOnce(
@@ -227,11 +234,15 @@ describe("RemoteNodeClient", () => {
const client = new RemoteNodeClient({ baseUrl: BASE_URL, apiKey: API_KEY });
await expect(client.health()).resolves.toEqual({
const request = client.health();
const expectation = expect(request).resolves.toEqual({
status: "ok",
version: "1.0.0",
uptime: 999,
});
await vi.advanceTimersByTimeAsync(1000);
await vi.advanceTimersByTimeAsync(2000);
await expectation;
expect(fetchMock).toHaveBeenCalledTimes(3);
});