-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
postject sometimes cannot inject the SEA blob on Windows, resulting in flaky test #49630
Comments
My Windows setup is not working right now. I can try to take a look in a couple of days. |
It can also fail on Linux during loading, I suspect we need https://chromium-review.googlesource.com/c/v8/v8/+/4608330 to fix it, but that seems separate from the postject failure on Windows |
Not sure why but this has stopped showing up in the CI.. |
PR-URL: nodejs#50223 Refs: nodejs#49630 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
I am not sure why they are still showing up in reliability reports after #50223 but they are still flaky, and it appears that other SEA tests are flaky too probably due to the same reason nodejs/reliability#718 - SIGSEGV when executing the SEA, on RHEL, Windows and macOS. Maybe we should open a separate issue because the original issue happened during injection. |
This also shows up for the asset test on Windows: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/26136/RUN_SUBSET=2,nodes=win2016-COMPILED_BY-vs2022-x86/console. I suspect that postject may have trouble injecting slightly bigger files under certain circumstances.
We could probably investigate building it with ASSERTIONS. |
I wonder if we just skip the test if any of the following failed:
In those cases, the failures are unlikely to be fixable by Node.js core anyway...though it might still be useful to know when it fails 100% which indicates that something is wrong with our workflow. Not sure how we can know that in single tests though. |
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>
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>
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>
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>
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>
Refs: nodejs/reliability#663
And from a recent run that includes #49543 https://ci.nodejs.org/job/node-test-binary-windows-js-suites/23086/RUN_SUBSET=1,nodes=win2016-COMPILED_BY-vs2022-x86/console
It looks like sometimes postject is unable to inject the blob? cc @RaisinTen
The text was updated successfully, but these errors were encountered: