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

postject sometimes cannot inject the SEA blob on Windows, resulting in flaky test #49630

Open
joyeecheung opened this issue Sep 12, 2023 · 8 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. single-executable Issues and PRs related to single-executable applications

Comments

@joyeecheung
Copy link
Member

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

14:07:48     �[36mStart injection of NODE_SEA_BLOB in C:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.944\sea.exe...�[0m
14:07:48     �[31mError: Aborted(). Build with -sASSERTIONS for more info.�[0m

It looks like sometimes postject is unable to inject the blob? cc @RaisinTen

@joyeecheung joyeecheung added flaky-test Issues and PRs related to the tests with unstable failures on the CI. single-executable Issues and PRs related to single-executable applications labels Sep 12, 2023
@RaisinTen
Copy link
Contributor

My Windows setup is not working right now. I can try to take a look in a couple of days.

@joyeecheung
Copy link
Member Author

joyeecheung commented Sep 18, 2023

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

@joyeecheung
Copy link
Member Author

Not sure why but this has stopped showing up in the CI..

@anonrig
Copy link
Member

anonrig commented Oct 17, 2023

nodejs-github-bot pushed a commit that referenced this issue Oct 18, 2023
PR-URL: #50223
Refs: #49630
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this issue Oct 23, 2023
PR-URL: #50223
Refs: #49630
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
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>
targos pushed a commit that referenced this issue Nov 11, 2023
PR-URL: #50223
Refs: #49630
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@joyeecheung
Copy link
Member Author

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.

@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 26, 2024

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.

14:23:39     �[36mStart injection of NODE_SEA_BLOB in C:\workspace\node-test-binary-windows-js-suites\node\test\.tmp.960\sea.exe...�[0m
14:23:39     �[31mError: Aborted(). Build with -sASSERTIONS for more info.�[0m

We could probably investigate building it with ASSERTIONS.

@joyeecheung joyeecheung changed the title test-single-executable-application-snapshot(-and-code-cache) is flaky postject sometimes cannot inject the SEA blob on Windows, resulting in flaky test Feb 26, 2024
@joyeecheung
Copy link
Member Author

joyeecheung commented Feb 26, 2024

I wonder if we just skip the test if any of the following failed:

  1. Copy
  2. Inject
  3. Code sign

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.

joyeecheung added a commit that referenced this issue 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 issue 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 issue 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 issue 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>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue 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
flaky-test Issues and PRs related to the tests with unstable failures on the CI. single-executable Issues and PRs related to single-executable applications
Projects
None yet
Development

No branches or pull requests

4 participants