-
Notifications
You must be signed in to change notification settings - Fork 29.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
test: limit stack size in test-error-serdes
#52674
test: limit stack size in test-error-serdes
#52674
Conversation
Stress-test CI: https://ci.nodejs.org/job/node-stress-single-test/497/ For comparison, stress-test CI on |
https://ci.nodejs.org/job/node-stress-single-test/499/ The test still fails occasionally but maybe it's less flaky. |
test-macOS: test/pummel/test-crypto-timing-safe-equal-benchmarks.js#L109 seems to be failing consistently (5/5) on this PR 🤔 I restarted it a fifth time, that doesn't sound promising though EDIT: it’s also failing on other PRs, confirming it’s unrelated. |
It seems to make the test more flaky on node-test-binary-armv7l 😬 |
This is strange... IIRC default stack size is lower on 32bit platforms, but it shouldn't be lower than 128KB. 🤔 |
From the machine that failed, I started > process.pid
4172727
# in another terminal
$ cat /proc/4172727/limits
Limit Soft Limit Hard Limit Units
Max cpu time unlimited unlimited seconds
Max file size unlimited unlimited bytes
Max data size unlimited unlimited bytes
Max stack size 8388608 unlimited bytes |
Is it possible to perform stress-single-test on binary-armv7l? If the test fails consistently there, it might be easier to debug. |
I get the same as you on the CI machine:
|
@@ -0,0 +1,29 @@ | |||
// Flags: --expose-internals --stack-size=128 |
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.
Are these just three tests testing the same thing but with different stack sizes?
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.
Yep, these belong to af75425 just to see if there's any difference in CI results on different platforms.
That commit also extracted the stack exhausting parts (primary suspect of flakiness) from the rest of the test to confirm that cyclic errors are the cause of timeout.
This should significantly speed up the part of test that exceeds the maximum call stack size and hopefully deflake it on CI.
Might fix: #52630