Skip to content

test: add tests to ensure that node.1 is kept in sync with cli.md - #58878

Merged
nodejs-github-bot merged 6 commits into
nodejs:mainfrom
dario-piotrowicz:dario/node.1-cli-sync-tests
Jun 30, 2025
Merged

test: add tests to ensure that node.1 is kept in sync with cli.md#58878
nodejs-github-bot merged 6 commits into
nodejs:mainfrom
dario-piotrowicz:dario/node.1-cli-sync-tests

Conversation

@dario-piotrowicz

Copy link
Copy Markdown
Member

add tests to make sure that the content of the doc/node.1 file is kept in snyc with the content of the doc/api/cli.md file (to make sure that when a flag or environment variable is added or removed to one, the same change is also applied to the other)

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 28, 2025
@codecov

codecovBot commented Jun 28, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.11%. Comparing base (4d5ee24) to head (c3be698).
Report is 13 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #58878 +/- ##
=======================================
Coverage 90.10% 90.11% =======================================
Files 640 640 Lines 188493 188426 -67 Branches 36971 36960 -11 =======================================
- Hits 169843 169793 -50 + Misses 11358 11332 -26 - Partials 7292 7301 +9 

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

@dario-piotrowicz
dario-piotrowiczforce-pushed the dario/node.1-cli-sync-tests branch 4 times, most recently from 436ee86 to f9bf6f8CompareJune 28, 2025 23:05
add tests to make sure that the content of the doc/node.1 file
is kept in snyc with the content of the doc/api/cli.md file
(to make sure that when a flag or environment variable is added
or removed to one, the same change is also applied to the other)
@dario-piotrowicz
dario-piotrowiczforce-pushed the dario/node.1-cli-sync-tests branch from f9bf6f8 to 31315b2CompareJune 28, 2025 23:27
@nodejs-github-bot

This comment was marked as outdated.

Comment threadtest/parallel/test-cli-node-cli-manpage-env-vars.mjs Outdated
Comment threadtest/parallel/test-cli-node-cli-manpage-options.mjs Outdated
Co-authored-by: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
@dario-piotrowiczdario-piotrowicz added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 29, 2025
@dario-piotrowicz

Copy link
Copy Markdown
MemberAuthor

@VoltrexKeyva, suggestions accepted, thanks for having a look at the PR 🫶

