Skip to content

test: reduce flakiness of test-fs-read-position-validation.mjs #42999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nodejs-github-bot merged 9 commits into from
May 15, 2022
Merged

test: reduce flakiness of test-fs-read-position-validation.mjs #42999

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fe48cc5
test: reduce flakiness of `test-fs-read-position-validation.mjs`
LiviaMedeiros May 6, 2022
21a0db5
squash: adjust promisification
LiviaMedeiros May 7, 2022
e2c07c9
squash: fix typo
LiviaMedeiros May 7, 2022
d9a88ba
squash: the actual fix: do not close file too early
LiviaMedeiros May 7, 2022
90e5cd7
squash: refactor callbacks, add test for buffer[, options]
LiviaMedeiros May 7, 2022
45b3626
squash: refactor promisification
LiviaMedeiros May 7, 2022
c85e84d
squash: add explicit async
LiviaMedeiros May 7, 2022
f114cbb
squash: reject on errors
LiviaMedeiros May 7, 2022
11a5b35
squash: fix race condition on closing fd
LiviaMedeiros May 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test: reduce flakiness of test-fs-read-position-validation.mjs
  • Loading branch information
@LiviaMedeiros
LiviaMedeiros committed May 15, 2022
commit fe48cc5321ce29366a9061369fe42d3f3b83cea4
101 changes: 31 additions & 70 deletions test/parallel/test-fs-read-position-validation.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as common from '../common/index.mjs';
import '../common/index.mjs';
import * as fixtures from '../common/fixtures.mjs';
import fs from 'fs';
import assert from 'assert';
import util from 'util';

// This test ensures that "position" argument is correctly validated

Expand All @@ -11,90 +12,50 @@ const buffer = Buffer.from('xyz\n');
const offset = 0;
const length = buffer.byteLength;

const close = util.promisify(fs.close);
const open = util.promisify(fs.open);
const read = util.promisify(fs.read);

// 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;
let fd;
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);
fd = await open(filepath, 'r');
if (allowedErrors.length) {
await read(fd, buffer, offset, length, position).catch((err) => {
if (!allowedErrors.includes(err.code)) {
assert.fail(err);
}
});
await read(fd, { buffer, offset, length, position }).catch((err) => {
if (!allowedErrors.includes(err.code)) {
assert.fail(err);
}
});
} else {
await read(fd, buffer, offset, length, position);
await read(fd, { buffer, offset, length, position });
}
} finally {
if (fdSync) fs.closeSync(fdSync);
if (fd) await close(fd);
}

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;
async function testInvalid(code, position) {
let fd;
try {
fdSync = fs.openSync(filepath, 'r');
assert.throws(
() => fs.readSync(fdSync, buffer, offset, length, position),
fd = await open(filepath, 'r');
await assert.rejects(
read(fd, buffer, offset, length, position),
{ code }
);
assert.throws(
() => fs.readSync(fdSync, buffer, { offset, length, position }),
await assert.rejects(
read(fd, { 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) {
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());
}
}));
} else {
fs.open(filepath, 'r', common.mustSucceed((fd) => {
try {
assert.throws(
() => fs.read(fd, buffer, offset, length, position, common.mustNotCall()),
{ code }
);
assert.throws(
() => fs.read(fd, { buffer, offset, length, position }, common.mustNotCall()),
{ code }
);
} finally {
fs.close(fd, common.mustSucceed());
}
}));
if (fd) await close(fd);
}
}

Expand Down
80 changes: 80 additions & 0 deletions test/parallel/test-fs-readSync-position-validation.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
}
}