Repository navigation
test: test-crypto-keygen.js should be split into multiple tests to avoid timeout in CI #49202
Description
Activity
- addedgood first issueIssues that are suitable for first-time contributors.Issues that are suitable for first-time contributors.
on Aug 16, 2023 If I want to reproduce it, can I run it with the command below?
python.exe tools/test.py -J --repeat=1000 test/parallel/test-crypto-keygen.js
You can just run it with
out/Release/node test/parallel/test-crypto-keygen.jsand see how long it takes and try to split it to make the split tests run faster, the time out only reproduces on very slow machines, running it many times would not make a difference if you run it on a fast machine.And test-crypto-dh.js too (which crams 19
crypto.createDiffieHellman()calls in one testI am going to do it to deflake the CI sooner than later.
- removedgood first issueIssues that are suitable for first-time contributors.Issues that are suitable for first-time contributors.
on Aug 17, 2023 I was checking to give it a try, but you've already fixed it. That's a good job. 👍
ok 57 parallel/test-crypto-secure-heap --- duration_ms: 1981.18900 ... ok 58 parallel/test-crypto-dh-constructor --- duration_ms: 4528.98300 ... ok 59 parallel/test-crypto-classes --- duration_ms: 5376.81000 ... ok 60 parallel/test-crypto-dh --- duration_ms: 9867.30400 ... ok 61 parallel/test-crypto-keygen --- duration_ms: 9152.01800Here is a list of crypto tests that took more than 1s for me locally (probably would be much slower in the CI on slow machines, in the failures referenced above, over 2 minutes)
Reacted by Jungku Lee- addedtestIssues and PRs related to Node.js core tests and test infrastructure.Issues and PRs related to Node.js core tests and test infrastructure.cryptoIssues and PRs related to the crypto subsystem.Issues and PRs related to the crypto subsystem.
on Aug 22, 2023 - added a commit that references this issue
on Sep 7, 2023 - added a commit that references this issue
on Sep 10, 2023 7 remaining items
- added a commit that references this issue
on Nov 15, 2023 - added 3 commits that reference this issue
on Nov 27, 2023 hi @joyeecheung seems like this was fixed and can be closed now, can it?
I think only the two largest tests from #49202 (comment) have been split, but then I think so far I haven't seen the other less large ones timing out in the CI, so maybe it's good for now. We can split some more if any of them time out again.
- added 8 commits that reference this issue
on Apr 25, 2024
It looks like #41206 is coming back to the CI, see https://ci.nodejs.org/job/node-test-binary-windows-js-suites/22596/RUN_SUBSET=2,nodes=win2012r2-COMPILED_BY-vs2019-x86/testReport/junit/(root)/parallel/test_crypto_keygen_/ It's a test with 1800+ lines that is crammed with computation-intensive crypto tests that seem to be logically separable. On my local machine which is quite powerful it still takes >5s to run. It would be good to split it into multiple tests to avoid timing out in the CI (I could do it if no one picks this up, but I feel like maybe people from @nodejs/crypto have better ideas about how to split them properly and give the split tests proper names - if I am doing it I would probably just name them
test-crypto-keygen-1.js,test-crypto-keygen-2.js..)