Fix #178: flaky fan-out finished-state test #179

Merged

crueber merged 1 commit from fix/issue-178 into main

2026-09-06 17:54:59 +00:00

crueber commented

2026-09-06 17:52:11 +00:00

Owner

ROOT CAUSE: test bug (flaky polling contract), NOT an implementation bug.

Evidence:

Implementation lifecycle is sound: taskTable.recent (internal/notify/tasks.go finishLocked) has no TTL/janitor; eviction only beyond 128 keys while this test uses 1. A Finished record is therefore always observable via TaskStatus — 'legitimately reaped' is impossible, so no implementation change.
Termination is prompt given CPU: lock order table.mu->entry.mu, no lock across store calls, non-blocking ubus.publish, bounded sem/wg. Measured 30/30 -race samples under 4x-oversubscribed 2-CPU load finish in <=84ms (solo ~8ms), always a single leader task ID.
The failure signature (all 60 delivered, TaskFinished unseen at 15.01s) is exactly what a single shared deadline produces on a starved runner: delivery at ~14.9s leaves no margin for the finish landing ms later, misreported as 'lost 0/60'.
TLS handshake line: expected test noise — TestWebhookInsecureTLS (only NewTLSServer in tests) asserts fail-closed rejection of self-signed certs; the server-side log is the asserted rejection.

Fix (test-only, guarantees preserved — law 11):

TestFanoutConcurrentEnqueueLosesNothing now polls in two phases: phase 1 keeps the UNCHANGED 15s no-loss pin (all 60 exactly-once); phase 2 gets a fresh 15s termination window asserting TaskFinished with a distinct diagnostic ('delivered X/60 but task never finished'). No budget weakened (15s was never a documented product budget — docs pin store round-trips, not fan-out wall time); no doc change (no semantics change, law 12).
TestWebhookInsecureTLS discards the expected handshake-error server log (assertions untouched).

Verification:

-race -count=20 isolation: 20/20 pass.
taskset 2-CPU go test -short ./internal/... (incl. e2e): green, notify ok.
Coverage internal/notify 96.1% (>=95%); gofmt/vet clean; store contract green.

ROOT CAUSE: test bug (flaky polling contract), NOT an implementation bug. Evidence: - Implementation lifecycle is sound: taskTable.recent (internal/notify/tasks.go finishLocked) has no TTL/janitor; eviction only beyond 128 keys while this test uses 1. A Finished record is therefore always observable via TaskStatus — 'legitimately reaped' is impossible, so no implementation change. - Termination is prompt given CPU: lock order table.mu->entry.mu, no lock across store calls, non-blocking ubus.publish, bounded sem/wg. Measured 30/30 -race samples under 4x-oversubscribed 2-CPU load finish in <=84ms (solo ~8ms), always a single leader task ID. - The failure signature (all 60 delivered, TaskFinished unseen at 15.01s) is exactly what a single shared deadline produces on a starved runner: delivery at ~14.9s leaves no margin for the finish landing ms later, misreported as 'lost 0/60'. - TLS handshake line: expected test noise — TestWebhookInsecureTLS (only NewTLSServer in tests) asserts fail-closed rejection of self-signed certs; the server-side log is the asserted rejection. Fix (test-only, guarantees preserved — law 11): - TestFanoutConcurrentEnqueueLosesNothing now polls in two phases: phase 1 keeps the UNCHANGED 15s no-loss pin (all 60 exactly-once); phase 2 gets a fresh 15s termination window asserting TaskFinished with a distinct diagnostic ('delivered X/60 but task never finished'). No budget weakened (15s was never a documented product budget — docs pin store round-trips, not fan-out wall time); no doc change (no semantics change, law 12). - TestWebhookInsecureTLS discards the expected handshake-error server log (assertions untouched). Verification: - -race -count=20 isolation: 20/20 pass. - taskset 2-CPU go test -short ./internal/... (incl. e2e): green, notify ok. - Coverage internal/notify 96.1% (>=95%); gofmt/vet clean; store contract green.

crueber added 1 commit

2026-09-06 17:52:11 +00:00

Fix #178 : split fan-out test into delivery + termination windows a364a8aefe

TestFanoutConcurrentEnqueueLosesNothing polled delivery and
TaskFinished under one shared 15s deadline, so a starved runner that
delivers at 14.9s reports the finish (ms later) as 'lost 0/60'.
Phase 1 keeps the 15s no-loss pin; phase 2 gets its own 15s
termination window with a distinct diagnostic. No product change:
the finished record is never reaped (bounded recent cache, no TTL),
so this was a test-contract flake, not an implementation bug.
Also silences the expected self-signed handshake-error line in
TestWebhookInsecureTLS (asserted rejection, not a symptom).

crueber referenced this pull request

2026-09-06 17:52:14 +00:00

Flaky fan-out test: TaskFinished never observed under load #178

crueber referenced this pull request

2026-09-06 17:54:41 +00:00

Flaky fan-out test: TaskFinished never observed under load #178

crueber merged commit 7855854b2f into main

2026-09-06 17:54:59 +00:00

crueber referenced this pull request from a commit

2026-09-06 17:54:59 +00:00

Merge pull request 'Fix #178: flaky fan-out finished-state test' (#179) from fix/issue-178 into main

crueber referenced this pull request