Skip to content

Commit

Permalink
test: add and use tmpdir.hasEnoughSpace()
Browse files Browse the repository at this point in the history
In general, we assume that the tmpdir will provide sufficient space for
most tests. Some tests, however, require hundreds of megabytes or even
gigabytes of space, which often causes them to fail, especially on our
macOS infrastructure. The most recent reliability report contains more
than 20 related CI failures.

This change adds a new function hasEnoughSpace() to the tmpdir module
that uses statfsSync() to guess whether allocating a certain amount of
space within the temporary directory will succeed.

This change also updates the most frequently failing tests to use the
new function such that the relevant parts of the tests are skipped if
tmpdir has insufficient space.

Refs: nodejs/reliability#549
PR-URL: nodejs#47767
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Richard Lau <rlau@redhat.com>
  • Loading branch information
tniessen authored and MoLow committed Jul 6, 2023
1 parent 72443bc commit 11a2d1c
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 22 deletions.
9 changes: 9 additions & 0 deletions test/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,15 @@ Avoid calling it more than once in an asynchronous context as one call
might refresh the temporary directory of a different context, causing
the test to fail somewhat mysteriously.

### `hasEnoughSpace(size)`

* `size` [\<number>][<number>] Required size, in bytes.

Returns `true` if the available blocks of the file system underlying `path`
are likely sufficient to hold a single file of `size` bytes. This is useful for
skipping tests that require hundreds of megabytes or even gigabytes of temporary
files, but it is inaccurate and susceptible to race conditions.

## UDP pair helper

The `common/udppair` module exports a function `makeUDPPair` and a class
Expand Down
6 changes: 6 additions & 0 deletions test/common/tmpdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,13 @@ function onexit() {
}
}

function hasEnoughSpace(size) {
const { bavail, bsize } = fs.statfsSync(tmpPath);
return bavail >= Math.ceil(size / bsize);
}

module.exports = {
path: tmpPath,
refresh,
hasEnoughSpace,
};
27 changes: 16 additions & 11 deletions test/parallel/test-fs-promises-file-handle-readFile.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,17 +106,22 @@ async function doReadAndCancel() {
// Variable taken from https://github.com/nodejs/node/blob/1377163f3351/lib/internal/fs/promises.js#L5
const kIoMaxLength = 2 ** 31 - 1;

const newFile = path.resolve(tmpDir, 'dogs-running3.txt');
await writeFile(newFile, Buffer.from('0'));
await truncate(newFile, kIoMaxLength + 1);

const fileHandle = await open(newFile, 'r');

await assert.rejects(fileHandle.readFile(), {
name: 'RangeError',
code: 'ERR_FS_FILE_TOO_LARGE'
});
await fileHandle.close();
if (!tmpdir.hasEnoughSpace(kIoMaxLength)) {
// truncate() will fail with ENOSPC if there is not enough space.
common.printSkipMessage(`Not enough space in ${tmpDir}`);
} else {
const newFile = path.resolve(tmpDir, 'dogs-running3.txt');
await writeFile(newFile, Buffer.from('0'));
await truncate(newFile, kIoMaxLength + 1);

const fileHandle = await open(newFile, 'r');

await assert.rejects(fileHandle.readFile(), {
name: 'RangeError',
code: 'ERR_FS_FILE_TOO_LARGE'
});
await fileHandle.close();
}
}
}

Expand Down
28 changes: 17 additions & 11 deletions test/parallel/test-fs-readfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,27 @@ for (const e of fileInfo) {
assert.deepStrictEqual(buf, e.contents);
}));
}
// Test readFile size too large

// readFile() and readFileSync() should fail if the file is too big.
{
const kIoMaxLength = 2 ** 31 - 1;

const file = path.join(tmpdir.path, `${prefix}-too-large.txt`);
fs.writeFileSync(file, Buffer.from('0'));
fs.truncateSync(file, kIoMaxLength + 1);
if (!tmpdir.hasEnoughSpace(kIoMaxLength)) {
// truncateSync() will fail with ENOSPC if there is not enough space.
common.printSkipMessage(`Not enough space in ${tmpdir.path}`);
} else {
const file = path.join(tmpdir.path, `${prefix}-too-large.txt`);
fs.writeFileSync(file, Buffer.from('0'));
fs.truncateSync(file, kIoMaxLength + 1);

fs.readFile(file, common.expectsError({
code: 'ERR_FS_FILE_TOO_LARGE',
name: 'RangeError',
}));
assert.throws(() => {
fs.readFileSync(file);
}, { code: 'ERR_FS_FILE_TOO_LARGE', name: 'RangeError' });
fs.readFile(file, common.expectsError({
code: 'ERR_FS_FILE_TOO_LARGE',
name: 'RangeError',
}));
assert.throws(() => {
fs.readFileSync(file);
}, { code: 'ERR_FS_FILE_TOO_LARGE', name: 'RangeError' });
}
}

{
Expand Down
4 changes: 4 additions & 0 deletions test/pummel/test-fs-readfile-tostring-fail.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ if (common.isAIX && (Number(cp.execSync('ulimit -f')) * 512) < kStringMaxLength)
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();

if (!tmpdir.hasEnoughSpace(kStringMaxLength)) {
common.skip(`Not enough space in ${tmpdir.path}`);
}

const file = path.join(tmpdir.path, 'toobig.txt');
const stream = fs.createWriteStream(file, {
flags: 'a',
Expand Down

0 comments on commit 11a2d1c

Please sign in to comment.