Skip to content

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Oct 13, 2025

test: move sea tests into test/sea

This makes skipping/running these tests easier to manage with a
dedicated test runner that can be tweaked for SEA.

test: skip sea tests on x64 macOS

It's unlikely that anyone would invest in fixing them on x64 macOS in the
near future, now that x64 macOS is being phased out. Simply skip them
for now.

Refs: #59553

@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 Oct 13, 2025
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Can you also migrate

[$system==linux && $arch==ppc64]
# https://github.com/nodejs/node/issues/59561
test-single-executable-application: SKIP
test-single-executable-application-assets: SKIP
test-single-executable-application-assets-raw: SKIP
test-single-executable-application-asset-keys-empty: SKIP
test-single-executable-application-asset-keys: SKIP
test-single-executable-application-disable-experimental-sea-warning: SKIP
test-single-executable-application-empty: SKIP
test-single-executable-application-exec-argv: SKIP
test-single-executable-application-exec-argv-empty: SKIP
test-single-executable-application-exec-argv-extension-cli: SKIP
test-single-executable-application-exec-argv-extension-env: SKIP
test-single-executable-application-exec-argv-extension-none: SKIP
test-single-executable-application-inspect-in-sea-flags: SKIP
test-single-executable-application-inspect: SKIP
test-single-executable-application-snapshot: SKIP
test-single-executable-application-snapshot-and-code-cache: SKIP
test-single-executable-application-snapshot-worker: SKIP
test-single-executable-application-use-code-cache: SKIP
from sequential.status to the new sea.status?

Copy link

codecov bot commented Oct 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (db0121b) to head (f14e663).
⚠️ Report is 42 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60250      +/-   ##
==========================================
+ Coverage   88.53%   88.56%   +0.02%     
==========================================
  Files         704      704              
  Lines      208087   208322     +235     
  Branches    40010    40037      +27     
==========================================
+ Hits       184239   184500     +261     
+ Misses      15866    15863       -3     
+ Partials     7982     7959      -23     

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

@joyeecheung joyeecheung force-pushed the skip-macos-64 branch 2 times, most recently from 425d75f to 4d84244 Compare October 15, 2025 11:41
@joyeecheung
Copy link
Member Author

Added the missing Linux PPC64 skips and fixed the linter complaints (seems to be only activated on the touched files). Can you take another look? Thanks @richardlau

@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2025
@nodejs-github-bot

This comment was marked as outdated.

@joyeecheung
Copy link
Member Author

Not sure where this linter comes from, but it's not correct:

==> Executing Duplicated Pattern Checker (39.243µs)
    Check OK
==> Executing File Exist Checker (248.206398ms)
    [err] line 161: "/test/parallel/test-single-executable-*" does not match any files in repository
    [err] line 162: "/test/sequential/test-single-executable-*" does not match any files in repository

@joyeecheung
Copy link
Member Author

Oh right, it's the codeowner file that needs to be updated..updating

@richardlau

This comment was marked as outdated.

@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 15, 2025

This comment was marked as outdated.

@richardlau richardlau added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 15, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Oct 17, 2025
@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 Oct 17, 2025
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/60250
✔  Done loading data for nodejs/node/pull/60250
----------------------------------- PR info ------------------------------------
Title      test: skip sea tests on x64 macOS (#60250)
Author     Joyee Cheung <[email protected]> (@joyeecheung)
Branch     joyeecheung:skip-macos-64 -> nodejs:main
Labels     test, needs-ci, commit-queue-rebase
Commits    2
 - test: move sea tests into test/sea
 - test: skip sea tests on x64 macOS
Committers 1
 - Joyee Cheung <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/60250
Refs: https://github.com/nodejs/node/issues/59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/60250
Refs: https://github.com/nodejs/node/issues/59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 13 Oct 2025 20:58:14 GMT
   ✔  Approvals: 4
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/60250#pullrequestreview-3339971328
   ✔  - Ulises Gascón (@UlisesGascon): https://github.com/nodejs/node/pull/60250#pullrequestreview-3340288264
   ✔  - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/60250#pullrequestreview-3342814703
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/60250#pullrequestreview-3343476785
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-10-15T23:21:32Z: https://ci.nodejs.org/job/node-test-pull-request/69751/
- Querying data for job/node-test-pull-request/69751/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 60250
From https://github.com/nodejs/node
 * branch                  refs/pull/60250/merge -> FETCH_HEAD
✔  Fetched commits as 71f5b1cbf054..f14e663c3ab1
--------------------------------------------------------------------------------
[main c22cc8aad3] test: move sea tests into test/sea
 Author: Joyee Cheung <[email protected]>
 Date: Mon Oct 13 22:42:22 2025 +0200
 24 files changed, 105 insertions(+), 107 deletions(-)
 create mode 100644 test/sea/sea.status
 rename test/{sequential => sea}/test-single-executable-application-asset-keys-empty.js (97%)
 rename test/{sequential => sea}/test-single-executable-application-asset-keys.js (98%)
 rename test/{sequential => sea}/test-single-executable-application-assets-raw.js (98%)
 rename test/{sequential => sea}/test-single-executable-application-assets.js (96%)
 rename test/{sequential => sea}/test-single-executable-application-disable-experimental-sea-warning.js (97%)
 rename test/{sequential => sea}/test-single-executable-application-empty.js (99%)
 rename test/{sequential => sea}/test-single-executable-application-exec-argv-empty.js (96%)
 rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-cli.js (96%)
 rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-env.js (98%)
 rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-none.js (96%)
 rename test/{sequential => sea}/test-single-executable-application-exec-argv.js (99%)
 rename test/{sequential => sea}/test-single-executable-application-inspect-in-sea-flags.js (98%)
 rename test/{sequential => sea}/test-single-executable-application-inspect.js (99%)
 rename test/{sequential => sea}/test-single-executable-application-snapshot-and-code-cache.js (98%)
 rename test/{sequential => sea}/test-single-executable-application-snapshot-worker.js (97%)
 rename test/{sequential => sea}/test-single-executable-application-snapshot.js (95%)
 rename test/{sequential => sea}/test-single-executable-application-use-code-cache.js (97%)
 rename test/{sequential => sea}/test-single-executable-application.js (97%)
 rename test/{parallel => sea}/test-single-executable-blob-config-errors.js (90%)
 rename test/{parallel => sea}/test-single-executable-blob-config.js (99%)
 create mode 100644 test/sea/testcfg.py
[main 46a1ded0e2] test: skip sea tests on x64 macOS
 Author: Joyee Cheung <[email protected]>
 Date: Mon Oct 13 22:57:48 2025 +0200
 1 file changed, 1 insertion(+), 19 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: move sea tests into test/sea

This makes skipping/running these tests easier to manage with a
dedicated test runner that can be tweaked for SEA.

PR-URL: #60250
Refs: #59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

[detached HEAD 06bf080634] test: move sea tests into test/sea
Author: Joyee Cheung <[email protected]>
Date: Mon Oct 13 22:42:22 2025 +0200
24 files changed, 105 insertions(+), 107 deletions(-)
create mode 100644 test/sea/sea.status
rename test/{sequential => sea}/test-single-executable-application-asset-keys-empty.js (97%)
rename test/{sequential => sea}/test-single-executable-application-asset-keys.js (98%)
rename test/{sequential => sea}/test-single-executable-application-assets-raw.js (98%)
rename test/{sequential => sea}/test-single-executable-application-assets.js (96%)
rename test/{sequential => sea}/test-single-executable-application-disable-experimental-sea-warning.js (97%)
rename test/{sequential => sea}/test-single-executable-application-empty.js (99%)
rename test/{sequential => sea}/test-single-executable-application-exec-argv-empty.js (96%)
rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-cli.js (96%)
rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-env.js (98%)
rename test/{sequential => sea}/test-single-executable-application-exec-argv-extension-none.js (96%)
rename test/{sequential => sea}/test-single-executable-application-exec-argv.js (99%)
rename test/{sequential => sea}/test-single-executable-application-inspect-in-sea-flags.js (98%)
rename test/{sequential => sea}/test-single-executable-application-inspect.js (99%)
rename test/{sequential => sea}/test-single-executable-application-snapshot-and-code-cache.js (98%)
rename test/{sequential => sea}/test-single-executable-application-snapshot-worker.js (97%)
rename test/{sequential => sea}/test-single-executable-application-snapshot.js (95%)
rename test/{sequential => sea}/test-single-executable-application-use-code-cache.js (97%)
rename test/{sequential => sea}/test-single-executable-application.js (97%)
rename test/{parallel => sea}/test-single-executable-blob-config-errors.js (90%)
rename test/{parallel => sea}/test-single-executable-blob-config.js (99%)
create mode 100644 test/sea/testcfg.py
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: skip sea tests on x64 macOS

It's unlikely that anyone would invest in fixing them on x64 macOS in the
near future, now that x64 macOS is being phased out. Simply skip them
for now.

PR-URL: #60250
Refs: #59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

[detached HEAD a989bf5ae3] test: skip sea tests on x64 macOS
Author: Joyee Cheung <[email protected]>
Date: Mon Oct 13 22:57:48 2025 +0200
1 file changed, 1 insertion(+), 19 deletions(-)
Successfully rebased and updated refs/heads/main.

✔ 06bf0806343840ccfd349f0750c2ae45dffa8e59
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 4:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
✖ a989bf5ae39b905fac667d86bdec17eb52267b0e
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✖ 1:72 Line should be <= 72 columns. line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 5:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/18579155019

This makes skipping/running these tests easier to manage with a
dedicated test runner that can be tweaked for SEA.

PR-URL: nodejs#60250
Refs: nodejs#59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
It's unlikely that anyone would invest in fixing them on x64 macOS
in the near future, now that x64 macOS is being phased out.
Simply skip them for now.

PR-URL: nodejs#60250
Refs: nodejs#59553
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@joyeecheung joyeecheung merged commit 9e8c753 into nodejs:main Oct 17, 2025
19 checks passed
@joyeecheung
Copy link
Member Author

Fixed the commit message line length and landed manually in c2f3c21...9e8c753

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

Labels

commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. 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