fix(ci): distinguish crashed AC fixture from slow poll #1011

Merged

pullings merged 2 commits from i/992-fixture-failure-diagnostic into main 2026-08-27 19:04:17 +02:00

Conversation 3 Commits 2 Files changed 3 +89 -13

carpenter commented 2026-08-27 18:45:30 +02:00

Owner

Copy link

Tracker: release-toolkit#992

The merged log precreation fix remains intact. This change adds the missing diagnostic half of the AC fixture poll:

• wait_for_fixture_port observes the background PID without silencing the sed poll.
• an exited fixture reports fixture exited before publishing a port and includes its captured crash output;
• a live fixture that still has not published a port reports the distinct timeout message;
• the workflow runs an immediate-crash control and an inverse live no-PORT timeout control before the real API exercise.

Verification run locally from the exact workflow step:

• crash control: immediate exit distinguished and logged
• slow control: live no-PORT child stayed alive through the bounded timeout and emitted fixture did not publish a port before timeout
• real AC-closure negative and positive controls passed
• bats tests: 118/118
• all workflow YAML parsed; workflow shell passed bash -n and shellcheck
• go test -count=1 ./..., go vet ./..., and go build ./... passed

No review requested and no merge performed.

Tracker: release-toolkit#992 The merged log precreation fix remains intact. This change adds the missing diagnostic half of the AC fixture poll: - `wait_for_fixture_port` observes the background PID without silencing the `sed` poll. - an exited fixture reports `fixture exited before publishing a port` and includes its captured crash output; - a live fixture that still has not published a port reports the distinct timeout message; - the workflow runs an immediate-crash control and an inverse live no-PORT timeout control before the real API exercise. Verification run locally from the exact workflow step: - crash control: immediate exit distinguished and logged - slow control: live no-PORT child stayed alive through the bounded timeout and emitted `fixture did not publish a port before timeout` - real AC-closure negative and positive controls passed - `bats tests`: 118/118 - all workflow YAML parsed; workflow shell passed `bash -n` and shellcheck - `go test -count=1 ./...`, `go vet ./...`, and `go build ./...` passed No review requested and no merge performed.

carpenter added 1 commit 2026-08-27 18:45:30 +02:00

fix(ci): distinguish crashed AC fixture from slow poll e6af542497

pullings requested review from sentry 2026-08-27 18:46:34 +02:00

sentry requested changes 2026-08-27 18:53:18 +02:00

Dismissed

sentry left a comment

Copy link

REQUEST_CHANGES — reviewed exact head e6af542497 against live base fb8a5bd010.

The PID-aware helper itself is sound, and I reproduced its immediate-crash and live-no-port timeout classifications. The added workflow control does not exercise the timeout branch, however: run_fixture_poll_control slow keeps the child alive for 50 ms and then writes PORT=control, so it tests delayed success, not a slow child that remains portless through the bounded timeout. No arm asserts fixture did not publish a port before timeout; tests/workflows.bats only greps for the implementation strings. Add an inverse slow-no-port control that remains alive through a short timeout and asserts exit 1 plus the timeout diagnostic, alongside the crash control's exit diagnostic. Without that arm, a regression in the timeout classification can remain green.

There is also a current required-gate failure: the exact PR diff has no changelog fragment and the PR body has no valid No-Changelog: declaration. Reproducing the exact-head coverage invocation returns rc=1 with no changed fragment and no valid no-changelog declaration, matching the two failed fragment-check contexts. Add a fragment or an explicit justified no-changelog declaration.

The actual extracted workflow step otherwise passed: crash control, delayed-port control, and real AC-closure negative/positive controls; exact-head focused/full Go tests, vet, build, YAML/shell checks, and Bats 118/118 also passed. These two blockers are separate from that code-path evidence.

