Skip to content

Consolidate the integration suite under test/live - #9

Merged
garethx merged 4 commits into
mainfrom
test/consolidate-live
Aug 13, 2026
Merged

Consolidate the integration suite under test/live#9
garethx merged 4 commits into
mainfrom
test/consolidate-live

Conversation

@garethx

@garethx garethx commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #8. That PR added test/live/ but left test/integration.test.mjs
in place, so main currently carries two live-test conventions.

They cover different ground — source binding and destination selection in one;
options, signatures and delivery in the other — but nothing made that split
legible, and each carried its own API client, node contexts and cleanup.

What changes

  • test/integration.test.mjs becomes test/live/provisioning.test.mjs and drops
    its private plumbing for the shared harness.
  • Its resources now carry the run id, which brings them under the delete
    guard.
    They were previously removed by an exact-name match — which would have
    deleted a same-named production source — and the guard would have rejected
    them outright.
  • The delivery suite checks for the Hookdeck CLI on PATH rather than spawning
    it blind.
    CI installs neither the Hookdeck nor the Stripe CLI, so without
    this it fails with ENOENT and reads like a broken node.
  • CI runs the whole test/live/ directory in place of test:integration.
  • The hook context exposes warnings (the warn and info levels the node uses
    for operator-facing guidance), which is what the moved tests assert on.

One test added

Applying a secret to a source that already exists is a no-op without Update
Existing Source
— the source stays unverified while activation reports success.
Found by the Stripe suite against a real endpoint in #8, and pinned here where
the adoption rules live.

Verification

npm run test:live — 45 pass, 1 skip (delivery groups, not enabled on the test
org), exit 0, no leaked resources, no orphaned processes. npm test 106 pass.
Skip paths checked directly: no API key skips every suite; a missing CLI skips
delivery and Stripe.

Unrelated, and left alone: both nodes trip icon-prefer-themed-variants — the
icon is a single file:hookdeck.svg where it was the { light, dark } form, and
only hookdeck.svg remains on disk. That is a verification-programme rule, so
worth its own fix.

garethx and others added 4 commits August 13, 2026 15:40
The repository had two live-test conventions: test/integration.test.mjs and
test/live/. They cover different ground — source binding and destination
selection in one, options, signatures and delivery in the other — but nothing
made that split legible, and each carried its own API client, node contexts and
cleanup.

The integration suite moves to test/live/provisioning.test.mjs and drops its
private plumbing for the shared harness. Its resources are renamed to carry the
run id, which is what brings them under the delete guard: they were previously
removed by an exact-name match that would have deleted a same-named production
source, and the guard would have rejected them outright.

One test is added while the behaviour is in view. Applying a secret to a source
that already exists is a no-op without Update Existing Source, so the source
stays unverified while activation reports success — found by the Stripe suite
against a real endpoint, and pinned here where the adoption rules live.

CI now runs the whole live directory. The delivery suite needs the Hookdeck CLI
and the Stripe suite an authenticated Stripe CLI, neither of which the runner
installs, so both skip with a stated reason instead of failing on a missing
binary.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI failed on two counts, neither of which said anything about the node.

A cleanup read answered 429. The project allows 240 requests a minute and these
suites run back to back, so a burst is expected rather than exceptional — the
API helper now honours `retry-after` and retries. Connection lookups resolve the
source by name and query by `source_id` instead of listing 250 connections and
filtering client-side, which also removes a silent failure once a project holds
more connections than the page returns.

The request lookup after an event was read once, with no polling, so the delay
between the edge accepting an event and the record becoming queryable read as a
missing record.

Separately, the retry test was flaky here too. `hookdeck listen` health-checks
the local server, and that test answers 503 on purpose; the forwarder takes the
origin to be down and stops delivering, so the retry never arrives. Running the
CLI with --no-healthcheck leaves the deliberate 503 to mean only what the test
intends.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The retry test fired its event at the edge without checking the answer, then
waited two and a half minutes for a delivery. An ingest the edge refuses creates
no event at all, so that wait could only ever time out — and it reported a retry
that never arrived, which points at Hookdeck's retries rather than at the
request that was rejected a step earlier.

Every ingest these tests depend on is now asserted, so a refusal says so at the
point it happens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The API helper these tests call directly already retried a 429, but the helper
the *node* calls did not, so a rate-limited run failed inside the node with
whatever it made of the rejection. CI hit it on five tests at once.

That path also parsed every response body as JSON. Hookdeck answers some errors
in plain text, and n8n's own helper hands back a body it could not parse rather
than throwing, so strict parsing here invented a failure the node never meets in
production: a 429 surfaced as "Unexpected token 'T'".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@garethx
garethx merged commit 9306ddb into main Aug 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant