Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: use spawnSyncAndExitWithoutError in sea tests #49543

Closed
wants to merge 2 commits into from
Closed

test: use spawnSyncAndExitWithoutError in sea tests #49543

wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Sep 7, 2023 •
edited
Loading

test: use spawnSyncAndExitWithoutError in test/common/sea.js

To display more information when the command fails.

test: use spawnSyncAndExitWithoutError in sea tests

To display more information when the command fails.

Refs: nodejs/reliability#658

@joyeecheung
test: use spawnSyncAndExitWithoutError in test/common/sea.js
586b209 
To display more information when the command fails.
@joyeecheung
test: use spawnSyncAndExitWithoutError in sea tests
6b8ecfb 
To display more information when the command fails.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/single-executable

@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 Sep 7, 2023
@joyeecheung
Copy link
Member Author

joyeecheung commented Sep 7, 2023 •
edited
Loading

By the way, the two tests are flaky in the CI but all we know is that postject aborted cc @RaisinTen (I see that there are output to stdout too, which execFileSync won't show for failed processes, but spawnSyncAndExitWithoutError would, hence the change)

not ok 943 sequential/test-single-executable-application-snapshot-and-code-cache
  ---
  duration_ms: 21301.53500
  severity: fail
  exitcode: 1
  stack: |-
    Aborted()
    node:child_process:929
        throw err;
        ^
    
    Error: Command failed: C:\workspace\node-test-binary-windows-js-suites\node\Release\node.exe C:\workspace\node-test-binary-windows-js-suites\node\test\fixtures\postject-copy\node_modules\postject\dist\cli.js C:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.942\sea.exe NODE_SEA_BLOB C:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.942\sea-prep.blob --sentinel-fuse NODE_SEA_FUSE_fce680ab2cc467b6e072b8b5df1996b2
    Aborted()
    
        at checkExecSyncError (node:child_process:890:11)
        at execFileSync (node:child_process:926:15)
        at injectAndCodeSign (C:\workspace\node-test-binary-windows-js-suites\node\test\common\sea.js:48:3)
        at Object.<anonymous> (C:\workspace\node-test-binary-windows-js-suites\node\test\sequential\test-singl...

RaisinTen
RaisinTen approved these changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

Good idea 👍

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53807/

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 8, 2023
This was referenced Sep 9, 2023
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53887/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53893/

@joyeecheung joyeecheung 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 Sep 11, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 11, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in dca4d84...65d396b

nodejs-github-bot pushed a commit that referenced this pull request Sep 11, 2023
@joyeecheung @nodejs-github-bot
test: use spawnSyncAndExitWithoutError in test/common/sea.js
31657f7 
To display more information when the command fails.

PR-URL: #49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs-github-bot pushed a commit that referenced this pull request Sep 11, 2023
@joyeecheung @nodejs-github-bot
test: use spawnSyncAndExitWithoutError in sea tests
65d396b 
To display more information when the command fails.

PR-URL: #49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@github-actions github-actions bot mentioned this pull request Sep 12, 2023
Open
19 tasks
This was referenced Sep 12, 2023
Closed
Open
This was referenced Sep 13, 2023
Open
Open
Open
Open
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@joyeecheung @ruyadorno
test: use spawnSyncAndExitWithoutError in test/common/sea.js
f79b153 
To display more information when the command fails.

PR-URL: #49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@joyeecheung @ruyadorno
test: use spawnSyncAndExitWithoutError in sea tests
3599eeb 
To display more information when the command fails.

PR-URL: #49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
This was referenced Sep 28, 2023
Closed
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@joyeecheung @alexfernandez
test: use spawnSyncAndExitWithoutError in test/common/sea.js
309ef81 
To display more information when the command fails.

PR-URL: nodejs#49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@joyeecheung @alexfernandez
test: use spawnSyncAndExitWithoutError in sea tests
6c5f261 
To display more information when the command fails.

PR-URL: nodejs#49543
Refs: nodejs/reliability#658
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anonrig anonrig anonrig approved these changes

@tony-go tony-go tony-go approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@joyeecheung @nodejs-github-bot @anonrig @tony-go @RaisinTen