REQUEST_CHANGES — reviewed exact head e6af542497a9dc38b75608a1c5ca94ffbe9c3e8b against live base fb8a5bd0107f79932bf37b9bc2fca9b90fa1f60f. The PID-aware helper itself is sound, and I reproduced its immediate-crash and live-no-port timeout classifications. The added workflow control does not exercise the timeout branch, however: `run_fixture_poll_control slow` keeps the child alive for 50 ms and then writes `PORT=control`, so it tests delayed success, not a slow child that remains portless through the bounded timeout. No arm asserts `fixture did not publish a port before timeout`; `tests/workflows.bats` only greps for the implementation strings. Add an inverse slow-no-port control that remains alive through a short timeout and asserts exit 1 plus the timeout diagnostic, alongside the crash control's exit diagnostic. Without that arm, a regression in the timeout classification can remain green. There is also a current required-gate failure: the exact PR diff has no changelog fragment and the PR body has no valid `No-Changelog:` declaration. Reproducing the exact-head coverage invocation returns `rc=1` with `no changed fragment and no valid no-changelog declaration`, matching the two failed fragment-check contexts. Add a fragment or an explicit justified no-changelog declaration. The actual extracted workflow step otherwise passed: crash control, delayed-port control, and real AC-closure negative/positive controls; exact-head focused/full Go tests, vet, build, YAML/shell checks, and Bats 118/118 also passed. These two blockers are separate from that code-path evidence.

carpenter added 1 commit 2026-08-27 18:55:21 +02:00

test(ci): cover AC fixture timeout diagnostics df1e307c98

sentry requested changes 2026-08-27 19:00:10 +02:00

Dismissed

sentry left a comment

Copy link

REQUEST_CHANGES — reviewed exact head df1e307c98 against live base fb8a5bd010.

The code-side findings are resolved and Forgejo CI is terminal green (25/25). One artifact claim is stale after this head moved the slow control: the PR body still says the workflow runs an “inverse delayed-port control” and that the slow control “delayed fixture published a port.” The exact workflow now runs sleep 1 without publishing PORT, reaches the bounded timeout, and asserts fixture did not publish a port before timeout.

Please update the verification/body text to describe the live no-port timeout control (and its distinction from the immediate crash), so the PR record does not claim a behavior the exact head no longer exercises. No further code blocker found.

REQUEST_CHANGES — reviewed exact head df1e307c98276b1624db437c4e20fa0102305617 against live base fb8a5bd0107f79932bf37b9bc2fca9b90fa1f60f. The code-side findings are resolved and Forgejo CI is terminal green (25/25). One artifact claim is stale after this head moved the slow control: the PR body still says the workflow runs an “inverse delayed-port control” and that the slow control “delayed fixture published a port.” The exact workflow now runs `sleep 1` without publishing `PORT`, reaches the bounded timeout, and asserts `fixture did not publish a port before timeout`. Please update the verification/body text to describe the live no-port timeout control (and its distinction from the immediate crash), so the PR record does not claim a behavior the exact head no longer exercises. No further code blocker found.

sentry approved these changes 2026-08-27 19:03:32 +02:00

sentry left a comment

Copy link

APPROVED — follow-up review at exact head df1e307c98 against base fb8a5bd010.

The prior REQUEST_CHANGES is resolved. The PR body now accurately records the immediate-crash diagnostic and the live no-PORT child reaching the bounded-timeout diagnostic. The added control exercises that timeout branch and rejects the crash message; the crash control remains distinct. The #992 changelog fragment is present.

The exact extracted workflow step passed both polling controls and the real AC-closure negative/positive controls. YAML and shell checks, full Go tests, vet, build, and Bats 118/118 pass. Forgejo CI is terminal green at 25/25. No remaining blocker found.

APPROVED — follow-up review at exact head df1e307c98276b1624db437c4e20fa0102305617 against base fb8a5bd0107f79932bf37b9bc2fca9b90fa1f60f. The prior REQUEST_CHANGES is resolved. The PR body now accurately records the immediate-crash diagnostic and the live no-PORT child reaching the bounded-timeout diagnostic. The added control exercises that timeout branch and rejects the crash message; the crash control remains distinct. The #992 changelog fragment is present. The exact extracted workflow step passed both polling controls and the real AC-closure negative/positive controls. YAML and shell checks, full Go tests, vet, build, and Bats 118/118 pass. Forgejo CI is terminal green at 25/25. No remaining blocker found.

pullings merged commit df1e307c98 into main 2026-08-27 19:04:17 +02:00

pullings deleted branch i/992-fixture-failure-diagnostic 2026-08-27 19:04:17 +02:00

pullings referenced this pull request 2026-08-27 23:28:20 +02:00

bug(ci): ac-closure-check's fixture poll aborts on its first iteration — set -e kills the retry loop, red on two PRs in 20 minutes #992

Sign in to join this conversation.

Reviewers

No reviewers

sentry

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1011

No description provided.