test: refactor common.ddCommand()#23411
Closed
Trott wants to merge 6 commits intonodejs:masterfrom
Closed
Conversation
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation.
refack
reviewed
Oct 10, 2018
| } else { | ||
| return `dd if=/dev/zero of="${filename}" bs=1024 count=${kilobytes}`; | ||
| } | ||
| function ddCommand(filename) { |
Contributor
There was a problem hiding this comment.
Then why not call it createZeroFilledFile?
| ### ddCommand(filename) | ||
|
|
||
| Platform normalizes the `dd` command | ||
| Creates a 10Mb file of all null characters. |
Contributor
There was a problem hiding this comment.
Mb -> MB to not be confused with megabit?
richardlau
approved these changes
Oct 10, 2018
| } | ||
| function ddCommand(filename) { | ||
| const fd = fs.openSync(filename, 'w'); | ||
| fs.ftruncateSync(fd, 10240 * 1024); |
Member
There was a problem hiding this comment.
I think this should either be 10 * 1024 * 1024 (my preference) or 10485760.
lpinca
approved these changes
Oct 10, 2018
Member
|
Two failures across the board: test-fs-error-messagesThe removed fixture is also referenced in https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel72-s390x/6012/console test-module-loadingThis PR makes |
richardlau
requested changes
Oct 10, 2018
Member
Author
|
Ugh! At least one of those tests writes to the fixtures directory. 😞 |
Member
Author
|
Fixed all nits, and (I think) fixed the test problems. |
Change common.ddCommand() to common.createZeroFilledFile().
Member
Author
|
Whoops, missed a nit. Now I think I got all the nits. |
refack
approved these changes
Oct 10, 2018
Member
Author
Member
Author
|
CI is green. Does this look good to you now, @richardlau? |
richardlau
approved these changes
Oct 10, 2018
Trott
added
the
author ready
benjamingr
approved these changes
Oct 11, 2018
thefourtheye
approved these changes
Oct 11, 2018
jasnell
approved these changes
Oct 12, 2018
gdams
approved these changes
Oct 12, 2018
Trott
added a commit
to Trott/io.js
that referenced
this pull request
Oct 12, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. PR-URL: nodejs#23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
Trott
added a commit
to Trott/io.js
that referenced
this pull request
Oct 12, 2018
Change common.ddCommand() to common.createZeroFilledFile(). PR-URL: nodejs#23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
Member
Author
|
Landed in bcbb937...3d62d38 |
Member
|
Should this be backported to |
Member
Author
Trott
added a commit
to Trott/io.js
that referenced
this pull request
Oct 13, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. PR-URL: nodejs#23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
Trott
added a commit
to Trott/io.js
that referenced
this pull request
Oct 13, 2018
Change common.ddCommand() to common.createZeroFilledFile(). PR-URL: nodejs#23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
targos
pushed a commit
that referenced
this pull request
Oct 13, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
targos
pushed a commit
that referenced
this pull request
Oct 13, 2018
Change common.ddCommand() to common.createZeroFilledFile(). Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
jasnell
pushed a commit
that referenced
this pull request
Oct 17, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
jasnell
pushed a commit
that referenced
this pull request
Oct 17, 2018
Change common.ddCommand() to common.createZeroFilledFile(). PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 30, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Oct 30, 2018
Change common.ddCommand() to common.createZeroFilledFile(). Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
Closed
rvagg
pushed a commit
that referenced
this pull request
Nov 28, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
rvagg
pushed a commit
that referenced
this pull request
Nov 28, 2018
Change common.ddCommand() to common.createZeroFilledFile(). Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Nov 29, 2018
* Remove different paths for Windows and POSIX. * Remove fixtures file. Simply run the command immediately/directly. * Since it is never called with more than one value for kilobytes, eliminate that argument. * Update/simplify tests that use this function. (They no longer need to use child_process to run the command.) * Update documentation. Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Nov 29, 2018
Change common.ddCommand() to common.createZeroFilledFile(). Backport-PR-URL: #23630 PR-URL: #23411 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: George Adams <[email protected]>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
eliminate that argument.
use child_process to run the command.)
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes