Skip to content

Commit

Permalink
test: refactor common.ddCommand()
Browse files Browse the repository at this point in the history
* 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 <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: George Adams <george.adams@uk.ibm.com>
  • Loading branch information
Trott authored and jasnell committed Oct 17, 2018
1 parent 9d6a1d6 commit 8561462
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 72 deletions.
5 changes: 2 additions & 3 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 [&lt;Object>]
### ddCommand(filename)

Platform normalizes the `dd` command
Creates a 10 MB file of all null characters.

### disableCrashOnUnhandledRejection()

Expand Down
12 changes: 4 additions & 8 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}


Expand Down
29 changes: 0 additions & 29 deletions test/fixtures/create-file.js

This file was deleted.

2 changes: 1 addition & 1 deletion test/parallel/test-fs-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
43 changes: 20 additions & 23 deletions test/parallel/test-http-chunk-problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
9 changes: 2 additions & 7 deletions test/parallel/test-pipe-file-to-http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion test/sequential/test-module-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ try {

assert.deepStrictEqual(children, {
'common/index.js': {
'common/fixtures.js': {},
'common/tmpdir.js': {}
},
'fixtures/not-main-module.js': {},
Expand Down

0 comments on commit 8561462

Please sign in to comment.