Skip to content

test: replace fixturesDir in tls-connect-no-host - #15986

Closed
dtex wants to merge 1 commit into
nodejs:masterfrom
dtex:master
Closed

test: replace fixturesDir in tls-connect-no-host#15986
dtex wants to merge 1 commit into
nodejs:masterfrom
dtex:master

Conversation

@dtex

@dtex dtex commented Oct 6, 2017

Copy link
Copy Markdown
Contributor

test: replace fixturesDir in test-tls-connect-no-host.js

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to Node.js core tests and test infrastructure. label Oct 6, 2017
@dtex dtex changed the title replace fixturesDir in test-tls-connect-no-host.js Oct 6, 2017
@dtex dtex changed the title tet:replace fixturesDir in test-tls-connect-no-host.js Oct 6, 2017
@dtex dtex changed the title test:replace fixturesDir in test-tls-connect-no-host.js Oct 6, 2017
@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Oct 6, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @dtex ! This is good but can be made even better using fixtures.readSync, e.g.

const cert = fixtures.readSync('test_cert.pem');
const key = fixtures.readSync('test_key.pem');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing me to that. I've made that update.

I went ahead and amended the existing commit rather than add a second for such a small change. I hope that's okay.

@dtex

dtex commented Oct 12, 2017

Copy link
Copy Markdown
Contributor Author

@joyeecheung Is that something I need to take action on?

@jasnell

jasnell commented Oct 13, 2017

Copy link
Copy Markdown
Member

@dtex ... nope! That's just our testing run. It's there for record keeping purposes.

jasnell pushed a commit that referenced this pull request Oct 13, 2017
PR-URL: #15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell

jasnell commented Oct 13, 2017

Copy link
Copy Markdown
Member

Landed in c0bba73. Thank you for the contribution and for participating in the code and learn!

@jasnell jasnell closed this Oct 13, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
PR-URL: nodejs/node#15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
PR-URL: #15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
PR-URL: #15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
PR-URL: #15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
PR-URL: #15986
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to Node.js core tests and test infrastructure. tls Issues and PRs related to the tls subsystem.

9 participants