Skip to content

test: use fs.copyFileSync() - #20340

Closed
richardlau wants to merge 1 commit into
nodejs:masterfrom
richardlau:copyfile
Closed

test: use fs.copyFileSync()#20340
richardlau wants to merge 1 commit into
nodejs:masterfrom
richardlau:copyfile

Conversation

@richardlau

@richardlaurichardlau commented Apr 26, 2018

Copy link
Copy Markdown
Member

These tests copy the node binary into the temp dir. Use the potentially more efficient fs.copyFileSync() instead of reading the whole file in and writing the whole file out in JavaScript.

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

@richardlau
test: use fs.copyFileSync()
71139a6
Use the potentially more efficient fs.copyFileSync() instead of reading
the whole file in and writing the whole file out in JavaScript.
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Apr 26, 2018
@richardlau

richardlau commented Apr 26, 2018

Copy link
Copy Markdown
MemberAuthor

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

@richardlaurichardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 26, 2018
@trivikr

trivikr commented May 3, 2018

Copy link
Copy Markdown
Member

CI re-run for node-test-commit-aix https://ci.nodejs.org/job/node-test-commit-aix/14695/

@trivikr

trivikr commented May 3, 2018

Copy link
Copy Markdown
Member

Landed in b457ec8

@trivikrtrivikr closed this May 3, 2018
trivikr pushed a commit that referenced this pull request May 3, 2018
@richardlau@trivikr
test: use fs.copyFileSync()
b457ec8
Use the potentially more efficient fs.copyFileSync() instead of reading
the whole file in and writing the whole file out in JavaScript.
PR-URL: #20340
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
@richardlau@MylesBorins
test: use fs.copyFileSync()
f438438
Use the potentially more efficient fs.copyFileSync() instead of reading
the whole file in and writing the whole file out in JavaScript.
PR-URL: #20340
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorinsMylesBorins mentioned this pull request May 8, 2018
Merged
MylesBorins pushed a commit that referenced this pull request May 8, 2018
@richardlau@MylesBorins
test: use fs.copyFileSync()
4dce39a
Use the potentially more efficient fs.copyFileSync() instead of reading
the whole file in and writing the whole file out in JavaScript.
PR-URL: #20340
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@addaleaxaddaleaxaddaleax approved these changes
@cjihrigcjihrigcjihrig approved these changes
@tniessentniessentniessen approved these changes
@BridgeARBridgeARBridgeAR approved these changes
@trivikrtrivikrtrivikr approved these changes
+2 more reviewers
@danbevdanbevdanbev approved these changes
@XadillaXXadillaXXadillaX approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@richardlau@trivikr@danbev@jasnell@addaleax@cjihrig@XadillaX@tniessen@BridgeAR@nodejs-github-bot