-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
fix: unsetting env variables broke #34239
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We should also fix
npm run itest
instances on the bots, they are running electron too. - Perhaps we can run something like
unset ELECTRON_SKIP_BINARY_DOWNLOAD
or a similar command instead of using this syntax? Should be less widespread in our yml files.
Test results for "tests 1"4 failed 9 flaky37509 passed, 654 skipped Merge workflow run. |
Test results for "tests 2"1 fatal errors, not part of any test 103 flaky257156 passed, 9904 skipped Merge workflow run. |
Sounds like a GitHub Actions bug - lets try to file upstream as well? |
Test results for "tests others"3 fatal errors, not part of any test |
Most of our tests set
ELECTRON_SKIP_BINARY_DOWNLOAD=1
to save on runtime. Intest_others.yml
we set that variable at the top-level and then unset it for the Electron tests. That syntax seems to have regressed - when SSHing into it, the variable is still set. This breaks our electron tests, because they're now missing binaries. To work around this regression, replaced the top-level variable with per-test variables.