Regarding non-capturing groups, I did accept the suggestions anyways I wanted to mention that I generally purposely avoid adding the ?:s unless actually necessary since they, in my opinion, add extra noise to regexes, which are already generally hard to read on their own (I appreciate that with ?: you're clearly indicating that you don't care about capturing the content inside that group, however I don't know if that is worth the extra (although small) complexity/noise 🤔 )

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Comment threadtest/parallel/test-cli-node-cli-manpage-env-vars.mjs
'experimental-require-module',
'experimental-sea-config',
'experimental-worker-inspection',
'expose-gc',

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.

This is the only one I'm not sure about adding to node.1. The rest should be fine tho.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

what's the rationale about wanting the flag in the CLI docs but not in the manpage? 🤔

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.

expose-gc is actually a v8 provided flag and we typically do not document them. I can't remember the rationale for why we added it to the CLI docs.

@dario-piotrowiczdario-piotrowiczJun 29, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The documentation was added in #53078

It seems like you and Joyee requested the doc's removal, but only part of it was actually removed? 🤔

(although Joyee's next suggestion makes it sound like they were ok with the docs after all?... 🤔)

Anyways, it looks likely that the docs for the flag were just added by mistake, what do you think?
I can open a PR removing the flag from the docs and see what people say?

Comment threadtest/parallel/test-cli-node-cli-manpage-options.mjs
@dario-piotrowiczdario-piotrowicz added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 29, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@dario-piotrowicz

Copy link
Copy Markdown
MemberAuthor

@jasnell Issues added for the missing env variables and options 🙂

I've also linked those in the code comments 👍

By the way, I put my name in the TODO comments as I am happy to look into adding those, however the issues also seems like nice potential good first issues (and we only have a few of those), what do you think? maybe it'd be better to remove my name from the TODO comments, add the good first issue label to the issues? 🤔

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

nodejs-github-bot commented Jun 30, 2025

Copy link
Copy Markdown
Collaborator

@jasnell

Copy link
Copy Markdown
Member

Add your name to the todo comments isn't really claiming them but providing a pointer for someone who may want to tackle those todos later... it basically identifies who to go ask about it :-)

@jasnelljasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 30, 2025
@atlowChemiatlowChemi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 30, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 30, 2025
@nodejs-github-bot
nodejs-github-bot merged commit 4b4aaf9 into nodejs:mainJun 30, 2025
60 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 4b4aaf9

@dario-piotrowicz
dario-piotrowicz deleted the dario/node.1-cli-sync-tests branch June 30, 2025 21:24
targos pushed a commit that referenced this pull request Jul 3, 2025
add tests to make sure that the content of the doc/node.1 file
is kept in snyc with the content of the doc/api/cli.md file
(to make sure that when a flag or environment variable is added
or removed to one, the same change is also applied to the other)
PR-URL: #58878
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 21, 2025
add tests to make sure that the content of the doc/node.1 file
is kept in snyc with the content of the doc/api/cli.md file
(to make sure that when a flag or environment variable is added
or removed to one, the same change is also applied to the other)
PR-URL: #58878
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 24, 2025
add tests to make sure that the content of the doc/node.1 file
is kept in snyc with the content of the doc/api/cli.md file
(to make sure that when a flag or environment variable is added
or removed to one, the same change is also applied to the other)
PR-URL: #58878
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 4, 2025
jkleinsc pushed a commit to electron/electron that referenced this pull request Aug 4, 2025
* chore: bump node in DEPS to v22.18.0
* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
nodejs/node#58845
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58960
* permission: propagate permission model flags on spawn
nodejs/node#58853
* esm: syncify default path of ModuleLoader\.load
nodejs/node#57419
* src: remove fast API for InternalModuleStat
nodejs/node#58489
* src: simplify adding fast APIs to ExternalReferenceRegistry
nodejs/node#58896
* chore: fixup patch indices
* src: fix internalModuleStat v8 fast path
nodejs/node#58054
* test: add tests to ensure that node.1 is kept in sync with cli.md
nodejs/node#58878
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58942
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
* chore: bump node in DEPS to v22.18.0
* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
nodejs/node#58845
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58960
* permission: propagate permission model flags on spawn
nodejs/node#58853
* esm: syncify default path of ModuleLoader\.load
nodejs/node#57419
* src: remove fast API for InternalModuleStat
nodejs/node#58489
* src: simplify adding fast APIs to ExternalReferenceRegistry
nodejs/node#58896
* chore: fixup patch indices
* src: fix internalModuleStat v8 fast path
nodejs/node#58054
* test: add tests to ensure that node.1 is kept in sync with cli.md
nodejs/node#58878
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58942
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Aug 5, 2025
chore: bump node to v22.18.0 (main) (#47937)
* chore: bump node in DEPS to v22.18.0
* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
nodejs/node#58845
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58960
* permission: propagate permission model flags on spawn
nodejs/node#58853
* esm: syncify default path of ModuleLoader\.load
nodejs/node#57419
* src: remove fast API for InternalModuleStat
nodejs/node#58489
* src: simplify adding fast APIs to ExternalReferenceRegistry
nodejs/node#58896
* chore: fixup patch indices
* src: fix internalModuleStat v8 fast path
nodejs/node#58054
* test: add tests to ensure that node.1 is kept in sync with cli.md
nodejs/node#58878
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58942
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
kigh-ota pushed a commit to kigh-ota/electron that referenced this pull request Sep 30, 2025
* chore: bump node in DEPS to v22.18.0
* crypto: fix inclusion of OPENSSL_IS_BORINGSSL define
nodejs/node#58845
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58960
* permission: propagate permission model flags on spawn
nodejs/node#58853
* esm: syncify default path of ModuleLoader\.load
nodejs/node#57419
* src: remove fast API for InternalModuleStat
nodejs/node#58489
* src: simplify adding fast APIs to ExternalReferenceRegistry
nodejs/node#58896
* chore: fixup patch indices
* src: fix internalModuleStat v8 fast path
nodejs/node#58054
* test: add tests to ensure that node.1 is kept in sync with cli.md
nodejs/node#58878
* crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
nodejs/node#58942
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@dario-piotrowicz@nodejs-github-bot@jasnell@VoltrexKeyva@atlowChemi