Skip to content

Commit de60ea5

Browse files
committed
fix(test): stop Windows star-route hangs and raise trash restore timeout
Route tests were spawning real gh.cmd and burning AUTH_TIMEOUT equal to Bun's 5s deadline. Inject a fast fake via setStarDepsForTests, and give the slow compressed trash restore 20s on Windows CI.
1 parent 0afd2d6 commit de60ea5

3 files changed

Lines changed: 48 additions & 16 deletions

File tree

src/github/star-state.ts

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,23 @@ async function spawnGh(args: string[], timeoutMs: number): Promise<{ status: num
8080
}
8181

8282
const defaultDeps: StarDeps = { runGh: spawnGh, nowMs: () => Date.now() };
83+
/**
84+
* Route tests call the real management dispatcher, which has no place to pass
85+
* `StarDeps`. Without an override, Windows CI spawns the real `gh.cmd` shim and
86+
* burns the full `AUTH_TIMEOUT_MS` — equal to Bun's default test timeout — so
87+
* `GET /api/github/star` flakes as a 5s hang. Tests install a fast fake here.
88+
*/
89+
let overrideDeps: StarDeps | null = null;
90+
91+
function resolveDeps(deps?: StarDeps): StarDeps {
92+
return deps ?? overrideDeps ?? defaultDeps;
93+
}
94+
95+
/** Test-only: swap the `gh` runner used when callers omit `deps`. Pass `null` to clear. */
96+
export function setStarDepsForTests(deps: StarDeps | null): void {
97+
overrideDeps = deps;
98+
invalidateStarStatusCache();
99+
}
83100

