Skip to content

test: replace fixturesDir with fixtures in https agent create connect… - #15913

Closed
Greg-GB wants to merge 2 commits into
nodejs:masterfrom
Greg-GB:NodeInteractiveFixtures
Closed

test: replace fixturesDir with fixtures in https agent create connect…#15913
Greg-GB wants to merge 2 commits into
nodejs:masterfrom
Greg-GB:NodeInteractiveFixtures

Conversation

@Greg-GB

@Greg-GB Greg-GB commented Oct 6, 2017

Copy link
Copy Markdown
Contributor

…ion test

Replace fixturesDir with common.fixtures module and removed unused fs module.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • 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
watilde
watilde previously requested changes Oct 6, 2017
const options = {
key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`),
cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`),
key: fixtures.readSync('keys/agent1-key.pem'),

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.

I think we also can use fixtures.readKey.

@mscdex mscdex added the https Issues and PRs related to the https 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
@gibfahn
gibfahn dismissed watilde’s stale review October 7, 2017 18:37

Requested changes have been made.

@gibfahn

gibfahn commented Oct 7, 2017

Copy link
Copy Markdown
Member
@gibfahn

gibfahn commented Oct 7, 2017

Copy link
Copy Markdown
Member

Landing...

@gibfahn

gibfahn commented Oct 7, 2017

Copy link
Copy Markdown
Member

Landed in d36433e

@gibfahn gibfahn closed this Oct 7, 2017
@gibfahn

gibfahn commented Oct 7, 2017

Copy link
Copy Markdown
Member

@Greg-GB congratulations on becoming a Node.js contributor!

gibfahn pushed a commit that referenced this pull request Oct 7, 2017
PR-URL: #15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 10, 2017
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
PR-URL: #15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
PR-URL: nodejs/node#15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 14, 2017
PR-URL: #15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@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: #15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
PR-URL: #15913
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@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. https Issues and PRs related to the https subsystem. test Issues and PRs related to Node.js core tests and test infrastructure.

8 participants