From 1a810b1922dc9a14fabb169f829969310fd43cb0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 10 Oct 2018 11:24:51 -0700 Subject: [PATCH] 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. Backport-PR-URL: https://github.com/nodejs/node/pull/23630 PR-URL: https://github.com/nodejs/node/pull/23411 Reviewed-By: Refael Ackermann Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Sakthipriyan Vairamani Reviewed-By: James M Snell Reviewed-By: George Adams --- test/common/README.md | 5 ++- test/common/index.js | 12 +++---- test/fixtures/create-file.js | 29 ---------------- test/parallel/test-fs-error-messages.js | 2 +- test/parallel/test-http-chunk-problem.js | 43 +++++++++++------------- test/parallel/test-pipe-file-to-http.js | 9 ++--- test/sequential/test-module-loading.js | 1 - 7 files changed, 29 insertions(+), 72 deletions(-) delete mode 100644 test/fixtures/create-file.js diff --git a/test/common/README.md b/test/common/README.md index c3f2c5f6b8a7f1..4d914f4f89e5ad 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 10 MB file of all null characters. ### disableCrashOnUnhandledRejection() diff --git a/test/common/index.js b/test/common/index.js index f3f1bbf2f4607e..cc1548a22a9e1a 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -29,7 +29,6 @@ const os = require('os'); const { exec, execSync, spawnSync } = require('child_process'); const util = require('util'); const Timer = process.binding('timer_wrap').Timer; -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, 10 * 1024 * 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-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 8ef009e8128b0d..e0c8e4b584f5b5 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -31,7 +31,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; 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() { 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': {},