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: #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 authored and RafaelGSS committed Sep 7, 2022
1 parent c5413a1 commit 4c33e5d
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
@@ -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
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 4c33e5d

Please sign in to comment.