test(time-profiler): stabilize flaky "should update state"#362
Open
szegedi wants to merge 1 commit into
Open
Conversation
The test was intermittently failing across platforms (e.g. centos-test-26, darwin-arm builds) with either "No context found" or "Unexpected context". Two compounding causes: 1. It established the sampling context with a bare setContext(). Under useCPED that only takes effect when the current continuation already has an AsyncContextFrame; otherwise it silently no-ops. Whether one existed depended on what ran before, so the context was often never stored and samples came back with an undefined context. 2. It stopped after a single sample. The first sample is taken at profiler start (before any context is set) and is skipped during context association, so one sample left nothing reliable to attach the context to. Establish the context the way production code does — runWithContext when useCPED is enabled, setContext otherwise — and wait for several samples before stopping. Reproduced on Node 26: the test failed 30/30 runs in isolation before, and passes 30/30 in isolation and 25/25 in the full file after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Overall package sizeSelf size: 2.19 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | pprof-format | 2.2.2 | 500.53 kB | 500.53 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | node-gyp-build | 4.8.4 | 13.86 kB | 13.86 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stabilizes the intermittently-failing
Time Profiler › profile › should update statetest, which flakes across platforms (e.g.centos-test-26, darwin-arm) with eitherNo context foundorUnexpected context.Root cause
Two compounding issues:
Context was often never stored. The test established the sampling context with a bare
time.setContext(initialContext). UnderuseCPED,setContextonly takes effect when the current continuation already has anAsyncContextFrame(the native side bails withif (!cped->IsMap()) return;); otherwise it silently no-ops. Whether such a frame existed depended on what ran before the test, so the context was frequently never stored and samples came back with anundefinedcontext →Unexpected context. (Run in isolation on Node 26 it failed 100% for this reason.)It stopped after a single sample. The wait loop exited as soon as
kSampleCountreached 1. The first sample is taken at profiler start — before any context is set — and is explicitly skipped during context association (GetContextsByNode), so a single sample left nothing reliable to attach the context to →No context found.Fix
time.runWithContext(...)whenuseCPEDis enabled (which reliably installs theAsyncContextFrame), andsetContext(...)otherwise.The test still verifies the same things (context associated with samples, the stored context reflects a live mutation of the object, and timestamps fall within range).
Verification
Reproduced and verified on Node 26 (in a container):
--grep), 30 runs🤖 Generated with Claude Code