84101
let cached: { timestamp: number; state: StarState } | null = null;
85102
/** Coalesces concurrent probes so parallel sidebar polls share one `gh` run. */
@@ -97,10 +114,11 @@ let generation = 0;
97114
* starred and 404 when not, so a non-zero exit is only meaningful once we know
98115
* the CLI is authenticated — hence the auth check first.
99116
*/
100-
export async function probeStarState(deps: StarDeps = defaultDeps): Promise<StarState> {
101-
const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
117+
export async function probeStarState(deps?: StarDeps): Promise<StarState> {
118+
const resolved = resolveDeps(deps);
119+
const auth = await resolved.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
102120
if (!auth || auth.status !== 0) return "unauthenticated";
103-
const starred = await deps.runGh(
121+
const starred = await resolved.runGh(
104122
["api", "--hostname", GH_HOSTNAME, `/user/starred/${STAR_REPO}`],
105123
API_TIMEOUT_MS,
106124
);
@@ -109,8 +127,9 @@ export async function probeStarState(deps: StarDeps = defaultDeps): Promise<Star
109127
}
110128

111129
/** Cached star state; `gh` is only spawned when the cache is cold or expired. */
112-
export async function getStarStatus(deps: StarDeps = defaultDeps): Promise<StarStatus> {
113-
const now = deps.nowMs();
130+
export async function getStarStatus(deps?: StarDeps): Promise<StarStatus> {
131+
const resolved = resolveDeps(deps);
132+
const now = resolved.nowMs();
114133
if (cached && now - cached.timestamp < CACHE_TTL_MS) {
115134
return { state: cached.state, repo: STAR_REPO, url: STAR_REPO_URL };
116135
}
@@ -123,7 +142,7 @@ export async function getStarStatus(deps: StarDeps = defaultDeps): Promise<StarS
123142
// The slot is cleared inside the same continuation that commits the cache. Using
124143
// `.finally()` for that defers it by a microtask, which leaves a window where the
125144
// next caller awaits an already-settled probe instead of starting a fresh one.
126-
const probe = probeStarState(deps).then(
145+
const probe = probeStarState(resolved).then(
127146
state => {
128147
if (inflight === probe) inflight = null;
129148
// A write landed while this read was in flight — its result is authoritative.
@@ -159,19 +178,20 @@ export function invalidateStarStatusCache(): void {
159178
* management API.
160179
*/
161180
export async function starRepository(
162-
deps: StarDeps = defaultDeps,
181+
deps?: StarDeps,
163182
): Promise<{ ok: boolean; status: StarStatus; code?: StarErrorCode }> {
164-
const auth = await deps.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
183+
const resolved = resolveDeps(deps);
184+
const auth = await resolved.runGh(["auth", "status", "--hostname", GH_HOSTNAME], AUTH_TIMEOUT_MS);
165185
if (!auth || auth.status !== 0) {
166186
generation += 1;
167-
cached = { timestamp: deps.nowMs(), state: "unauthenticated" };
187+
cached = { timestamp: resolved.nowMs(), state: "unauthenticated" };
168188
return {
169189
ok: false,
170190
status: { state: "unauthenticated", repo: STAR_REPO, url: STAR_REPO_URL },
171191
code: "gh_unavailable",
172192
};
173193
}
174-
const result = await deps.runGh(
194+
const result = await resolved.runGh(
175195
["api", "--hostname", GH_HOSTNAME, "-X", "PUT", `/user/starred/${STAR_REPO}`],
176196
API_TIMEOUT_MS,
177197
);
@@ -186,6 +206,6 @@ export async function starRepository(
186206
// Authoritative: this call just starred the repo. Bumping the generation makes any
187207
// read that is still in flight discard its now-obsolete observation.
188208
generation += 1;
189-
cached = { timestamp: deps.nowMs(), state: "starred" };
209+
cached = { timestamp: resolved.nowMs(), state: "starred" };
190210
return { ok: true, status: { state: "starred", repo: STAR_REPO, url: STAR_REPO_URL } };
191211
}

tests/sidebar-routes.test.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,34 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
22
import { handleManagementAPI } from "../src/server/management-api";
3-
import { invalidateStarStatusCache } from "../src/github/star-state";
3+
import { setStarDepsForTests } from "../src/github/star-state";
44
import type { OcxConfig } from "../src/types";
55

66
/**
77
* Route-level proof for the two sidebar endpoints. The unit tests cover the state
88
* machine; this file checks that the routes are actually reachable through the
99
* management dispatcher and that the serialized bytes carry no `gh` output, token,
1010
* or account identifier.
11+
*
12+
* Star probes install a fake `gh` runner: the real Windows `gh.cmd` shim can burn
13+
* the full auth timeout, which equals Bun's default 5s test deadline.
1114
*/
1215
const config = {
1316
port: 10100,
1417
defaultProvider: "openai",
1518
providers: {},
1619
} as OcxConfig;
1720

21+
beforeEach(() => {
22+
setStarDepsForTests({
23+
runGh: async () => ({ status: 1 }),
24+
nowMs: () => Date.now(),
25+
});
26+
});
27+
28+
afterEach(() => {
29+
setStarDepsForTests(null);
30+
});
31+
1832
async function call(
1933
method: string,
2034
pathname: string,
@@ -52,7 +66,6 @@ describe("GET /api/update/badge", () => {
5266

5367
describe("GET /api/github/star", () => {
5468
test("is routed and reports one of the three known states", async () => {
55-
invalidateStarStatusCache();
5669
const { status, body } = await call("GET", "/api/github/star");
5770
expect(status).toBe(200);
5871
const star = body as Record<string, unknown>;
@@ -62,7 +75,6 @@ describe("GET /api/github/star", () => {
6275
});
6376

6477
test("never serializes gh output, tokens, or account identifiers", async () => {
65-
invalidateStarStatusCache();
6678
const { raw } = await call("GET", "/api/github/star");
6779
// `gh auth status` prints "Logged in to github.com account <name>" and the token
6880
// scopes; none of that may cross this boundary.

tests/storage-cleanup.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@ describe("listTrashEntries + restoreTrashEntry", () => {
14411441
has_user_event: 1,
14421442
archived: 1,
14431443
});
1444-
});
1444+
}, { timeout: 20_000 });
14451445

14461446
test.each([
14471447
["failAfterStateCommit", { failAfterStateCommit: true }, "db_reconcile_failed"],

0 commit comments

Comments
 (0)