From edf1a7cda4a0059703bae04940ab9c0b760ad9fe Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 11:24:51 -0700 Subject: [PATCH 1/6] test: refactor common.ddCommand() * 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. --- test/common/README.md | 5 ++- test/common/index.js | 12 +++---- test/fixtures/create-file.js | 29 ---------------- test/parallel/test-http-chunk-problem.js | 43 +++++++++++------------- test/parallel/test-pipe-file-to-http.js | 9 ++--- 5 files changed, 28 insertions(+), 70 deletions(-) delete mode 100644 test/fixtures/create-file.js diff --git a/test/common/README.md b/test/common/README.md index c3f2c5f6b8a7f1..1d27da04098799 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -52,10 +52,9 @@ symlinks ([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716(v=vs.85).aspx)). On non-Windows platforms, this always returns `true`. -### ddCommand(filename, kilobytes) -* return [<Object>] +### ddCommand(filename) -Platform normalizes the `dd` command +Creates a 10Mb file of all null characters. ### disableCrashOnUnhandledRejection() diff --git a/test/common/index.js b/test/common/index.js index 75fe2e1548ea8a..828832b937b33b 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -28,7 +28,6 @@ const assert = require('assert'); const os = require('os'); const { exec, execSync, spawnSync } = require('child_process'); const util = require('util'); -const { fixturesDir } = require('./fixtures'); const tmpdir = require('./tmpdir'); const { bits, @@ -174,13 +173,10 @@ function childShouldThrowAndAbort() { }); } -function ddCommand(filename, kilobytes) { - if (isWindows) { - const p = path.resolve(fixturesDir, 'create-file.js'); - return `"${process.argv[0]}" "${p}" "${filename}" ${kilobytes * 1024}`; - } else { - return `dd if=/dev/zero of="${filename}" bs=1024 count=${kilobytes}`; - } +function ddCommand(filename) { + const fd = fs.openSync(filename, 'w'); + fs.ftruncateSync(fd, 10240 * 1024); + fs.closeSync(fd); } diff --git a/test/fixtures/create-file.js b/test/fixtures/create-file.js deleted file mode 100644 index ec0d8243d355e5..00000000000000 --- a/test/fixtures/create-file.js +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -const fs = require('fs'); - -var file_name = process.argv[2]; -var file_size = parseInt(process.argv[3]); - -var fd = fs.openSync(file_name, 'w'); -fs.ftruncateSync(fd, file_size); -fs.closeSync(fd); diff --git a/test/parallel/test-http-chunk-problem.js b/test/parallel/test-http-chunk-problem.js index f999f055fc0a91..dcc3a9c3fc8eed 100644 --- a/test/parallel/test-http-chunk-problem.js +++ b/test/parallel/test-http-chunk-problem.js @@ -63,34 +63,31 @@ function executeRequest(cb) { tmpdir.refresh(); -const ddcmd = common.ddCommand(filename, 10240); +common.ddCommand(filename); -cp.exec(ddcmd, function(err, stdout, stderr) { - assert.ifError(err); - server = http.createServer(function(req, res) { - res.writeHead(200); +server = http.createServer(function(req, res) { + res.writeHead(200); - // Create the subprocess - const cat = cp.spawn('cat', [filename]); + // Create the subprocess + const cat = cp.spawn('cat', [filename]); - // Stream the data through to the response as binary chunks - cat.stdout.on('data', (data) => { - res.write(data); - }); - - cat.stdout.on('end', () => res.end()); + // Stream the data through to the response as binary chunks + cat.stdout.on('data', (data) => { + res.write(data); + }); - // End the response on exit (and log errors) - cat.on('exit', (code) => { - if (code !== 0) { - console.error(`subprocess exited with code ${code}`); - process.exit(1); - } - }); + cat.stdout.on('end', () => res.end()); + // End the response on exit (and log errors) + cat.on('exit', (code) => { + if (code !== 0) { + console.error(`subprocess exited with code ${code}`); + process.exit(1); + } }); - server.listen(0, () => { - executeRequest(() => server.close()); - }); +}); + +server.listen(0, () => { + executeRequest(() => server.close()); }); diff --git a/test/parallel/test-pipe-file-to-http.js b/test/parallel/test-pipe-file-to-http.js index cfe289c30caa9d..70805873f1988e 100644 --- a/test/parallel/test-pipe-file-to-http.js +++ b/test/parallel/test-pipe-file-to-http.js @@ -25,7 +25,6 @@ const assert = require('assert'); const fs = require('fs'); const http = require('http'); const path = require('path'); -const cp = require('child_process'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -57,12 +56,8 @@ const server = http.createServer(function(req, res) { server.listen(0); server.on('listening', function() { - const cmd = common.ddCommand(filename, 10240); - - cp.exec(cmd, function(err) { - assert.ifError(err); - makeRequest(); - }); + common.ddCommand(filename); + makeRequest(); }); function makeRequest() { From fd0eb95a8c1fbddbabce4029e00d889f8cb6f1bc Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 12:50:35 -0700 Subject: [PATCH 2/6] fixup! test: refactor common.ddCommand() --- test/common/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 1d27da04098799..4d914f4f89e5ad 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -54,7 +54,7 @@ On non-Windows platforms, this always returns `true`. ### ddCommand(filename) -Creates a 10Mb file of all null characters. +Creates a 10 MB file of all null characters. ### disableCrashOnUnhandledRejection() From 74bc7df84922a5bfd9f6d90780fd69c39abc58b7 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 13:02:51 -0700 Subject: [PATCH 3/6] fixup! fixup! test: refactor common.ddCommand() --- test/parallel/test-fs-error-messages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 4ca9fb2ef9c21c..b0480059edd37f 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -32,7 +32,7 @@ tmpdir.refresh(); const nonexistentFile = fixtures.path('non-existent'); const nonexistentDir = fixtures.path('non-existent', 'foo', 'bar'); const existingFile = fixtures.path('exit.js'); -const existingFile2 = fixtures.path('create-file.js'); +const existingFile2 = fixtures.path('a.js'); const existingDir = tmpdir.path; const existingDir2 = fixtures.path('keys'); const { COPYFILE_EXCL } = fs.constants; From d3d2bab5aafa413a9df6bbcfee30ae222406f58f Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 13:06:11 -0700 Subject: [PATCH 4/6] test: rename common.ddCommand() Change common.ddCommand() to common.createZeroFilledFile(). --- test/common/README.md | 2 +- test/common/index.js | 4 ++-- test/common/index.mjs | 4 ++-- test/parallel/test-http-chunk-problem.js | 2 +- test/parallel/test-pipe-file-to-http.js | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 4d914f4f89e5ad..e63c83fd806dfb 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -52,7 +52,7 @@ symlinks ([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716(v=vs.85).aspx)). On non-Windows platforms, this always returns `true`. -### ddCommand(filename) +### createZeroFilledFile(filename) Creates a 10 MB file of all null characters. diff --git a/test/common/index.js b/test/common/index.js index 828832b937b33b..de33141fcfb9fb 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -173,7 +173,7 @@ function childShouldThrowAndAbort() { }); } -function ddCommand(filename) { +function createZeroFilledFile(filename) { const fd = fs.openSync(filename, 'w'); fs.ftruncateSync(fd, 10240 * 1024); fs.closeSync(fd); @@ -697,7 +697,7 @@ module.exports = { busyLoop, canCreateSymLink, childShouldThrowAndAbort, - ddCommand, + createZeroFilledFile, disableCrashOnUnhandledRejection, enoughTestCpu, enoughTestMem, diff --git a/test/common/index.mjs b/test/common/index.mjs index 832f68a9ee0d3e..bb17d9d37171a6 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -22,7 +22,7 @@ const { PIPE, hasIPv6, childShouldThrowAndAbort, - ddCommand, + createZeroFilledFile, platformTimeout, allowGlobals, mustCall, @@ -71,7 +71,7 @@ export { PIPE, hasIPv6, childShouldThrowAndAbort, - ddCommand, + createZeroFilledFile, platformTimeout, allowGlobals, mustCall, diff --git a/test/parallel/test-http-chunk-problem.js b/test/parallel/test-http-chunk-problem.js index dcc3a9c3fc8eed..c2502a891d325c 100644 --- a/test/parallel/test-http-chunk-problem.js +++ b/test/parallel/test-http-chunk-problem.js @@ -63,7 +63,7 @@ function executeRequest(cb) { tmpdir.refresh(); -common.ddCommand(filename); +common.createZeroFilledFile(filename); server = http.createServer(function(req, res) { res.writeHead(200); diff --git a/test/parallel/test-pipe-file-to-http.js b/test/parallel/test-pipe-file-to-http.js index 70805873f1988e..3cadec7a27237e 100644 --- a/test/parallel/test-pipe-file-to-http.js +++ b/test/parallel/test-pipe-file-to-http.js @@ -56,7 +56,7 @@ const server = http.createServer(function(req, res) { server.listen(0); server.on('listening', function() { - common.ddCommand(filename); + common.createZeroFilledFile(filename); makeRequest(); }); From bf4f33cc7932a985c8294d936d7a5709dc470d66 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 13:08:56 -0700 Subject: [PATCH 5/6] fixup! fixup! fixup! test: refactor common.ddCommand() --- test/common/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/index.js b/test/common/index.js index de33141fcfb9fb..e76dfcce8b0b8f 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -175,7 +175,7 @@ function childShouldThrowAndAbort() { function createZeroFilledFile(filename) { const fd = fs.openSync(filename, 'w'); - fs.ftruncateSync(fd, 10240 * 1024); + fs.ftruncateSync(fd, 10 * 1024 * 1024); fs.closeSync(fd); } From c1f684dfa3e648eb0866f29d91e5655efdef0389 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Wed, 10 Oct 2018 16:34:05 -0400 Subject: [PATCH 6/6] fixup! in module loader --- test/sequential/test-module-loading.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index 42916a7903c344..0e8aafe51b744f 100644 --- a/test/sequential/test-module-loading.js +++ b/test/sequential/test-module-loading.js @@ -251,7 +251,6 @@ try { assert.deepStrictEqual(children, { 'common/index.js': { - 'common/fixtures.js': {}, 'common/tmpdir.js': {} }, 'fixtures/not-main-module.js': {},