test(npm): streampub coherence-sink lifecycle tests (part 7/8)
📦 What this MR does
Part 7/8 of the stack splitting the streampub streaming publish-envelope library (S11 Step 13). See Part 1/8 (!432) for the stack overview and the byte-identity contract against the reference, !420 (closed).
- Targets
10io/npm-local-step-13-part6of8(Part 6/8). GitLab retargets tomainas the parent merges. - Depends on Part 6/8 (set as a merge-request dependency).
Tests only — the drain-on-error / CloseWithError lifecycle contract (made panic-safe in Part 6/8) already shipped with the pipeline; this part adds its goroutine-lifecycle coverage.
lifecycle_test.go(new):TestProcess_ContextCancellation— a canceled context surfaces as an error and leaves no resident goroutine.TestProcess_NoGoroutineLeakOnSuccess— the inspector goroutine has exited onceProcessreturns on the success path.- Both are serial (
//nolint:paralleltest): they sample the process-globalruntime.NumGoroutine()baseline, which is unstable under concurrent execution (S11 AC 49).
streampub_test.go— theassertNoGoroutineLeakhelper. It samplesruntime.NumGoroutine()synchronously in the calling goroutine rather than viarequire.Eventually(whose checker goroutine would itself bump the count and make<= baselinestructurally unsatisfiable), polling with a short settle so a just-signalled goroutine is off the count before the comparison.
The two fixture-driven lifecycle tests — DrainOnError_BoundedTime and BackpressureBufferWired — depend on the checked-in tarball corpus and land with it in Part 8/8.
🧪 Tests
golangci-lint(repo-pinned v2.12) → 0 issuesGOEXPERIMENT=jsonv2 go test -race -count=1 ./internal/format/npm/streampub/...→ pass
🚩 Deviations from the reference (!420 (closed))
No production changes in this part, and no new deviations: lifecycle_test.go and assertNoGoroutineLeak are byte-identical to the reference. The previously-documented reviewed deltas (Parts 1–6) stand: error.go/streampub.go (ProcessError + six-cap validate), sink.go (sizeLimitWriter byte count), pipeline.go (deferred pipe-close panic backstop), inspector.go (non-regular tar entry skip), and the test-comment corrections.
Review feedback (Duo + AppSec) was assessed and declined with reasoning in-thread, keeping the test code as the reference wrote it: the unreachable-return nit is harmless dead code; the WaitGroup-hook suggestion would add a process-global test seam to production code (a design change the reference deliberately avoided, and its stated polling failure-modes don't hold); and the ContextCancellation "vacuous" observation is covered by the suite as a whole (NoGoroutineLeakOnSuccess and the Part 8 drain-on-error test exercise the spawn-then-exit lifecycle).
🔗 References
- Reference MR (read-only): !420 (closed)
- Related work item: #131 (closed) (not closed by this MR)