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.

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

test: skip SEA tests when SEA generation fails #51887

Closed
wants to merge 2 commits into from

Conversation

joyeecheung
Copy link
Member

In the SEA tests, if any of these steps fail:

  1. Copy the executable
  2. Inject the SEA blob
  3. Signing the SEA

We skip the test because the error likely comes from the system or postject and is not something the Node.js core can fix. We only leave an exception for a basic test that test injecting empty files as SEA to ensure the workflow is working (but we still skip if copying fails or signing fails on Windows).

Refs: #49630

In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).
@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 Feb 26, 2024
@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2024
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 26, 2024
@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 1, 2024
@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 Mar 1, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/51887
✔  Done loading data for nodejs/node/pull/51887
----------------------------------- PR info ------------------------------------
Title      test: skip SEA tests when SEA generation fails (#51887)
Author     Joyee Cheung  (@joyeecheung)
Branch     joyeecheung:skip-sea -> nodejs:main
Labels     test, needs-ci, commit-queue-squash
Commits    2
 - test: skip SEA tests when SEA generation fails
 - fixup! test: skip SEA tests when SEA generation fails
Committers 2
 - Joyee Cheung 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/51887
Refs: https://github.com/nodejs/node/issues/49630
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51887
Refs: https://github.com/nodejs/node/issues/49630
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Mon, 26 Feb 2024 16:04:07 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51887#pullrequestreview-1901910843
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/51887#pullrequestreview-1910607541
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2024-02-26T21:22:47Z: https://ci.nodejs.org/job/node-test-pull-request/57443/
- Querying data for job/node-test-pull-request/57443/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8113019460

@joyeecheung
Copy link
Member Author

joyeecheung commented Mar 1, 2024

hmm, somehow the bot cannot understand that CI has passed (even though locally git node land understands?)

Landed in 17e0e3e.

@joyeecheung joyeecheung closed this Mar 1, 2024
joyeecheung added a commit that referenced this pull request Mar 1, 2024
In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).

PR-URL: #51887
Refs: #49630
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this pull request Mar 7, 2024
In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).

PR-URL: #51887
Refs: #49630
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@targos targos mentioned this pull request Mar 7, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).

PR-URL: #51887
Refs: #49630
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).

PR-URL: #51887
Refs: #49630
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
In the SEA tests, if any of these steps fail:

1. Copy the executable
2. Inject the SEA blob
3. Signing the SEA

We skip the test because the error likely comes from the system or
postject and is not something the Node.js core can fix. We only leave
an exception for a basic test that test injecting empty files as
SEA to ensure the workflow is working (but we still skip if copying
fails or signing fails on Windows).

PR-URL: nodejs#51887
Refs: nodejs#49630
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
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-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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.

5 participants