Skip to content

test(run): cover the branch where the service dies by signal - #284

Merged
Antruly merged 3 commits into
mainfrom
test/281-signal-branch
Oct 19, 2026
Merged

Antruly merged 3 commits into
mainfrom
test/281-signal-branch

Conversation

@Antruly

@Antruly Antruly commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

Adds the regression test for uvp_run's signal-death branch, which #283 merged
without: a fixture that signals itself, asserting exit_code: null,
exit_signal: "SIGTERM", a process_exit unknown and the "was killed by
SIGTERM before the probe answered" wording. Skipped on Windows with the reason
in the test title — the platform reports a code for a terminated process.

Refs #281

Why this is a separate PR

The owner's review of #283 noted the signal branch was implemented but untested
and suggested exactly this fixture. I wrote it, but the commit (4839475) was
pushed after the owner merged #283 at a5ce0dd (5d0a3da), so it is on the
PR branch and not on main. A merged branch is not amended; this is the
change rebased onto main as its own branch, and the correction is recorded on
#283.

Problem and expected behavior

A child that ends by signal produced exit_code: null with exit_signal set.
Nothing pinned that: the other two fixtures exit with a code, so a regression
that dropped the signal — or reported a null code as a code — would have passed.
Expected: the observed signal and the null code are both asserted, and the
unknown names the signal.

Change and scope

  • tests/conformance/fixtures/killsItselfWithSigterm.mjs — the self-signalling
    child.
  • tests/conformance/handlers.test.ts — one case for it, skipped on Windows with
    the reason in the title.

No source change: this is coverage for behaviour already on main.

Contract and boundary impact

  • Earliest changed layer: none (tests only).
  • Tool contract changes: none.
  • libuvcpp feature mapping changes: none.
  • Generated-code impact: none.
  • Evidence, unknown, or error-contract semantics: none.
  • Generated metadata, package, or installation impact: none.

Expected effects: none — this PR changes test files only and starts no service
outside the test process.

Evidence and regression coverage

  • Tests added: the signal case. Observed on main as merged (5d0a3da) with the
    case added: npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts → 129 passed.
  • Mutation: forcing the reason back to resource_limit fails the exited-child
    case; dropping exitInfo() fails 2 cases (expected undefined to be 3,
    expected undefined to be null) — the second is this new one.
  • The exact command a reviewer can run:
    npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts
  • Remaining proof gaps: the case runs only on non-Windows hosts, for the reason
    the title names.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable in the output.
  • Coverage that was not examined is reported as unknown, not complete.
  • A check that could not run yields undetermined, not a fabricated yes or no.
  • Resource kinds and provenance survive the boundary unchanged.

Validation performed

  • npm run compile — exit 0.

  • npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts — 129 passed.

  • npm run typecheck — exit 0; npm run lint — exit 0 (see below); npm run format:check — exit 0.

  • Test isolation: both readiness cases now pass binary: svc.path (b3bd315).
    Before, filtering to either case spawned build/svc — the launcher an earlier
    case installed — and failed with an ENOENT unrelated to the case; observed by
    the owner at 4adf419. -t "signal death" and -t "bound-bound" each pass
    alone now, and the full file remains 108 passed.

  • The static lane's first run failed on max-lines-per-function in
    tests/conformance/handlers.test.ts: the three readiness cases pushed the
    launch and stop callback to 163 lines. Fixed at 4adf419 by moving them into
    their own describe — no test changed, and the suite stayed at 108 passed.

  • A template change was generated, built, and run against a real libuvcpp install.

  • Not applicable (no template or libuvcpp-facing change).

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • libuvcpp version floor impact: none.
  • New network exposure or listening behaviour: none.
  • Package or release metadata impact: none.

Author readiness

Human owner: @Antruly

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover the changed observable behaviour and its meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI or MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Antruly-dev-1 · AI Engineer 1 · agent:dev-ai-1

The exit-code branch is pinned by the wants-to-bind fixture; the branch where
the process ends by signal was implemented and schema-checked but had no test,
so a regression that reported a null code as an exit or lost the signal name
would have passed.

A fixture that signals itself covers it: the result must carry
`exit_signal: "SIGTERM"` with a null `exit_code`, and the unknown must say the
process was killed by the signal. Skipped on Windows with the reason in the
title, because the platform has no POSIX signals and reports a code instead.

Assisted-by: Claude Code (Antruly-dev-1)
The three readiness cases pushed the `launch and stop` describe's callback past
the 150-line limit, which the static lane rejected. They are a coherent group of
their own — what a run reports when the process ends before the probe answers —
so they move to their own describe rather than being trimmed. No test changed.

Assisted-by: Claude Code (Antruly-dev-1)
@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

🤖 Antruly-dev-1 · AI Engineer 1 · agent:dev-ai-1

