test: split test-crypto-keygen.js #49221
Conversation
To avoid timing out on ARM machines in the CI.
nodejs-github-bot
added
needs-ci
github-actions
bot
removed
the
request-ci
This comment was marked as outdated.
This comment was marked as outdated.
|
Added some additional changes other than copy-pasting, locally before the changes: After: This is on a relatively fast machine (it's on ARM, I had to comment out the SKIP in |
|
I think the length changes in the last commit should not affect the effectiveness of the tests, but cc @nodejs/crypto to be sure |
github-actions
bot
removed
the
request-ci
Can we run a stress test to find out? |
|
Let's see if the stress test job understands wildcard.. https://ci.nodejs.org/job/node-stress-single-test/419/ |
|
Welp, it looks like the stress test job does not understand wildcards, let's see if we can pass all the tests into it... https://ci.nodejs.org/job/node-stress-single-test/421/ if it still does not work, I guess we can only rely on the full CI to check. |
github-actions
bot
removed
the
request-ci
panva
added
the
author ready
panva
removed
the
author ready
| // Tests that a key pair can be used for encryption / decryption. | ||
| function testEncryptDecrypt(publicKey, privateKey) { | ||
| const message = 'Hello Node.js world!'; | ||
| const plaintext = Buffer.from(message, 'utf8'); | ||
| for (const key of [publicKey, privateKey]) { | ||
| const ciphertext = publicEncrypt(key, plaintext); | ||
| const received = privateDecrypt(privateKey, ciphertext); | ||
| assert.strictEqual(received.toString('utf8'), message); | ||
| } | ||
| } | ||
|
|
||
| // Tests that a key pair can be used for signing / verification. | ||
| function testSignVerify(publicKey, privateKey) { | ||
| const message = Buffer.from('Hello Node.js world!'); | ||
|
|
||
| function oldSign(algo, data, key) { | ||
| return createSign(algo).update(data).sign(key); | ||
| } | ||
|
|
||
| function oldVerify(algo, data, key, signature) { | ||
| return createVerify(algo).update(data).verify(key, signature); | ||
| } | ||
|
|
||
| for (const signFn of [sign, oldSign]) { | ||
| const signature = signFn('SHA256', message, privateKey); | ||
| for (const verifyFn of [verify, oldVerify]) { | ||
| for (const key of [publicKey, privateKey]) { | ||
| const okay = verifyFn('SHA256', message, key, signature); | ||
| assert(okay); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
These functions have very generic names but very specific implementations. For example, testEncryptDecrypt works with certain asymmetric key pairs only, whereas most encryption and decryption operations in cryptography rely on symmetric keys.
There was a problem hiding this comment.
Do you have a suggestions for the names? i think generally it's fine to have very generic names in the test commons - that's what we've been doing for many tests utils too. The point is just sharing code among tests. The were named that way in one long test test anyway, so keeping the name doesn't make much of a difference.
There was a problem hiding this comment.
The were named that way in one long test test anyway, so keeping the name doesn't make much of a difference.
Yes, in a test file that exclusively used asymmetric encryption. But moving them into common, these functions become available to all tests.
I don't want to make this more work for you than it has to be, so I am fine with leaving the names as they are.
Mind that moving to pummel means usually people do not run them locally (we don't run them in |
joyeecheung
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in a81d5e1...b781eaf |
To avoid timing out on ARM machines in the CI. PR-URL: nodejs#49221 Refs: nodejs#49202 Refs: nodejs#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#49221 Refs: nodejs#49202 Refs: nodejs#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#49221 Refs: nodejs#49202 Refs: nodejs#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
To avoid timing out on ARM machines in the CI. PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
To avoid timing out on ARM machines in the CI. PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#49221 Refs: nodejs/node#49202 Refs: nodejs/node#41206 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
To avoid timing out on slow machines in the CI.
Refs: #49202
Refs: #41206