From b4084c5a2e381a2a08452e604544d3a10d19aff9 Mon Sep 17 00:00:00 2001 From: Yusuke Wada Date: Thu, 17 Sep 2026 12:36:21 +0900 Subject: [PATCH] fix(batch): fail a step without expect on a 4xx or 5xx --- README.md | 2 +- docs/agent-dx-log.md | 19 +++++++++++++ src/commands/batch/batch.test.ts | 47 ++++++++++++++++++++++++++++---- src/commands/batch/batch.ts | 33 ++++++++++++---------- src/commands/batch/index.ts | 1 + 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index f98e66f..3616504 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,7 @@ hono batch - <<'EOF' EOF ``` -One JSON object per line: `method`, `path`, `body`, `headers`, `expect`, `save`. `save` stores a value from the response body by dot path, and later steps use it as `{{id}}` (a whole-variable string keeps the saved type). `expect` declares the acceptance criteria: `status` matches exactly, `body` is a deep partial match (declared fields must match, extra response fields are ignored). The output carries the actual `status` and `body`, `pass` per step, and a `summary` — rerun until `failed` is 0. +One JSON object per line: `method`, `path`, `body`, `headers`, `expect`, `save`. `save` stores a value from the response body by dot path, and later steps use it as `{{id}}` (a whole-variable string keeps the saved type). `expect` declares the acceptance criteria: `status` matches exactly, `body` is a deep partial match (declared fields must match, extra response fields are ignored). The output carries the actual `status` and `body`, `pass` per step, and a `summary` — rerun until `failed` is 0. A step without `expect` passes on any 2xx or 3xx and fails on a 4xx or 5xx; to accept a 4xx on purpose, declare it with `expect.status`. ### `snapshot` diff --git a/docs/agent-dx-log.md b/docs/agent-dx-log.md index 398a974..9d35610 100644 --- a/docs/agent-dx-log.md +++ b/docs/agent-dx-log.md @@ -3,6 +3,25 @@ How measurements from [honojs/agent-dx](https://github.com/honojs/agent-dx) changed Hono CLI. Newest first. +## 2026-09-17: A 500 is not `failed: 0` + +**Experiment**: preparing the D1 A/B for `next.8` (bindings-aware +`request`/`batch`). On `next.7`, where `c.env` has no D1, `/users` +returns a 500. + +**Findings**: + +- A batch line without `expect` counted that 500 as `pass: true`, so + the summary said `failed: 0`. The verification tool reported a + broken endpoint as working; an agent in the baseline condition would + read that as done. +- The rule "no `expect` means facts only" was too literal. The facts + are still there, but the summary is what agents act on. + +**Change**: a step without `expect` now fails on a 4xx or 5xx, with a +`diff` line that says how to accept one on purpose +(`expect.status`). 2xx and 3xx pass as before. + ## 2026-09-07: The diff converges the laps **Experiment**: `next.7` on the large fixture, plus the wording A/B. diff --git a/src/commands/batch/batch.test.ts b/src/commands/batch/batch.test.ts index 43505bc..2cbc09b 100644 --- a/src/commands/batch/batch.test.ts +++ b/src/commands/batch/batch.test.ts @@ -127,16 +127,51 @@ describe('runBatch', () => { expect(result.steps[1].body).toEqual({ id: 1, name: 'Momo' }) }) - it('reports the status and body as facts', async () => { - const result = await runBatch(crudApp(), parseBatch('{"path":"/nope"}')) + it('passes a step without expect on a 2xx or 3xx', async () => { + const app = crudApp() + app.get('/moved', (c) => c.redirect('/users')) + const result = await runBatch(app, parseBatch('{"path":"/users"}\n{"path":"/moved"}')) expect(result.steps[0]).toEqual({ method: 'GET', - path: '/nope', - status: 404, - body: '404 Not Found', + path: '/users', + status: 200, + body: [], pass: true, }) - expect(result.summary).toEqual({ total: 1, passed: 1, failed: 0 }) + expect(result.steps[1].status).toBe(302) + expect(result.steps[1].pass).toBe(true) + expect(result.summary).toEqual({ total: 2, passed: 2, failed: 0 }) + }) + + it('fails a step without expect on a 4xx or 5xx', async () => { + const app = crudApp() + app.get('/boom', () => { + throw new Error('boom') + }) + const result = await runBatch(app, parseBatch('{"path":"/nope"}\n{"path":"/boom"}')) + expect(result.steps[0].status).toBe(404) + expect(result.steps[0].pass).toBe(false) + expect(result.steps[0].diff).toEqual([ + 'status: expected 2xx or 3xx, got 404 (set expect.status to accept it)', + ]) + expect(result.steps[0].suggestions).toEqual([ + 'See which routes matched: hono request /nope --trace', + ]) + expect(result.steps[1].status).toBe(500) + expect(result.steps[1].pass).toBe(false) + expect(result.steps[1].diff).toEqual([ + 'status: expected 2xx or 3xx, got 500 (set expect.status to accept it)', + ]) + expect(result.summary).toEqual({ total: 2, passed: 0, failed: 2 }) + }) + + it('passes a 4xx when expect.status declares it', async () => { + const result = await runBatch( + crudApp(), + parseBatch('{"path":"/users/999","expect":{"status":404}}') + ) + expect(result.steps[0].pass).toBe(true) + expect(result.steps[0].diff).toBeUndefined() }) it('checks expect.status and expect.body as a deep partial match', async () => { diff --git a/src/commands/batch/batch.ts b/src/commands/batch/batch.ts index 03bc4ef..977ba05 100644 --- a/src/commands/batch/batch.ts +++ b/src/commands/batch/batch.ts @@ -208,7 +208,9 @@ export const getByPath = (body: unknown, path: string): unknown => { * carries from step to step. Each result carries the facts — status * and body — and, when the step declares `expect`, the deterministic * check against them. Agents miss lines when they compare a spec - * table by eye, so the comparison belongs to the CLI. + * table by eye, so the comparison belongs to the CLI. A step without + * `expect` still fails on a 4xx or 5xx: a summary that reports a 500 + * as `failed: 0` reads as "it works". */ export const runBatch = async ( app: Hono, @@ -263,19 +265,22 @@ export const runBatch = async ( ...(step.expect === undefined ? {} : { expect: interpolate(step.expect, vars) }), } - if (result.expect !== undefined) { - const diff = [ - ...(result.expect.status !== undefined && response.status !== result.expect.status - ? [`status: expected ${result.expect.status}, got ${response.status}`] - : []), - ...(result.expect.body === undefined ? [] : subsetDiff(body, result.expect.body)), - ] - if (diff.length > 0) { - result.pass = false - result.diff = diff - if (response.status === 404) { - result.suggestions = [`See which routes matched: hono request ${path} --trace`] - } + const diff = + result.expect === undefined + ? response.status >= 400 + ? [`status: expected 2xx or 3xx, got ${response.status} (set expect.status to accept it)`] + : [] + : [ + ...(result.expect.status !== undefined && response.status !== result.expect.status + ? [`status: expected ${result.expect.status}, got ${response.status}`] + : []), + ...(result.expect.body === undefined ? [] : subsetDiff(body, result.expect.body)), + ] + if (diff.length > 0) { + result.pass = false + result.diff = diff + if (response.status === 404) { + result.suggestions = [`See which routes matched: hono request ${path} --trace`] } } diff --git a/src/commands/batch/index.ts b/src/commands/batch/index.ts index 0a571d3..62ecd3e 100644 --- a/src/commands/batch/index.ts +++ b/src/commands/batch/index.ts @@ -30,6 +30,7 @@ EOF`, 'Runs many requests in one call, in order, against one app instance — in-memory state carries between steps. One JSON object per line: {"method","path","body","headers","expect","save"}.', '"save" stores a value from the response body by dot path (e.g. {"id":".id"}), and later steps use it as {{id}}. A whole-variable string like "{{id}}" keeps the saved type.', 'Declare the acceptance criteria in "expect": {"status":201} and/or {"body":{...}} (a deep partial match — declared fields must match, extra response fields are ignored). Turn the spec into batch lines and rerun until "failed" is 0 — comparing a spec table by eye misses lines.', + 'A step without "expect" passes on any 2xx or 3xx and fails on a 4xx or 5xx. To accept a 4xx on purpose, declare it: {"expect":{"status":404}}.', 'A shared header from -H goes to every step. Prefer a heredoc over writing a file: the lines live in your context.', '--compact prints only the failed steps and the summary — use it when you only need the failed: 0 loop.', 'A failed step carries "diff": one line per mismatch (e.g. "body.name: expected \'Alice\', got \'Bob\'"). Fix what the diff names — no need to compare the bodies yourself.',