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 to main as 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 once Process returns on the success path.
    • Both are serial (//nolint:paralleltest): they sample the process-global runtime.NumGoroutine() baseline, which is unstable under concurrent execution (S11 AC 49).
  • streampub_test.go — the assertNoGoroutineLeak helper. It samples runtime.NumGoroutine() synchronously in the calling goroutine rather than via require.Eventually (whose checker goroutine would itself bump the count and make <= baseline structurally 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 issues
  • GOEXPERIMENT=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).

Edited by David Fernandez

Merge request reports

Loading
Loading