Skip to content

Commit

Permalink
test: avoid race in file write stream handle tests
Browse files Browse the repository at this point in the history
The test previously created two fs.promises.open calls on the
same file with w+ back-to-back, and one of them could fail
when checking the contents of that file if the other happened
to be opening the file for write. Split them into different
tests (with different tmpdir) to avoid the race.

PR-URL: nodejs#44380
Refs: nodejs/reliability#354
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
  • Loading branch information
joyeecheung committed Sep 1, 2022
1 parent 27e3202 commit 4189825
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 23 deletions.
33 changes: 33 additions & 0 deletions test/parallel/test-fs-write-stream-file-handle-2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';
const common = require('../common');
const fs = require('fs');
const path = require('path');
const assert = require('assert');
const tmpdir = require('../common/tmpdir');
const file = path.join(tmpdir.path, 'write_stream_filehandle_test.txt');
const input = 'hello world';

tmpdir.refresh();

fs.promises.open(file, 'w+').then((handle) => {
let calls = 0;
const {
write: originalWriteFunction,
writev: originalWritevFunction
} = handle;
handle.write = function write() {
calls++;
return Reflect.apply(originalWriteFunction, this, arguments);
};
handle.writev = function writev() {
calls++;
return Reflect.apply(originalWritevFunction, this, arguments);
};
const stream = fs.createWriteStream(null, { fd: handle });

stream.end(input);
stream.on('close', common.mustCall(() => {
assert(calls > 0, 'expected at least one call to fileHandle.write or ' +
'fileHandle.writev, got 0');
}));
}).then(common.mustCall());
23 changes: 0 additions & 23 deletions test/parallel/test-fs-write-stream-file-handle.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,3 @@ fs.promises.open(file, 'w+').then((handle) => {
assert.strictEqual(output, input);
}));
}).then(common.mustCall());

fs.promises.open(file, 'w+').then((handle) => {
let calls = 0;
const {
write: originalWriteFunction,
writev: originalWritevFunction
} = handle;
handle.write = function write() {
calls++;
return Reflect.apply(originalWriteFunction, this, arguments);
};
handle.writev = function writev() {
calls++;
return Reflect.apply(originalWritevFunction, this, arguments);
};
const stream = fs.createWriteStream(null, { fd: handle });

stream.end(input);
stream.on('close', common.mustCall(() => {
assert(calls > 0, 'expected at least one call to fileHandle.write or ' +
'fileHandle.writev, got 0');
}));
}).then(common.mustCall());

0 comments on commit 4189825

Please sign in to comment.