Skip to content

test: use spawnSync helpers in cpu-prof tests - #65694

Open
DevJunz wants to merge 1 commit into
nodejs:mainfrom
DevJunz:junhwan/test-cpu-prof-spawn-sync-helpers
Open

test: use spawnSync helpers in cpu-prof tests#65694
DevJunz wants to merge 1 commit into
nodejs:mainfrom
DevJunz:junhwan/test-cpu-prof-spawn-sync-helpers

Conversation

@DevJunz

@DevJunz DevJunz commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Replaces the manual spawnSync() status checks in the eight --cpu-prof tests with spawnSyncAndExitWithoutError() from test/common/child_process, following the same approach as #65552.

These tests logged stderr when the status was non-zero and then asserted the status, so a CI failure never showed stdout, the signal, or the command that ran. The helper reports all of them. Status 0 is still required, and signal null is now checked as well, matching the helper defaults.

Only success-path blocks whose assertions map one-to-one onto the helper defaults are converted.

Refs: #65552

The cpu-prof tests hand-rolled a partial version of what
test/common/child_process already provides: log the child's stderr
when the status is non-zero, then assert the status. That manual
version never shows stdout, the signal, or the command that ran, so
a CI failure gives less information than the helper would.

Replace those blocks with spawnSyncAndExitWithoutError(). Every
existing expectation is preserved -- status 0 is still required, and
signal null is now checked as well, matching the helper defaults.

Only success-path blocks whose assertions map one-to-one onto the
helper defaults are converted.

Refs: nodejs#65351
Refs: nodejs#65552
Assisted-by: claude:opus-5
Signed-off-by: Junhwan Choi <devjunsday@gmail.com>
@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 1, 2026
@DevJunz
DevJunz marked this pull request as draft September 1, 2026 06:49
@DevJunz
DevJunz marked this pull request as ready for review September 1, 2026 07:12
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.07%. Comparing base (242909f) to head (3a58076).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #65694   +/-   ##
=======================================
  Coverage   90.06%   90.07%           
=======================================
  Files         754      754           
  Lines      256395   256395           
  Branches    48500    48499    -1     
=======================================
+ Hits       230922   230941   +19     
- Misses      16574    16580    +6     
+ Partials     8899     8874   -25     

see 38 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.

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 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

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.

3 participants