Repository navigation
ci: run both leak harnesses nightly and gate on injected leaks - #254
truthixify merged 3 commits into
Conversation
|
@collinsezedike Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
truthixify
left a comment
There was a problem hiding this comment.
The deliberate leak proof does not trip the streaming gate. Reproduction: SCAN_LEAK_ANNOUNCEMENTS=1 SCAN_LEAK_ITERATIONS=300 SCAN_LEAK_SAMPLE_INTERVAL=25 SCAN_LEAK_INJECT=1 pnpm test:leak exits 0 with a 0.0149 MB-per-step slope. LeakSentinel retains an 8 KB Uint8Array, whose backing allocation is largely external to V8 heapUsed; additionally, 25 injected buffers nominally add about 0.195 MB per sample, below the 0.2 MB threshold even if fully counted. The nightly leak-injection job will therefore flag the detector as ineffective. Please inject heap-accounted retention or measure external and array-buffer memory too, set the injected growth clearly above threshold, and keep a regression proving the injection command exits nonzero.
|
@truthixify thank you for the detailed root cause analysis. You're right. The 8 KB Uint8Array backing is external to V8
Right now, when,
The |
truthixify
left a comment
There was a problem hiding this comment.
The updated memory metric detects the injected retained ArrayBuffer payload while the normal harness remains below threshold. Approved.
Summary
test:leak(the streaming scan slope harness) existed but no workflow ran it, so memory regressions in the streaming path could accumulate unnoticed. The constructor-count heap snapshot test was scheduled, but the streaming path has different allocation behaviour that it does not cover. This schedules both harnesses, uploads diagnostics only on failure, and adds a job that proves a deliberate leak actually trips each gate.Changes
stream-leak-regression: nightly job that runspnpm test:leakon Node 22 and uploadsscan-leak-diagnostics.jsonwhen the gate fails.heap-regression: same harness as before, but diagnostics now upload only on failure and are limited to the compactheap-diff.json.leak-injection: nightly job that runs each harness with a deliberately retained leak (SCAN_LEAK_INJECT=1,HEAP_LEAK_INJECT=1) and fails when either gate reports success, proving the thresholds catch real leaks.scan-leak.test.ts: writes a compactscan-leak-diagnostics.json(samples, fitted slope, threshold) and only writes the largeheap-final.heapsnapshotwhen the gate trips. AddsSCAN_LEAK_ANNOUNCEMENTS,SCAN_LEAK_ITERATIONS,SCAN_LEAK_SAMPLE_INTERVAL,SCAN_LEAK_THRESHOLD_MB, andSCAN_LEAK_INJECTknobs with CI-feasible defaults, and requires--expose-gcfor a stable slope.test:leaknow launches Vitest through Node with--expose-gc --pool=threads, matchingtest:heap-leak, so forced GC keeps the slope threshold stable.Notes
The streaming harness defaults were reduced because the previous 10000 iterations over 10000 announcements never completed within a CI budget. The threshold stays at 0.2 MB per sample step, and the reduced workload keeps enough samples for a stable least-squares fit. The full heap snapshot is written on failure for triage but is not uploaded, keeping the nightly artifacts compact.
Closes #246