From 20cdd16172fbbb6d8ad78fc83aeb53d725a15f83 Mon Sep 17 00:00:00 2001 From: sebi Date: Tue, 8 Sep 2026 12:33:02 -0500 Subject: [PATCH] fix(test): stop awaiting the bad-credentials login through expect().rejects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "a persona whose credentials are wrong fails the run instead of opening it signed out" has been failing on `main` since 2026-09-02 (runs 33969855127, 33647731476), timing out at exactly 30,000ms. The cause is the assertion, not the behaviour it asserts. Awaiting a long, multi-round-trip browser flow THROUGH the `.rejects` matcher starves the loop driving it, so every CDP call inside the pending promise costs ~1s. The login types 20 characters and presses 6 keys, so it never finished inside the test's 30s ceiling. Measured, same `performLogin` / adapter / page, only the await differing: awaited directly 1.4s awaited via .rejects 31.5s `login.ts`'s `NAVIGATION_SETTLE_MS` note already chased this timeout once and fixed a real contributor (a 30s idle wait, now 5s). It was not the whole cost — the settle wait measures 3ms here, while the typing measured 32s. Catching the rejection with try/catch and asserting on the caught error takes the flow out of the matcher: 30s timeout -> 1.84s, well inside the existing ceiling, so no timeout needed raising. Left `.rejects` alone everywhere else in the suite: every other use wraps a call that fails on one round trip or none, where a single stall costs nothing. The rule is about how much work is in flight, not about the matcher. Verified: `bun test src/services` 127/127, lint clean, typecheck clean. The 29 failures in browser-adapter.integration.test.ts are pre-existing and environmental (the dummy/web fixture CI builds is not built locally) — they fail identically on stock main. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015KQipy9s9tJFkGBeGWnZCX --- src/services/session-builder.test.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/services/session-builder.test.ts b/src/services/session-builder.test.ts index 90c4eff..ac06f8c 100644 --- a/src/services/session-builder.test.ts +++ b/src/services/session-builder.test.ts @@ -550,7 +550,27 @@ function serveLoginApp(credentials: { email: string; password: string }) { as: 'admin', }); try { - await expect(built.open()).rejects.toThrow(AuthError); + // Deliberately NOT `await expect(built.open()).rejects.toThrow(AuthError)`. + // + // Awaiting a long, multi-round-trip browser flow THROUGH the `.rejects` + // matcher starves the loop driving it: every CDP call inside the pending + // promise then costs ~1s. Measured on this exact login — the same + // `performLogin`, same adapter, same page — 1.4s awaited directly, 31.5s + // awaited via `.rejects`. That is what timed this test out at exactly + // 30,000ms on `main` (runs 33969855127 and 33647731476): not the behaviour + // it asserts, but the way it awaited it. + // + // `.rejects` stays fine everywhere else in this suite — every other use + // wraps a call that fails on ONE round trip (or none), where a single + // stall costs nothing. The rule is about how much work is in flight, not + // about the matcher. + let thrown: unknown; + try { + await built.open(); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(AuthError); } finally { await built.session.close(); server.stop(true);