test: skip C++ symbols in tick-processor-arguments - #65906
Conversation
The test only checks that a CLI flag is passed through to the V8 tick processor, but processing a --prof log makes the tick processor resolve the C++ symbols of every shared library listed in it by shelling out to nm (plus c++filt on macOS) once per library. On a --shared build that links around a hundred dylibs this takes longer than the test timeout on the macOS x86_64 GitHub Actions runner, and the outcome depends on the host toolchain rather than on node. Drop the shared-library entries from the log before processing it so the test exercises argument handling only. C++ symbol resolution is covered by test/tick-processor. Signed-off-by: Philipp Dunkel <pip@pipobscure.com>
ccb5bc7 to
bdec92d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65906 +/- ##
=======================================
Coverage 90.16% 90.16%
=======================================
Files 771 771
Lines 265097 265097
Branches 50358 50355 -3
=======================================
+ Hits 239026 239031 +5
- Misses 17011 17022 +11
+ Partials 9060 9044 -16 🚀 New features to boost your workflow:
|
|
https://github.com/nodejs/node/actions/workflows/test-shared.yml?query=is%3Afailure+is%3Atimed_out shows multiple workflows failing with timeout, for instance https://github.com/nodejs/node/actions/runs/34212576163 from @panva x86_64-darwin: with shared libraries / build: test/parallel/test-tick-processor-arguments.js#L0
Command: out/Release/node /Users/runner/work/_temp/node-v27.0.0-nightly2026-09-081b200078a0-slim/test/parallel/test-tick-processor-arguments.js
--- TIMEOUT ---so this PR does deserve attention! |
|
Can someone please request-ci for me please! |
It needs a technical review of your PR and at least one approval before
request-ci
|
This comment was marked as outdated.
This comment was marked as outdated.
|
Yikes! What's happening here? |
|
CI: https://ci.nodejs.org/job/node-test-pull-request/77236/ Edit: this was aborted by the Releasers Team due to release priorities. I submitted a new
request-ci
I would really like to see this PR succeed, as I'm seeing CI fail in multiple other PRs and re-running the CI often just repeats the issue. |
|
Yikes! What happened there? |
|
I didn't realise that the commit queue is still disabled, so to avoid confusion, I've removed the
request-ci
|
test/parallel/test-tick-processor-argumentsonly checks that a CLI flag (--preprocess) is passed through to the V8 tick processor. Processing a--proflog, however, makes the tick processor resolve the C++ symbols of every shared library listed in the log by shelling out tonmonce per library (and, in Node's polyfill, piping the result throughc++filtagain on macOS).On the
x86_64-darwin: with shared librariesGitHub Actions job, node links roughly a hundred dylibs (V8 split into eight libraries, ~85 abseil libraries, ICU, libuv, ...), so a single--prof-processrun spawns a couple of hundred external processes on a slow Intel runner and exceeds the 120s test timeout. It has done so on unrelated PRs, e.g. https://github.com/nodejs/node/actions/runs/34138185061/job/101794526205 and https://github.com/nodejs/node/actions/runs/34169515699. The outcome of the test also depends on the host toolchain (nmavailability, Xcode license state) rather than on node itself; #50050 was a different symptom of the same dependency.This drops the
shared-library,entries from the log before handing it to--prof-process, so the test exercises argument handling only. The log remains a valid V8 log and the output is still checked to be valid JSON. C++ symbol resolution is covered by the tests intest/tick-processor.Measured locally on Linux (static build):
--prof-processnm)The SmartOS
SKIPentry inparallel.statusis left as is; I cannot verify whether the address-range problem from #50050 is confined to the shared-library lines.Refs: #50050