Skip to content

test: skip C++ symbols in tick-processor-arguments - #65906

Open
pipobscure wants to merge 1 commit into
nodejs:mainfrom
pipobscure:tick-processor-arguments-test
Open

test: skip C++ symbols in tick-processor-arguments#65906
pipobscure wants to merge 1 commit into
nodejs:mainfrom
pipobscure:tick-processor-arguments-test

Conversation

@pipobscure

@pipobscure pipobscure commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

test/parallel/test-tick-processor-arguments only checks that a CLI flag (--preprocess) is passed through to the V8 tick processor. Processing a --prof log, however, makes the tick processor resolve the C++ symbols of every shared library listed in the log by shelling out to nm once per library (and, in Node's polyfill, piping the result through c++filt again on macOS).

On the x86_64-darwin: with shared libraries GitHub Actions job, node links roughly a hundred dylibs (V8 split into eight libraries, ~85 abseil libraries, ICU, libuv, ...), so a single --prof-process run 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 (nm availability, 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 in test/tick-processor.

Measured locally on Linux (static build):

variant wall time processes spawned by --prof-process
before ~630 ms 649 (408 of them nm)
after ~170 ms 1

The SmartOS SKIP entry in parallel.status is left as is; I cannot verify whether the address-range problem from #50050 is confined to the shared-library lines.

Refs: #50050

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Sep 8, 2026
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>
@pipobscure
pipobscure force-pushed the tick-processor-arguments-test branch from ccb5bc7 to bdec92d Compare September 8, 2026 07:15
@pipobscure pipobscure changed the title test: stop resolving C++ symbols in test-tick-processor-arguments test: skip C++ symbols in tick-processor-arguments Sep 8, 2026
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.16%. Comparing base (2987a59) to head (bdec92d).
⚠️ Report is 22 commits behind head on main.

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     

see 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399 MikeMcC399 added the flaky-test Issues and PRs involving tests that fail intermittently in CI. label Sep 8, 2026
@MikeMcC399

Copy link
Copy Markdown
Contributor

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!

@pipobscure

Copy link
Copy Markdown
Contributor Author

Can someone please request-ci for me please!

@MikeMcC399

Copy link
Copy Markdown
Contributor

Can someone please request-ci for me please!

It needs a technical review of your PR and at least one approval before request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. will be accepted.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 8, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flaky-test Issues and PRs involving tests that fail intermittently in CI. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants