From f0617c9468a97a6b81b9cd236c5ced8a8b65c403 Mon Sep 17 00:00:00 2001 From: garethx Date: Wed, 12 Aug 2026 12:41:37 +0100 Subject: [PATCH] Test the dashboard bundle, with no toolchain to maintain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #9. `dist/index.js` ships in the wheel and had no tests of any kind. It was also invisible in the 86% figure, which is Python only — so the number read better than the coverage was. Takes the issue's preferred option. The bundle gets React, its hooks, its components and its fetch from the injected SDK, so faking that SDK and running the file under `node:vm` exercises the shipped bytes with no package.json, no lockfile and no second ecosystem. `createElement` returns plain objects rather than rendering: every question here is about the tree and the requests, and none of them need a DOM. Node's own test runner runs it, so CI adds a job and nothing else. 15 tests, each one mutation-checked. Inverting the pause/resume expression, inverting only its label, dropping `disabled: busy`, removing the SDK guard, changing the retry endpoint and typo'ing the registered name all now fail. Two of them failed to catch their mutation on the first pass, and both were the test's fault rather than the code's: * the SDK-guard test could not express "this host has no SDK", because the harness always built a complete one and merged the caller's over it. It was asserting that a valid host loads. * the busy-on-failure test asserted no button was left disabled, and passed because a failed action swaps the whole page for the error card — which has no disabled buttons at all. It would have passed against anything. The second is worth recording: the issue expected `setBusy(false)` in `act()`'s catch to be what stops the panel wedging. It is not. The error card replaces the page and its Retry has no `disabled` binding, so the way back is open whatever `busy` holds. The test now asserts the recovery that actually exists. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 14 ++ docs/development.md | 15 ++ tests/dashboard/bundle.test.mjs | 271 ++++++++++++++++++++++++++++++++ tests/dashboard/harness.mjs | 206 ++++++++++++++++++++++++ 4 files changed, 506 insertions(+) create mode 100644 tests/dashboard/bundle.test.mjs create mode 100644 tests/dashboard/harness.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69d84ae..9eb89fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,20 @@ jobs: - run: pip install -e '.[dev]' - run: ruff check --output-format=github . + dashboard: + # The dashboard bundle ships in the wheel and its two action buttons + # mutate a live Hookdeck project, so it needs a check of its own — the + # Python suite cannot see it. No npm install: the bundle takes everything + # from the injected SDK, so a fake one under node:vm exercises the real + # file, and node's own test runner runs it. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: "22" + - run: node --test "tests/dashboard/*.test.mjs" + test: runs-on: ubuntu-latest strategy: diff --git a/docs/development.md b/docs/development.md index 8f4cd39..630863d 100644 --- a/docs/development.md +++ b/docs/development.md @@ -14,6 +14,21 @@ The tests stub the Hermes internals the adapter imports (`tests/hermes_stub.py`) so the ingest path — verification, dedup, admission control, ack modes, outcome reporting — is exercised without a Hermes checkout. +The dashboard bundle is JavaScript, so pytest cannot see it. It has its own +suite, run by node's built-in test runner: + +```bash +node --test "tests/dashboard/*.test.mjs" +``` + +No `package.json`, no install, nothing to build. The bundle takes React, its +hooks and its fetch from the injected `window.__HERMES_PLUGIN_SDK__`, so +`tests/dashboard/harness.mjs` fakes that SDK and runs the shipped file under +`node:vm` — the real `dist/index.js`, byte for byte. `createElement` returns +plain objects instead of rendering, because the questions worth asking are +which endpoint a button posts to and whether it is disabled, and neither needs +a DOM. + ## Code layout | Module | What lives there | diff --git a/tests/dashboard/bundle.test.mjs b/tests/dashboard/bundle.test.mjs new file mode 100644 index 0000000..071425a --- /dev/null +++ b/tests/dashboard/bundle.test.mjs @@ -0,0 +1,271 @@ +/** + * Tests for the shipped dashboard bundle. + * + * Run with `node --test tests/dashboard/`. No dependencies: the bundle takes + * everything it uses from the injected SDK, so a fake one is enough — see + * harness.mjs for why that beats adding a JS toolchain to test one file. + * + * The emphasis is the two buttons that mutate a live Hookdeck project. A + * mislabelled one is invisible on screen and only shows up in traffic. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { mount, mountRaw, nodesOfType, allText, buttonsLabelled } from "./harness.mjs"; + +const OVERVIEW = "/api/plugins/hookdeck/overview"; + +/** An overview payload with the fields the page reads, overridable per test. */ +function overview({ hookdeck = {}, local = {} } = {}) { + return { + hookdeck: { + configured: true, + depth: { max_depth: 0 }, + failed: [], + issues: [], + connections: [], + other_connection_count: 0, + ...hookdeck, + }, + local: { exists: true, counts: {}, failures: [], stranded: [], ...local }, + }; +} + +async function page(payload, extra = {}) { + const app = mount({ responses: { [OVERVIEW]: payload, ...extra } }); + await app.settle(); + return app; +} + +// ---------------------------------------------------------------------- +// The pause/resume toggle +// ---------------------------------------------------------------------- +// +// The label and the endpoint are both derived from `c.paused`. Invert that +// expression and the UI still reads correctly while doing the opposite — a +// button labelled "Pause" that resumes production traffic. + +test("a paused connection offers Resume, and resuming is what it posts", async () => { + const app = await page( + overview({ hookdeck: { connections: [{ id: "web_1", name: "stripe", paused: true }] } }), + ); + + const [button] = buttonsLabelled(app.tree, "Resume"); + assert.ok(button, "a paused connection should offer Resume"); + assert.equal(buttonsLabelled(app.tree, "Pause").length, 0); + + button.props.onClick(); + assert.deepEqual(app.requests.at(-1), { + path: "/api/plugins/hookdeck/connections/web_1/resume", + method: "POST", + }); +}); + +test("an active connection offers Pause, and pausing is what it posts", async () => { + const app = await page( + overview({ hookdeck: { connections: [{ id: "web_1", name: "stripe", paused: false }] } }), + ); + + const [button] = buttonsLabelled(app.tree, "Pause"); + assert.ok(button, "an active connection should offer Pause"); + assert.equal(buttonsLabelled(app.tree, "Resume").length, 0); + + button.props.onClick(); + assert.deepEqual(app.requests.at(-1), { + path: "/api/plugins/hookdeck/connections/web_1/pause", + method: "POST", + }); +}); + +test("each connection acts on its own id", async () => { + // One shared handler closing over the wrong row would pause a connection + // the operator did not click. + const app = await page( + overview({ + hookdeck: { + connections: [ + { id: "web_1", name: "a", paused: false }, + { id: "web_2", name: "b", paused: false }, + ], + }, + }), + ); + + const buttons = buttonsLabelled(app.tree, "Pause"); + assert.equal(buttons.length, 2); + buttons[1].props.onClick(); + assert.match(app.requests.at(-1).path, /web_2\/pause$/); +}); + +// ---------------------------------------------------------------------- +// Retry is a redelivery +// ---------------------------------------------------------------------- + +test("retrying a failed delivery posts to that event's retry endpoint", async () => { + const app = await page( + overview({ hookdeck: { failed: [{ id: "evt_1", response_status: 500, attempts: 2 }] } }), + ); + + const [button] = buttonsLabelled(app.tree, "Retry"); + button.props.onClick(); + assert.deepEqual(app.requests.at(-1), { + path: "/api/plugins/hookdeck/events/evt_1/retry", + method: "POST", + }); +}); + +test("retrying a failed agent run posts for that run's event", async () => { + const app = await page( + overview({ + local: { failures: [{ event_id: "evt_9", route: "stripe", status: "failed", agent_attempts: 2 }] }, + }), + ); + + const [button] = buttonsLabelled(app.tree, "Retry"); + button.props.onClick(); + assert.match(app.requests.at(-1).path, /events\/evt_9\/retry$/); +}); + +// ---------------------------------------------------------------------- +// `busy` — the guard against a double redelivery +// ---------------------------------------------------------------------- + +test("every action is disabled while a request is in flight", async () => { + // This plugin exists so one event runs the agent once. A double-click that + // fires two retries is that guarantee failing in the UI instead of the + // adapter. + const app = await page( + overview({ + hookdeck: { + failed: [{ id: "evt_1", response_status: 500, attempts: 1 }], + connections: [{ id: "web_1", name: "a", paused: false }], + }, + local: { failures: [{ event_id: "evt_9", route: "r", status: "failed", agent_attempts: 1 }] }, + }), + // Never resolves, so `busy` stays true and the tree can be inspected + // mid-flight. + { "/api/plugins/hookdeck/events/evt_1/retry": () => new Promise(() => {}) }, + ); + + buttonsLabelled(app.tree, "Retry")[0].props.onClick(); + const actionable = nodesOfType(app.tree, "Button").filter( + (b) => b.props.onClick && "disabled" in b.props, + ); + assert.ok(actionable.length >= 3, "retry, retry and pause all carry a disabled flag"); + for (const button of actionable) { + assert.equal(button.props.disabled, true); + } +}); + +test("a failed action leaves a way back rather than a dead panel", async () => { + // A failed action swaps the whole page for the error card, so the recovery + // that matters is that card's Retry — it reloads the overview and restores + // the page. (`busy` is not the guard here: the error card's button has no + // `disabled` binding, so it stays clickable regardless.) + const overviewPayload = overview({ + hookdeck: { connections: [{ id: "web_1", name: "a", paused: false }] }, + }); + const app = await page(overviewPayload, { + "/api/plugins/hookdeck/connections/web_1/pause": () => Promise.reject(new Error("nope")), + }); + + buttonsLabelled(app.tree, "Pause")[0].props.onClick(); + for (let i = 0; i < 20; i++) await Promise.resolve(); + assert.match(allText(app.tree), /nope/, "the failure is shown, not swallowed"); + + const [back] = buttonsLabelled(app.tree, "Retry"); + assert.ok(back, "a failed action must leave something to click"); + assert.equal(back.props.disabled, undefined, "the way back is never disabled"); + + back.props.onClick(); + await app.settle(); + assert.match(allText(app.tree), /Connections/, "the page comes back"); +}); + +// ---------------------------------------------------------------------- +// Loading against a host that does not have what it needs +// ---------------------------------------------------------------------- + +test("an SDK older than the bundle is a quiet no-op, not a throw", async () => { + // Upgrading the plugin before Hermes is the normal order, so the bundle + // will meet hosts that lack what it uses. Throwing here goes into the + // host's bundle loader and takes the whole dashboard down with it. + const hosts = [ + ["no SDK at all", {}], + ["an SDK but no plugin registry", { __HERMES_PLUGIN_SDK__: { React: {} } }], + ["a registry with no register()", { __HERMES_PLUGIN_SDK__: { React: {} }, __HERMES_PLUGINS__: {} }], + ]; + for (const [what, window] of hosts) { + let registered; + assert.doesNotThrow(() => { + registered = mountRaw(window); + }, `should load quietly against ${what}`); + assert.equal(registered.size, 0, `should register nothing against ${what}`); + } +}); + +test("the tab registers under the name its manifest declares", async () => { + const app = mount({ responses: { [OVERVIEW]: overview() } }); + assert.ok(app.registered.has("hookdeck"), "the host pairs the component to the tab by this name"); +}); + +// ---------------------------------------------------------------------- +// States an operator will actually see +// ---------------------------------------------------------------------- + +test("a missing API key explains itself and asks for nothing else", async () => { + const app = await page({ hookdeck: { configured: false }, local: {} }); + const text = allText(app.tree); + assert.match(text, /HOOKDECK_EG_API_KEY/); + assert.equal(nodesOfType(app.tree, "Button").length, 0, "nothing to click without a key"); +}); + +test("an empty queue says so rather than rendering a bare panel", async () => { + const app = await page(overview()); + assert.match(allText(app.tree), /None\./); + assert.match(allText(app.tree), /No connection matches a configured route yet\./); +}); + +test("hidden connections are counted without justifying themselves", async () => { + const app = await page(overview({ hookdeck: { other_connection_count: 26 } })); + const text = allText(app.tree); + assert.match(text, /26 other connection\(s\)/); + assert.match(text, /Only connections matching a configured route appear here\./); +}); + +test("stranded runs are surfaced with what to do about them", async () => { + const app = await page(overview({ local: { stranded: [{ event_id: "evt_1" }] } })); + assert.match(allText(app.tree), /still marked running after an hour/); + assert.match(allText(app.tree), /Restarting the gateway retries them/); +}); + +// ---------------------------------------------------------------------- +// Error rendering +// ---------------------------------------------------------------------- + +test("a rejection is shown whether or not it is an Error", async () => { + // `String((e && e.message) || e)` is written for both shapes; a plain + // string rejection is the one less likely to have been tried. + for (const [thrown, expected] of [ + [new Error("boom"), /boom/], + ["plain string failure", /plain string failure/], + ]) { + const app = mount({ + responses: { [OVERVIEW]: () => Promise.reject(thrown) }, + }); + await app.settle(); + assert.match(allText(app.tree), expected); + // And a way back, rather than a dead panel. + assert.ok(buttonsLabelled(app.tree, "Retry").length >= 1); + } +}); + +test("the error panel's Retry reloads the overview", async () => { + const app = mount({ responses: { [OVERVIEW]: () => Promise.reject(new Error("down")) } }); + await app.settle(); + const before = app.requests.length; + buttonsLabelled(app.tree, "Retry")[0].props.onClick(); + assert.equal(app.requests.at(-1).path, OVERVIEW); + assert.ok(app.requests.length > before); +}); diff --git a/tests/dashboard/harness.mjs b/tests/dashboard/harness.mjs new file mode 100644 index 0000000..7588ff3 --- /dev/null +++ b/tests/dashboard/harness.mjs @@ -0,0 +1,206 @@ +/** + * A fake Hermes plugin SDK, so the dashboard bundle can be tested as shipped. + * + * `dist/index.js` is committed rather than generated — no package.json, no + * bundler, no build step — and this keeps that property. It takes React, its + * hooks, its components and its fetch from `window.__HERMES_PLUGIN_SDK__`, so + * everything it touches is injectable: run it under `node:vm` with the SDK + * faked and you exercise the real file, byte for byte, with no dependencies. + * + * `createElement` returns plain objects rather than rendering, because every + * question worth asking here is about the tree and the requests — which + * endpoint a button posts to, whether it is disabled — and none of them need a + * DOM to answer. + */ + +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, join } from "node:path"; +import vm from "node:vm"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +export const BUNDLE = join(HERE, "..", "..", "hookdeck", "dashboard", "dist", "index.js"); + +/** A rendered node: `{type, props, children}` with `type` a string. */ +function createElement(type, props, ...children) { + const flat = []; + for (const child of children.flat(Infinity)) { + if (child !== null && child !== undefined && child !== false) flat.push(child); + } + return { + type: typeof type === "function" ? type.name || "fn" : String(type), + props: props || {}, + children: flat, + }; +} + +/** + * Hooks for a single component rendered repeatedly. + * + * State lives in a slot array keyed by call order — the same contract React + * relies on — so a setter can trigger a re-render and the next pass reads the + * updated value from the same slot. + */ +function makeHooks(rerender) { + let slots = []; + let cursor = 0; + const effects = []; + + return { + beginRender() { + cursor = 0; + effects.length = 0; + }, + runEffects() { + // Copied first: an effect may setState, which re-renders and refills + // the list while we are walking it. + for (const effect of [...effects]) effect(); + }, + hooks: { + useState(initial) { + const slot = cursor++; + if (slots.length <= slot) slots[slot] = initial; + const set = (next) => { + slots[slot] = typeof next === "function" ? next(slots[slot]) : next; + rerender(); + }; + return [slots[slot], set]; + }, + // Dependencies are ignored deliberately: this harness re-renders from + // scratch, so a stale-closure bug is not something it can observe, and + // pretending otherwise would be a test that lies about its own reach. + useCallback: (fn) => fn, + useEffect(fn) { + const slot = cursor++; + if (slots[slot] === undefined) { + slots[slot] = "ran"; + effects.push(fn); + } + }, + }, + reset() { + slots = []; + cursor = 0; + }, + }; +} + +/** + * Load the bundle with a faked SDK and render its page component. + * + * `responses` maps a request path to either a value to resolve or an `Error` + * to reject with — `{"/overview": {...}}`. + */ +export function mount({ responses = {}, sdk = {}, plugins = {} } = {}) { + const requests = []; + const registered = new Map(); + + const fetchJSON = (path, options) => { + requests.push({ path, method: (options && options.method) || "GET" }); + const answer = responses[path]; + if (answer instanceof Error) return Promise.reject(answer); + if (typeof answer === "function") return answer(); + return Promise.resolve(answer === undefined ? {} : answer); + }; + + let tree = null; + let component = null; + const hookStore = makeHooks(() => render()); + + function render() { + if (!component) return null; + hookStore.beginRender(); + tree = component({}); + return tree; + } + + const window = { + __HERMES_PLUGIN_SDK__: { + React: { createElement }, + // Plain strings: `createElement` stringifies a non-function type, so + // the node's `type` comes out as "Button" and stays greppable. + components: Object.fromEntries( + ["Card", "CardHeader", "CardTitle", "CardContent", "Badge", "Button"].map( + (name) => [name, name], + ), + ), + hooks: hookStore.hooks, + fetchJSON, + ...sdk, + }, + __HERMES_PLUGINS__: { + register: (name, fn) => registered.set(name, fn), + ...plugins, + }, + }; + + vm.runInNewContext(readFileSync(BUNDLE, "utf8"), { window, console }); + + component = registered.get("hookdeck") || null; + render(); + + return { + requests, + registered, + get tree() { + return tree; + }, + /** Render, then flush effects and any renders they cause. */ + async settle() { + hookStore.runEffects(); + for (let i = 0; i < 20; i++) await Promise.resolve(); + render(); + hookStore.runEffects(); + for (let i = 0; i < 20; i++) await Promise.resolve(); + return render(); + }, + }; +} + +/** + * Run the bundle against a window supplied verbatim. + * + * `mount` always builds a complete SDK, so it cannot express "this host does + * not have one" — which is exactly the case the bundle's opening guard exists + * for. Returns what the bundle registered, or throws if it threw. + */ +export function mountRaw(window) { + const registered = new Map(); + if (window && window.__HERMES_PLUGINS__ && !window.__HERMES_PLUGINS__.register) { + // left absent on purpose by the caller + } else if (window && window.__HERMES_PLUGINS__) { + window.__HERMES_PLUGINS__.register = (name, fn) => registered.set(name, fn); + } + vm.runInNewContext(readFileSync(BUNDLE, "utf8"), { window, console }); + return registered; +} + +/** Every node in the tree, depth first. */ +export function walk(node, out = []) { + if (!node || typeof node !== "object") return out; + out.push(node); + for (const child of node.children || []) walk(child, out); + return out; +} + +/** Nodes whose type matches, e.g. `nodesOfType(tree, "Button")`. */ +export function nodesOfType(tree, type) { + return walk(tree).filter((n) => n.type === type); +} + +/** Visible text of a node and everything under it. */ +export function textOf(node) { + return walk(node) + .flatMap((n) => n.children.filter((c) => typeof c === "string")) + .join(" "); +} + +/** All text in the tree, for "does it mention X" assertions. */ +export function allText(tree) { + return textOf(tree); +} + +/** Buttons whose own label matches `label` exactly. */ +export function buttonsLabelled(tree, label) { + return nodesOfType(tree, "Button").filter((b) => textOf(b).trim() === label); +}