From 8d9bbe49565fa73c1fbfb205638590c552cb06a2 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 25 Aug 2022 02:16:56 +0800 Subject: [PATCH] test: avoid race in file write stream handle tests 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: https://github.com/nodejs/node/pull/44380 Refs: https://github.com/nodejs/reliability/issues/354 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Evan Lucas --- .../test-fs-write-stream-file-handle-2.js | 33 +++++++++++++++++++ .../test-fs-write-stream-file-handle.js | 23 ------------- 2 files changed, 33 insertions(+), 23 deletions(-) create mode 100644 test/parallel/test-fs-write-stream-file-handle-2.js diff --git a/test/parallel/test-fs-write-stream-file-handle-2.js b/test/parallel/test-fs-write-stream-file-handle-2.js new file mode 100644 index 00000000000..25f68b14da7 --- /dev/null +++ b/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()); diff --git a/test/parallel/test-fs-write-stream-file-handle.js b/test/parallel/test-fs-write-stream-file-handle.js index 23ddf21c50f..01c490ef5d0 100644 --- a/test/parallel/test-fs-write-stream-file-handle.js +++ b/test/parallel/test-fs-write-stream-file-handle.js @@ -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());