Skip to content

Commit dff2221

Browse files
committed
refactor(test): extract pollUntil/pollForLength to test/lib/polling
Removes bespoke waitForQuestionCount / waitForPermissionCount helpers from run-events.test.ts. Future tests that need to poll for a service list length can use the shared pollForLength helper instead of writing yet another local 100-iteration loop. Addresses audit finding F3 (codex-5.3 diamond review, 2026-04-22).
1 parent 6567721 commit dff2221

2 files changed

Lines changed: 52 additions & 37 deletions

File tree

packages/opencode/test/cli/run-events.test.ts

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner"
33
import { Cause, Effect, Exit, Fiber, Layer } from "effect"
44
import { testEffect } from "../lib/effect"
55
import { provideTmpdirInstance } from "../fixture/fixture"
6+
import { pollForLength, pollUntil } from "../lib/polling"
67
import { Question } from "../../src/question"
78
import { Permission } from "../../src/permission"
89
import { Session } from "../../src/session"
@@ -20,32 +21,6 @@ const it = testEffect(
2021
),
2122
)
2223

23-
const waitForQuestionCount = (
24-
question: Question.Interface,
25-
count: number,
26-
): Effect.Effect<ReadonlyArray<Question.Request>, Error> =>
27-
Effect.gen(function* () {
28-
for (const _ of Array.from({ length: 100 })) {
29-
const pending = yield* question.list()
30-
if (pending.length === count) return pending
31-
yield* Effect.sleep("10 millis")
32-
}
33-
return yield* Effect.fail(new Error(`timed out waiting for ${count} question(s)`))
34-
})
35-
36-
const waitForPermissionCount = (
37-
permission: Permission.Interface,
38-
count: number,
39-
): Effect.Effect<ReadonlyArray<Permission.Request>, Error> =>
40-
Effect.gen(function* () {
41-
for (const _ of Array.from({ length: 100 })) {
42-
const pending = yield* permission.list()
43-
if (pending.length === count) return pending
44-
yield* Effect.sleep("10 millis")
45-
}
46-
return yield* Effect.fail(new Error(`timed out waiting for ${count} permission(s)`))
47-
})
48-
4924
describe("cli/run-events", () => {
5025
it.live("auto-rejects question.asked for the root session (non-attach, non-json)", () =>
5126
provideTmpdirInstance(() =>
@@ -196,7 +171,7 @@ describe("cli/run-events", () => {
196171
})
197172
.pipe(Effect.forkScoped)
198173

199-
const pending = yield* waitForQuestionCount(question, 1)
174+
const pending = yield* pollForLength(() => question.list(), 1)
200175

201176
expect(handler.stats.autoRejectedQuestions).toBe(0)
202177
expect(pending[0].sessionID).toBe(unrelatedSessionID)
@@ -244,7 +219,7 @@ describe("cli/run-events", () => {
244219
})
245220
.pipe(Effect.forkScoped)
246221

247-
const pending = yield* waitForQuestionCount(question, 1)
222+
const pending = yield* pollForLength(() => question.list(), 1)
248223
expect(pending[0].sessionID).toBe(deepSessionID)
249224
expect(handler.stats.autoRejectedQuestions).toBe(0)
250225

@@ -371,15 +346,15 @@ describe("cli/run-events", () => {
371346
})
372347

373348
const yFiber = yield* askPermission(y.id).pipe(Effect.forkScoped)
374-
const firstPending = yield* waitForPermissionCount(permission, 1)
349+
const firstPending = yield* pollForLength(() => permission.list(), 1)
375350
expect(firstPending[0].sessionID).toBe(y.id)
376351
expect(handler.stats.autoRejectedPermissions).toBe(0)
377352
yield* permission.reply({ requestID: firstPending[0].id, reply: "once" })
378353
const yExit = yield* Fiber.await(yFiber)
379354
expect(Exit.isSuccess(yExit)).toBe(true)
380355

381356
const xFiber = yield* askPermission(x.id).pipe(Effect.forkScoped)
382-
const secondPending = yield* waitForPermissionCount(permission, 1)
357+
const secondPending = yield* pollForLength(() => permission.list(), 1)
383358
expect(secondPending[0].sessionID).toBe(x.id)
384359
expect(handler.stats.autoRejectedPermissions).toBe(0)
385360
yield* permission.reply({ requestID: secondPending[0].id, reply: "once" })
@@ -426,13 +401,10 @@ describe("cli/run-events", () => {
426401
}),
427402
)
428403

429-
yield* Effect.gen(function* () {
430-
for (const _ of Array.from({ length: 100 })) {
431-
if (replies.length === 1) return
432-
yield* Effect.sleep("10 millis")
433-
}
434-
return yield* Effect.fail(new Error("timed out waiting for permission.replied event"))
435-
})
404+
yield* pollUntil(
405+
() => Effect.sync(() => (replies.length === 1 ? true : undefined)),
406+
{ label: "permission.replied event" },
407+
)
436408

437409
expect(Exit.isSuccess(exit)).toBe(true)
438410
expect(handler.stats.autoRejectedPermissions).toBe(0)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Generic "wait for predicate" helper for test fiber synchronization.
2+
// Replaces per-file bespoke waitForXxx polling loops so the polling
3+
// interval + timeout budget stays consistent across the suite.
4+
5+
import { Effect } from "effect"
6+
7+
export interface PollOptions {
8+
readonly iterations?: number // default 100
9+
readonly intervalMillis?: number // default 10
10+
readonly label?: string // for timeout error message
11+
}
12+
13+
export function pollUntil<A>(
14+
probe: () => Effect.Effect<A | undefined>,
15+
opts: PollOptions = {},
16+
): Effect.Effect<A, Error> {
17+
const iterations = opts.iterations ?? 100
18+
const interval = `${opts.intervalMillis ?? 10} millis` as const
19+
const label = opts.label ?? "pollUntil predicate"
20+
return Effect.gen(function* () {
21+
for (const _ of Array.from({ length: iterations })) {
22+
const value = yield* probe()
23+
if (value !== undefined) return value
24+
yield* Effect.sleep(interval)
25+
}
26+
return yield* Effect.fail(new Error(`timed out waiting for ${label}`))
27+
})
28+
}
29+
30+
export function pollForLength<A>(
31+
probe: () => Effect.Effect<ReadonlyArray<A>>,
32+
count: number,
33+
opts: PollOptions = {},
34+
): Effect.Effect<ReadonlyArray<A>, Error> {
35+
return pollUntil(
36+
() =>
37+
Effect.gen(function* () {
38+
const list = yield* probe()
39+
return list.length === count ? list : undefined
40+
}),
41+
{ ...opts, label: opts.label ?? `list length=${count}` },
42+
)
43+
}

0 commit comments

Comments
 (0)