Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: using common.fixtures.fixturesDir in test-https-timeout-server.js #15871

Closed
wants to merge 1 commit into from
Closed

test: using common.fixtures.fixturesDir in test-https-timeout-server.js #15871

wants to merge 1 commit into from

Conversation

nicolasnoble
Copy link
Contributor

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows [commit guidelines]

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdex mscdex added the https Issues or 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
@Trott
Copy link
Member

Trott commented Oct 8, 2017

Hi, @nicolasnoble! Welcome and thank you for this! Can you update it to use fixtures.readKey()? See docs at https://github.com/nodejs/node/tree/master/test/common#fixtures-module

jasnell
jasnell reviewed Oct 10, 2017
View reviewed changes
test/parallel/test-https-timeout-server.js Outdated
key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`),
cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`),
key: fs.readFileSync(`${fixtures.fixturesDir}/keys/agent1-key.pem`),
cert: fs.readFileSync(`${fixtures.fixturesDir}/keys/agent1-cert.pem`),
Copy link
Member

Choose a reason for hiding this comment

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

these can be:

key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem')

@joyeecheung
Copy link
Member

Ping @nicolasnoble

1 similar comment
@BridgeAR
Copy link
Member

Ping @nicolasnoble

@nicolasnoble @Trott
test: use fixtures.readKey in https-timeout-server
6c2e23f 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.
@Trott
Copy link
Member

Trott commented Oct 21, 2017

Nits fixed.

CI: https://ci.nodejs.org/job/node-test-pull-request/10891/

gireeshpunathil
gireeshpunathil approved these changes Oct 22, 2017
View reviewed changes
Copy link
Member

@gireeshpunathil gireeshpunathil left a comment

Choose a reason for hiding this comment

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

CI failures unrelated and good to land, but wait for a couple of LGTMs

@gireeshpunathil
Copy link
Member

Landed in cfea677 , thanks for the contribution!

MylesBorins pushed a commit that referenced this pull request Oct 23, 2017
@nicolasnoble @MylesBorins
test: use fixtures.readKey in https-timeout-server
0d5ee95 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: #15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
@nicolasnoble @addaleax
test: use fixtures.readKey in https-timeout-server
2ca46a1 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: nodejs/node#15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
@nicolasnoble @MylesBorins
test: use fixtures.readKey in https-timeout-server
ddf2550 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: #15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@nicolasnoble @MylesBorins
test: use fixtures.readKey in https-timeout-server
adcf0e8 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: #15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@nicolasnoble @MylesBorins
test: use fixtures.readKey in https-timeout-server
9c229b4 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: #15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
@nicolasnoble @addaleax
test: use fixtures.readKey in https-timeout-server
785500c 
Replace `common.fixturesDir` usage in test-https-timeout-server.js with
`fixtures.readKey()`.

PR-URL: nodejs/node#15871
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell left review comments

@joyeecheung joyeecheung joyeecheung approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

Assignees
No one assigned
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@nicolasnoble @Trott @joyeecheung @BridgeAR @gireeshpunathil @jasnell @mscdex @MylesBorins @nodejs-github-bot