Replace common.fixtiresDir with readFixtureKey#16817
Replace common.fixtiresDir with readFixtureKey#16817wsierakowski wants to merge 1 commit intonodejs:masterfrom
Conversation
|
Typo in commit message and unnecessary merge commit. |
gireeshpunathil
added
the
code-and-learn
test/parallel/test-tls-js-stream.js
Outdated
There was a problem hiding this comment.
these can be:
key: fixtures.readSync('agent1-key.pem'),
cert: fixtures.readSync('agent1-cert.pem')
There was a problem hiding this comment.
@gireeshpunathil (and for @wsierakowski's benefit too): I think you mean readKey() rather than readSync(). (Aside: API should have probably been readKeySync() rather than readKey() but whatever. :-D )
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
Hi, @wsierakowski! Welcome and thank you for the PR! readFixtureKey() is not a function in fixtures. This test with this change will not run. Can you change that function name to readKey() instead? You can run the test from the command line with tools/test.py test/parallel/test-tls-js-stream.js to make sure it works. After commiting the change to your branch, pushing it to your GitHub repo will update this PR with the new code.
(You don't need to worry about removing the merge commit mentioned by mscdex because we will squash that out when landing your code. But if you get rid of it, that's fine too.)
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
I went ahead and made the requested changes myself. Hope that's OK.
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: nodejs#16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in fa8aee8. |
|
Thanks for fixing this @Trott, sorry for not doing this earlier by myself, I got too involved with conference-related activities in the meantime :) |
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: #16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: #16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: #16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: #16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Use fixtures module in test/parallel/test-tls-js-stream.js. PR-URL: #16817 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
7 participants
Replaced common.fixturesDir with the readFixtureKey from the fixtures module.