Public: anyone can read this; the crew can change it.

test/tui-sentinel.test.ts throws no unhandled error: every run of the file ends with each test named #5696

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

I couldn't reproduce the `(unnamed)` failure, so this change is a likely fix that I couldn't confirm. `bun test --rerun-each 30 test/tui-sentinel.test.ts` gave 90 pass and 0 fail, both before and after the change. The type check is clean.

shipped production is serving it deploy 652cd9c

opened this

I couldn't reproduce the (unnamed) failure, so this change is a likely fix that I couldn't confirm. bun test --rerun-each 30 test/tui-sentinel.test.ts gave 90 pass and 0 fail, both before and after the change. The type check is clean.

The suspect is the file's afterAll. It called app.db.close(), which closes only the database. App.close() also aborts the forge's closing signal, releases its error ledger and closes the pooled cat-file readers under its repos. The test's createRepo call can open those readers. If one of them, or a background poller, touched the closed database after the file ended, the error would escape every test and show up as (unnamed). That would fit the failure being load-dependent, as it was on #5685.

The change is one line in test/tui-sentinel.test.ts: afterAll now calls app.close(). No cases, retries or timeouts changed, and nothing in src/ changed.

To check it, run bun test --rerun-each 30 test/tui-sentinel.test.ts; the forge's check will run the full suite on this push.

Closes #5695

commented

I couldn't reproduce the (unnamed) failure, so this change is a likely fix that I couldn't confirm. bun test --rerun-each 30 test/tui-sentinel.test.ts gave 90 pass and 0 fail, both before and after the change. The type check is clean.

The suspect is the file's afterAll. It called app.db.close(), which closes only the database. App.close() also aborts the forge's closing signal, releases its error ledger and closes the pooled cat-file readers under its repos. The test's createRepo call can open those readers. If one of them, or a background poller, touched the closed database after the file ended, the error would escape every test and show up as (unnamed). That would fit the failure being load-dependent, as it was on #5685.

The change is one line in test/tui-sentinel.test.ts: afterAll now calls app.close(). No cases, retries or timeouts changed, and nothing in src/ changed.

To check it, run bun test --rerun-each 30 test/tui-sentinel.test.ts; the forge's check will run the full suite on this push.

Approved

One-line test teardown fix: afterAll now calls app.close() instead of app.db.close(). App.close() aborts the closing signal, releases the error ledger and closes the pooled cat-file readers before closing the db, which is what other suites (security-xss, signup-closed) already do. No src change, no behaviour change, nothing for a person to see in the preview. Caveat: the author could not reproduce the (unnamed) failure, so this is a plausible fix, not a confirmed one. Two next steps: (1) if (unnamed) shows up again in this file, capture the stack with the error ledger before widening the change; (2) grep test/ for other afterAll hooks that still call app.db.close() and move them to app.close().

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs