test: fix flaky test-crypto-timing-safe-dqual-benchmarks#38476
test: fix flaky test-crypto-timing-safe-dqual-benchmarks#38476Trott wants to merge 1 commit intonodejs:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
You can see in #38226 where I bisected to determine that adding I still don't know if the issue here is with the test or with there really being a timing problem in Node.js core, but I'm guessing the latter but that it only manifests with a very fast CPU. Regardless, fast-track to fix CI? Maybe leave the underlying issue open and investigate what's going on here? |
|
( |
|
https://ci.nodejs.org/job/node-stress-single-test/293/ is a stress test to show improvement. It will probably still show some failures, but not nearly as many as https://ci.nodejs.org/job/node-stress-single-test/273/ from this morning which ran against master and failed 941 times out of 1000 runs. |
|
@nodejs/testing |
|
CI is green. Let's land this? Stress test failed 192 times out of 1000 runs, which isn't great, but is way better than the current master branch which failed 941 times out of 1000 runs. |
|
Would it be worth trying to identify which |
aduh95
left a comment
There was a problem hiding this comment.
That's a very surprising result, but the stress test results look promising.
|
Fast-track has been requested by @jasnell. Please 👍 to approve. |
|
Landed in c7ccab3 |
I've got another fix in the works that should make it possible to add back the Proxy() stuff removed here. Stay tuned. |
Fixes: #38226