From 0ade229eb895f164f25203069fe4a02c2a03e254 Mon Sep 17 00:00:00 2001 From: omartelo Date: Sun, 13 Sep 2026 17:30:41 -0300 Subject: [PATCH] test: read every report before asserting on the stub A hook exiting does not mean the stub has read its report. When the test process is starved past curl's --max-time 1, curl gives up with the request already sent, and the child's exit can reach the event loop before that connection is accepted, so the test counted 0 requests. runHook now settles the stub first: one more connection drains the FIFO listen queue, then every accepted socket closing means its request was read. --- tests/contract.mjs | 44 +++++++++++++++++++++++++++++++++++++++++--- tests/hooks.test.mjs | 6 +++++- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/tests/contract.mjs b/tests/contract.mjs index dfba45f..ad73f2d 100644 --- a/tests/contract.mjs +++ b/tests/contract.mjs @@ -10,7 +10,9 @@ // clients are held to the same lines. import assert from 'node:assert/strict' +import { once } from 'node:events' import { createServer } from 'node:http' +import { connect } from 'node:net' import { readFileSync } from 'node:fs' import path from 'node:path' import { fileURLToPath } from 'node:url' @@ -173,8 +175,23 @@ export function assertContractHonoured(endpoint, request) { // ------------------------------------------------------------------ harness -- +const liveStubs = new Set() + +/** + * Resolves once every live stub has read each connection made to it so far. + * + * A hook exiting does not mean its report was read. When the stub's process is + * starved past curl's --max-time, curl gives up with the request already in the + * socket, and the child's exit can reach the event loop before that connection + * is even accepted. Connecting once more and waiting to accept it drains the + * listen queue, which is FIFO; each accepted socket closing means its request + * was read. Call it after a hook exits and before asserting on `requests`. + */ +export const settleStubs = () => Promise.all([...liveStubs].map((stub) => stub.settle())) + export async function startStub({ status = 204 } = {}) { const requests = [] + const sockets = new Set() const server = createServer((req, res) => { let raw = '' req.setEncoding('utf8') @@ -186,12 +203,33 @@ export async function startStub({ status = 204 } = {}) { res.writeHead(status).end() }) }) + server.on('connection', (socket) => { + sockets.add(socket) + socket.on('close', () => sockets.delete(socket)) + }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) - return { + const port = server.address().port + + const settle = async () => { + const probe = connect(port, '127.0.0.1') + await once(probe, 'connect') + const accepted = () => [...sockets].some((s) => s.remotePort === probe.localPort) + while (!accepted()) await once(server, 'connection') + probe.destroy() + await Promise.all([...sockets].map((s) => once(s, 'close'))) + } + + const stub = { requests, - port: server.address().port, - close: () => new Promise((resolve) => server.close(resolve)), + port, + settle, + close: () => { + liveStubs.delete(stub) + return new Promise((resolve) => server.close(resolve)) + }, } + liveStubs.add(stub) + return stub } export async function withStub(fn, options) { diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index 1b688d7..648e313 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -26,6 +26,7 @@ import { assertContractHonoured, lichEnv, rejected, + settleStubs, startStub, withStub, } from './contract.mjs' @@ -75,7 +76,9 @@ function runHook(command, { env = {}, stdin = '', pluginRoot = true, tmpdir = nu }) child.stdin.on('error', () => {}) // a script that reads no stdin is fine child.stdin.end(stdin) - child.on('close', (code) => resolve({ code, stdout, stderr })) + child.on('close', (code) => + settleStubs().then(() => resolve({ code, stdout, stderr })), + ) }) } @@ -654,6 +657,7 @@ for (const state of ['busy', 'done', 'idle']) { child.stdin.on('error', () => {}) const code = await new Promise((resolve) => child.on('close', resolve)) child.stdin.destroy() + await settleStubs() assert.equal(code, 0) assert.equal(stub.requests.length, 1) assert.equal(assertContractHonoured('/hook', stub.requests[0]).body.state, state)