test: skip SEA tests when SEA generation fails - #51887
Closed
joyeecheung wants to merge 2 commits into
Closed
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
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).
Copy link
Copy Markdown
Collaborator
|
Review requested:
|
Sorry, something went wrong.
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
lpinca
approved these changes
Feb 26, 2024
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
aymen94
approved these changes
Feb 27, 2024
benjamingr
approved these changes
Mar 1, 2024
Copy link
Copy Markdown
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/.ncuhttps://github.com/nodejs/node/actions/runs/8113019460 |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
|
hmm, somehow the bot cannot understand that CI has passed (even though locally git node land understands?) Landed in 17e0e3e. |
Sorry, something went wrong.
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>
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>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the SEA tests, if any of these steps fail:
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