test(child_process): load the mock agent once per suite#9113
Conversation
Overall package sizeSelf size: 6.39 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
The Integration and Bluebird suites called agent.load / agent.close in beforeEach / afterEach, so every test tore down the mock agent and the next brought a new one up on a fresh port. Async child processes spawned in a test finish on their own 'close' event, which can fire after the test resolves — during the teardown gap. The leaked command_execution span then flushed to the previous test's now-closed port (ECONNREFUSED), and a stale assertion handler matched the leftover manual 'parent' span instead, timing the next test out. Under CI load the first miss cascaded across the whole suite. Loading the agent once per describe block in before / after keeps a single port alive for every test in the block, so a late span lands on the still-open agent. The Bluebird global.Promise swap stays per-test in beforeEach / afterEach. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28281208791
1c92aa8 to
78c984b
Compare
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 7a066a5 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9113 +/- ##
=======================================
Coverage 93.69% 93.69%
=======================================
Files 889 889
Lines 50856 50856
Branches 11830 11830
=======================================
+ Hits 47647 47648 +1
+ Misses 3209 3208 -1 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-06-27 23:28:44 Comparing candidate commit 7a066a5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2254 metrics, 32 unstable metrics.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78c984b1a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The shared mock agent now stays alive across every test in the Integration suite. The "should maintain previous span" cases spawn an async ls child process but never await its command_execution span, so a late flush can land during a later test whose expectSomeSpan carries the same ls expectation and be consumed there, masking a regression in that later command instead of failing. Awaiting the span in the originating test consumes it before the next test runs. Refs: #9113 (comment)
* test(child_process): load the mock agent once per suite The Integration and Bluebird suites called agent.load / agent.close in beforeEach / afterEach, so every test tore down the mock agent and the next brought a new one up on a fresh port. Async child processes spawned in a test finish on their own 'close' event, which can fire after the test resolves — during the teardown gap. The leaked command_execution span then flushed to the previous test's now-closed port (ECONNREFUSED), and a stale assertion handler matched the leftover manual 'parent' span instead, timing the next test out. Under CI load the first miss cascaded across the whole suite. Loading the agent once per describe block in before / after keeps a single port alive for every test in the block, so a late span lands on the still-open agent. The Bluebird global.Promise swap stays per-test in beforeEach / afterEach. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28281208791 * test(child_process): drain ls spans in span-maintenance tests The shared mock agent now stays alive across every test in the Integration suite. The "should maintain previous span" cases spawn an async ls child process but never await its command_execution span, so a late flush can land during a later test whose expectSomeSpan carries the same ls expectation and be consumed there, masking a regression in that later command instead of failing. Awaiting the span in the originating test consumes it before the next test runs. Refs: #9113 (comment)
* test(child_process): load the mock agent once per suite The Integration and Bluebird suites called agent.load / agent.close in beforeEach / afterEach, so every test tore down the mock agent and the next brought a new one up on a fresh port. Async child processes spawned in a test finish on their own 'close' event, which can fire after the test resolves — during the teardown gap. The leaked command_execution span then flushed to the previous test's now-closed port (ECONNREFUSED), and a stale assertion handler matched the leftover manual 'parent' span instead, timing the next test out. Under CI load the first miss cascaded across the whole suite. Loading the agent once per describe block in before / after keeps a single port alive for every test in the block, so a late span lands on the still-open agent. The Bluebird global.Promise swap stays per-test in beforeEach / afterEach. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28281208791 * test(child_process): drain ls spans in span-maintenance tests The shared mock agent now stays alive across every test in the Integration suite. The "should maintain previous span" cases spawn an async ls child process but never await its command_execution span, so a late flush can land during a later test whose expectSomeSpan carries the same ls expectation and be consumed there, masking a regression in that later command instead of failing. Awaiting the span in the originating test consumes it before the next test runs. Refs: #9113 (comment)
* test(child_process): load the mock agent once per suite The Integration and Bluebird suites called agent.load / agent.close in beforeEach / afterEach, so every test tore down the mock agent and the next brought a new one up on a fresh port. Async child processes spawned in a test finish on their own 'close' event, which can fire after the test resolves — during the teardown gap. The leaked command_execution span then flushed to the previous test's now-closed port (ECONNREFUSED), and a stale assertion handler matched the leftover manual 'parent' span instead, timing the next test out. Under CI load the first miss cascaded across the whole suite. Loading the agent once per describe block in before / after keeps a single port alive for every test in the block, so a late span lands on the still-open agent. The Bluebird global.Promise swap stays per-test in beforeEach / afterEach. Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28281208791 * test(child_process): drain ls spans in span-maintenance tests The shared mock agent now stays alive across every test in the Integration suite. The "should maintain previous span" cases spawn an async ls child process but never await its command_execution span, so a late flush can land during a later test whose expectSomeSpan carries the same ls expectation and be consumed there, masking a regression in that later command instead of failing. Awaiting the span in the originating test consumes it before the next test runs. Refs: #9113 (comment)
Summary
The Integration and Bluebird suites in
packages/datadog-plugin-child_process/test/index.spec.jscalledagent.load/agent.closeinbeforeEach/afterEach, tearing down the mock agent and standing a new one up on a fresh port between every test. Async child processes spawned in a test finish on their owncloseevent, which can fire after the test resolves — during the teardown gap. The leakedcommand_executionspan then flushed to the previous test's now-closed port (ECONNREFUSED), and a stale assertion handler matched the leftover manualparentspan (_dd.integration: opentracing) instead, so the next test timed out. Under CI load the first miss cascaded across the whole suite (the run linked below failed 47 tests on newest-maintenance-lts).Loading the agent once per describe block in
before/afterkeeps a single port alive for every test in the block, so a span flushed slightly late lands on the still-open agent. The Bluebirdglobal.Promiseswap stays per-test inbeforeEach/afterEach.Why
flushInterval: 1on its own isn't the cause — the span can finish (and flush synchronously even atflushInterval: 0) inside the window between one test'sagent.close()and the next test'sagent.load(). Removing the per-test agent churn is what closes that window.Test plan
Reproduced by running the suite under parallel load (the contention CI's
nyc+ busy runner provides):ECONNREFUSED+ wrong-parent-span signature (10/160 at 4 concurrent, 15/160 at 8).Fixes: https://github.com/DataDog/dd-trace-js/actions/runs/28281208791