Replace common.fixtiresDir with readFixtureKey - #16817
Conversation
Sorry, something went wrong.
|
Typo in commit message and unnecessary merge commit. |
Sorry, something went wrong.
There was a problem hiding this comment.
these can be:
key: fixtures.readSync('agent1-key.pem'),
cert: fixtures.readSync('agent1-cert.pem')
Sorry, something went wrong.
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 )
Sorry, something went wrong.
There was a problem hiding this comment.
@Trott - yes, thanks for the correction!
Sorry, something went wrong.
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.)
Sorry, something went wrong.
Trott
left a comment
There was a problem hiding this comment.
I went ahead and made the requested changes myself. Hope that's OK.
Sorry, something went wrong.
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
Thanks for fixing this @Trott, sorry for not doing this earlier by myself, I got too involved with conference-related activities in the meantime :) |
Sorry, something went wrong.
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>
Replaced common.fixturesDir with the readFixtureKey from the fixtures module.