From 615129f20ff7983931d327f833814458a1c5a08a Mon Sep 17 00:00:00 2001 From: Livia Medeiros <74449973+LiviaMedeiros@users.noreply.github.com> Date: Sun, 15 May 2022 22:52:06 +0800 Subject: [PATCH] test: reduce flakiness of `test-fs-read-position-validation.mjs` PR-URL: https://github.com/nodejs/node/pull/42999 Reviewed-By: Antoine du Hamel --- .../test-fs-read-position-validation.mjs | 93 ++++++------------- .../test-fs-readSync-position-validation.mjs | 80 ++++++++++++++++ 2 files changed, 107 insertions(+), 66 deletions(-) create mode 100644 test/parallel/test-fs-readSync-position-validation.mjs diff --git a/test/parallel/test-fs-read-position-validation.mjs b/test/parallel/test-fs-read-position-validation.mjs index bbd4e9ab4176fe..919a8dd1419903 100644 --- a/test/parallel/test-fs-read-position-validation.mjs +++ b/test/parallel/test-fs-read-position-validation.mjs @@ -14,73 +14,27 @@ const length = buffer.byteLength; // allowedErrors is an array of acceptable internal errors // For example, on some platforms read syscall might return -EFBIG async function testValid(position, allowedErrors = []) { - let fdSync; - try { - fdSync = fs.openSync(filepath, 'r'); - fs.readSync(fdSync, buffer, offset, length, position); - fs.readSync(fdSync, buffer, { offset, length, position }); - } catch (err) { - if (!allowedErrors.includes(err.code)) { - assert.fail(err); - } - } finally { - if (fdSync) fs.closeSync(fdSync); - } - - fs.open(filepath, 'r', common.mustSucceed((fd) => { - try { - if (allowedErrors.length) { - fs.read(fd, buffer, offset, length, position, common.mustCall((err, ...results) => { - if (err && !allowedErrors.includes(err.code)) { - assert.fail(err); - } - })); - fs.read(fd, { buffer, offset, length, position }, common.mustCall((err, ...results) => { - if (err && !allowedErrors.includes(err.code)) { - assert.fail(err); - } - })); - } else { - fs.read(fd, buffer, offset, length, position, common.mustSucceed()); - fs.read(fd, { buffer, offset, length, position }, common.mustSucceed()); - } - } finally { - fs.close(fd, common.mustSucceed()); - } - })); -} - -async function testInvalid(code, position, internalCatch = false) { - let fdSync; - try { - fdSync = fs.openSync(filepath, 'r'); - assert.throws( - () => fs.readSync(fdSync, buffer, offset, length, position), - { code } - ); - assert.throws( - () => fs.readSync(fdSync, buffer, { offset, length, position }), - { code } - ); - } finally { - if (fdSync) fs.closeSync(fdSync); - } - - // Set this flag for catching errors via first argument of callback function - if (internalCatch) { + return new Promise((resolve, reject) => { fs.open(filepath, 'r', common.mustSucceed((fd) => { - try { - fs.read(fd, buffer, offset, length, position, (err, ...results) => { - assert.strictEqual(err.code, code); - }); - fs.read(fd, { buffer, offset, length, position }, (err, ...results) => { - assert.strictEqual(err.code, code); - }); - } finally { - fs.close(fd, common.mustSucceed()); - } + let callCount = 3; + const handler = common.mustCall((err) => { + callCount--; + if (err && !allowedErrors.includes(err.code)) { + fs.close(fd, common.mustSucceed()); + reject(err); + } else if (callCount === 0) { + fs.close(fd, common.mustSucceed(resolve)); + } + }, callCount); + fs.read(fd, buffer, offset, length, position, handler); + fs.read(fd, { buffer, offset, length, position }, handler); + fs.read(fd, buffer, { offset, length, position }, handler); })); - } else { + }); +} + +async function testInvalid(code, position) { + return new Promise((resolve, reject) => { fs.open(filepath, 'r', common.mustSucceed((fd) => { try { assert.throws( @@ -91,11 +45,18 @@ async function testInvalid(code, position, internalCatch = false) { () => fs.read(fd, { buffer, offset, length, position }, common.mustNotCall()), { code } ); + assert.throws( + () => fs.read(fd, buffer, { offset, length, position }, common.mustNotCall()), + { code } + ); + resolve(); + } catch (err) { + reject(err); } finally { fs.close(fd, common.mustSucceed()); } })); - } + }); } { diff --git a/test/parallel/test-fs-readSync-position-validation.mjs b/test/parallel/test-fs-readSync-position-validation.mjs new file mode 100644 index 00000000000000..5cf40ba1b08578 --- /dev/null +++ b/test/parallel/test-fs-readSync-position-validation.mjs @@ -0,0 +1,80 @@ +import '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import fs from 'fs'; +import assert from 'assert'; + +// This test ensures that "position" argument is correctly validated + +const filepath = fixtures.path('x.txt'); + +const buffer = Buffer.from('xyz\n'); +const offset = 0; +const length = buffer.byteLength; + +// allowedErrors is an array of acceptable internal errors +// For example, on some platforms read syscall might return -EFBIG +function testValid(position, allowedErrors = []) { + let fdSync; + try { + fdSync = fs.openSync(filepath, 'r'); + fs.readSync(fdSync, buffer, offset, length, position); + fs.readSync(fdSync, buffer, { offset, length, position }); + } catch (err) { + if (!allowedErrors.includes(err.code)) { + assert.fail(err); + } + } finally { + if (fdSync) fs.closeSync(fdSync); + } +} + +function testInvalid(code, position, internalCatch = false) { + let fdSync; + try { + fdSync = fs.openSync(filepath, 'r'); + assert.throws( + () => fs.readSync(fdSync, buffer, offset, length, position), + { code } + ); + assert.throws( + () => fs.readSync(fdSync, buffer, { offset, length, position }), + { code } + ); + } finally { + if (fdSync) fs.closeSync(fdSync); + } +} + +{ + testValid(undefined); + testValid(null); + testValid(-1); + testValid(-1n); + + testValid(0); + testValid(0n); + testValid(1); + testValid(1n); + testValid(9); + testValid(9n); + testValid(Number.MAX_SAFE_INTEGER, [ 'EFBIG' ]); + + testValid(2n ** 63n - 1n - BigInt(length), [ 'EFBIG' ]); + testInvalid('ERR_OUT_OF_RANGE', 2n ** 63n); + + // TODO(LiviaMedeiros): test `2n ** 63n - BigInt(length)` + + testInvalid('ERR_OUT_OF_RANGE', NaN); + testInvalid('ERR_OUT_OF_RANGE', -Infinity); + testInvalid('ERR_OUT_OF_RANGE', Infinity); + testInvalid('ERR_OUT_OF_RANGE', -0.999); + testInvalid('ERR_OUT_OF_RANGE', -(2n ** 64n)); + testInvalid('ERR_OUT_OF_RANGE', Number.MAX_SAFE_INTEGER + 1); + testInvalid('ERR_OUT_OF_RANGE', Number.MAX_VALUE); + + for (const badTypeValue of [ + false, true, '1', Symbol(1), {}, [], () => {}, Promise.resolve(1), + ]) { + testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue); + } +}