The first CI run found one real problem, now fixed at 4adf419: the Static checks lane failed on max-lines-per-function in tests/conformance/handlers.test.ts — the three readiness cases grew the launch and stop callback to 163 lines against a 150 limit. Moved them into their own describe("a process that ends before readiness"); no test changed, npm run lint exits 0 locally, and the suite is 108 passed. CI re-ran on the new head.

@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Review at 4adf419

The signal branch is now covered, and the case passes.

  • npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts → 129 passed; npm run typecheck exit 0; npm run lint exit 0 (warnings only, pre-existing).
  • The fixture ends by SIGTERM, and the assertions pin exit_code: null (not 0), exit_signal: "SIGTERM", the process_exit reason, and the "was killed by SIGTERM before the probe answered" wording.
  • 4adf419 is a pure test reorganization: it moves the launch/stop cases into their own describe so the suite stays inside the lint budget. No behaviour change; the fixture and assertions are unchanged.
  • The Windows skip carries its reason in the test title, so a skipped run and a passing run do not look alike.

Non-blocking (my judgment): the case is not runnable in isolation. writeFakeBinary(workspace, "sigterm", …) installs build/sigterm, but uvp_run is called without binary, so it spawns the manifest's build/svc — the launcher installed by the earlier "refuses to bind" case. npx vitest run … -t 'signal death' therefore fails with spawn …/build/svc ENOENT (exit 1). It is fine in the full-file run, but a developer iterating on this one case gets an error unrelated to it. Passing binary: svc.path (or writing the launcher under svc) would make it self-contained; the silent-child case from #283 shares the pattern.

No blocking findings.

@Antruly
Antruly marked this pull request as ready for review October 19, 2026 23:29
Filtering the runner to either readiness case spawned the manifest's default
binary — `build/svc`, installed by the earlier refuses-to-bind case — instead of
the fixture the case had just written, so `-t 'signal death'` failed with an
ENOENT for a binary the case never created. Passing `binary: svc.path` makes
each case self-contained, which is also why the silent-child case is fixed here:
it had the same dependency.

Assisted-by: Claude Code (Antruly-dev-1)
@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

🤖 Antruly-dev-1 · AI Engineer 1 · agent:dev-ai-1

You were right about the isolation, and it was not only the signal case: the silent-child case had the same dependency on build/svc. Both now pass binary: svc.path (b3bd315), so each names the fixture it just wrote.

Evidence at b3bd315: npx vitest run tests/conformance/handlers.test.ts -t "signal death" → 1 passed | 107 skipped; -t "bound-bound" → 1 passed | 107 skipped; full file → 108 passed; npm run lint and npm run typecheck exit 0. The PR body records the finding and the fix. CI is re-running on this head.

@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Review at b3bd315

The signal branch is covered, and the case is now self-contained.

  • npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts → 129 passed; npx vitest run tests/conformance/handlers.test.ts -t 'signal death' → 1 passed on its own. The earlier head failed that filter with spawn …/build/svc ENOENT; b3bd315 passes binary: svc.path, so the case names the fixture it wrote (the same fix is applied to the silent-running case).
  • The fixture ends by SIGTERM, and the assertions pin exit_code: null (not 0), exit_signal: "SIGTERM", the process_exit reason, and the "was killed by SIGTERM before the probe answered" wording.
  • 4adf419 is a pure test reorganization to stay inside the lint budget; b3bd315 adds the binary path. npm run typecheck and npm run lint exit 0 (warnings only, pre-existing).
  • The Windows skip carries its reason in the test title, so a skipped run and a passing run do not look alike.

No blocking findings.

@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

🤖 Antruly-dev-1 · AI Engineer 1 · agent:dev-ai-1

Nothing has changed at b3bd315 since your review — the tree is the one you tested, CI is green (15 pass, the two classifier lanes skipped), and the PR body matches. The approval is the only gate left.

@Antruly

Antruly commented Oct 19, 2026

Copy link
Copy Markdown
Owner Author

Approved.

Reviewed head b3bd315f11c7be38a567841408154d34bc6fb739.

  • Fixes the coverage gap from feat(run): report the exit status when a service dies before readiness #283: the SIGTERM fixture asserts exit_code: null, exit_signal: "SIGTERM", the process_exit reason, and the wording. Windows skip reason is in the title.
  • npx vitest run tests/conformance/handlers.test.ts src/domain/unknown.test.ts → 129 passed; the signal case also passes when the runner is filtered to it (-t 'signal death'), because b3bd315 passes binary: svc.path.
  • npm run typecheck, npm run lint exit 0 (warnings only, pre-existing); all selected lanes green at this head; readiness declaration present.
  • git merge-tree --write-tree origin/main pr284 exit 0 against 5d0a3da.

Merging with --match-head-commit.

@Antruly
Antruly merged commit a2ad225 into main Oct 19, 2026
18 of 20 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