Skip to content

[WRONG BRANCH] test(ws-upstream): avoid unsupported fake timer advance #267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
luvs01 wants to merge 1 commit into
base: main
Choose a base branch
from
+9 −4
Draft

[WRONG BRANCH] test(ws-upstream): avoid unsupported fake timer advance #267

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions tests/ws-upstream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterEach, describe, expect, jest, test } from "bun:test";
import { afterEach, describe, expect, spyOn, test } from "bun:test";
import { providerFetch } from "../src/server/responses/fetch-helpers";
import { codexWsUpstreamFetch, shouldUseCodexWsUpstream } from "../src/server/responses/ws-upstream";
import type { OcxProviderConfig } from "../src/types";
Expand Down Expand Up @@ -190,7 +190,12 @@ describe("codexWsUpstreamFetch", () => {
});

test("falls back to the HTTP fetch when the upgrade deadline elapses without open or close", async () => {
jest.useFakeTimers();
let deadline: (() => void) | undefined;
const setTimeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation(((callback: () => void, delay?: number) => {
expect(delay).toBe(10_000);
deadline = callback;
return 0;
}) as typeof setTimeout);
try {
installFake(() => { /* handshake never settles */ });
const sentinel = new Response("sse-timeout-fallback", { status: 200 });
Expand All @@ -202,14 +207,14 @@ describe("codexWsUpstreamFetch", () => {

const responsePromise = codexWsUpstreamFetch(CODEX_URL, streamingInit(), fallback);
expect(FakeWebSocket.instances).toHaveLength(1);
jest.advanceTimersByTime(10_000);
deadline?.();
const response = await responsePromise;

expect(response).toBe(sentinel);
expect(fallbackCalls).toBe(1);
expect(FakeWebSocket.instances[0].closed).toBe(true);
} finally {
jest.useRealTimers();
setTimeoutSpy.mockRestore();
Comment on lines +210 to +217

@coderabbitai coderabbitai Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that the timeout callback was captured.

In tests/ws-upstream.test.ts, Line 210 skips the callback when deadline is undefined. The assertion at Line 195 does not detect a missing timer because it runs only when setTimeout is called. The test can then hang or complete through a different close path without testing the 10-second fallback.

Assert the callback before invoking it.

Proposed fix 
-      deadline?.();
+      expect(deadline).toBeDefined();
+      deadline!();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
deadline?.();
const response = await responsePromise;
expect(response).toBe(sentinel);
expect(fallbackCalls).toBe(1);
expect(FakeWebSocket.instances[0].closed).toBe(true);
} finally {
jest.useRealTimers();
setTimeoutSpy.mockRestore();
expect(deadline).toBeDefined();
deadline!();
const response = await responsePromise;
expect(response).toBe(sentinel);
expect(fallbackCalls).toBe(1);
expect(FakeWebSocket.instances[0].closed).toBe(true);
} finally {
setTimeoutSpy.mockRestore();
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/ws-upstream.test.ts` around lines 210 - 217, Update the test around the
deadline invocation to assert that deadline was captured before calling it,
ensuring the test fails when setTimeout does not register the fallback callback.
Keep the existing response, fallbackCalls, and socket-closed assertions
unchanged.

}
});

Expand Down
Loading