Skip to content

test: use spawnSyncAndAssert in windowsHide test - #65351

Open
ganjanggejang wants to merge 1 commit into
nodejs:mainfrom
ganjanggejang:replace-spawnsync
Open

test: use spawnSyncAndAssert in windowsHide test#65351
ganjanggejang wants to merge 1 commit into
nodejs:mainfrom
ganjanggejang:replace-spawnsync

Conversation

@ganjanggejang

Copy link
Copy Markdown
Contributor

Replace the manual status, signal, stdout and stderr assertions with
spawnSyncAndAssert() from test/common/child_process.

The previous code asserted the status first, so a CI failure only
reported the exit status and the child's output was never shown. On
failure the helper prints stdout, stderr, status, signal and the
command that was run.

Only changed sync spawn(), because there is no async helper yet.

Signed-off-by: Junsoo Ha <junsoo2018@naver.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 17, 2026
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 17, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2026
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.11%. Comparing base (cc60845) to head (ab91b39).
⚠️ Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65351      +/-   ##
==========================================
- Coverage   90.13%   90.11%   -0.02%     
==========================================
  Files         752      752              
  Lines      251820   251820              
  Branches    47352    47350       -2     
==========================================
- Hits       226974   226934      -40     
- Misses      16158    16229      +71     
+ Partials     8688     8657      -31     

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

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 18, 2026
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 19, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/65351
βœ”  Done loading data for nodejs/node/pull/65351
----------------------------------- PR info ------------------------------------
Title      test: use spawnSyncAndAssert in windowsHide test (#65351)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     ganjanggejang:replace-spawnsync -> nodejs:main
Labels     test, author ready, needs-ci, commit-queue
Commits    1
 - test: use spawnSyncAndAssert in windowsHide test
Committers 1
 - Junsoo Ha <junsoo2018@naver.com>
PR-URL: https://github.com/nodejs/node/pull/65351
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65351
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
--------------------------------------------------------------------------------
   β„Ή  This PR was created on Mon, 17 Aug 2026 11:58:51 GMT
   βœ”  Approvals: 4
   βœ”  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/65351#pullrequestreview-4951952749
   βœ”  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/65351#pullrequestreview-4953484636
   βœ”  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/65351#pullrequestreview-4955316846
   βœ”  - Stefan Stojanovic (@StefanStojanovic): https://github.com/nodejs/node/pull/65351#pullrequestreview-4959345464
   ✘  GitHub CI is still running
   β„Ή  Last Full PR CI on 2026-08-18T07:17:53Z: https://ci.nodejs.org/job/node-test-pull-request/75962/
- Querying data for job/node-test-pull-request/75962/
βœ”  Build data downloaded
   βœ”  Last Jenkins CI successful
--------------------------------------------------------------------------------
   βœ”  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/32251410497

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants