From 97a3d39b8f079de39cc1e27fa9c1f7239d6b2a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 1 May 2023 17:23:49 +0200 Subject: [PATCH] test: add and use tmpdir.hasEnoughSpace() 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: https://github.com/nodejs/reliability/issues/549 PR-URL: https://github.com/nodejs/node/pull/47767 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Yagiz Nizipli Reviewed-By: Moshe Atlow Reviewed-By: Richard Lau --- test/common/README.md | 9 ++++++ test/common/tmpdir.js | 6 ++++ .../test-fs-promises-file-handle-readFile.js | 27 ++++++++++-------- test/parallel/test-fs-readfile.js | 28 +++++++++++-------- test/pummel/test-fs-readfile-tostring-fail.js | 4 +++ 5 files changed, 52 insertions(+), 22 deletions(-) diff --git a/test/common/README.md b/test/common/README.md index 3587dcaec86e42..8e89e473f2aed5 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -1030,6 +1030,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` [\][] 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 diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index 3c4ca546d062d3..60e5c7919f0f32 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -69,7 +69,13 @@ function onexit(useSpawn) { } } +function hasEnoughSpace(size) { + const { bavail, bsize } = fs.statfsSync(tmpPath); + return bavail >= Math.ceil(size / bsize); +} + module.exports = { path: tmpPath, refresh, + hasEnoughSpace, }; diff --git a/test/parallel/test-fs-promises-file-handle-readFile.js b/test/parallel/test-fs-promises-file-handle-readFile.js index 4cc2e59bb52780..3c6815973cb54b 100644 --- a/test/parallel/test-fs-promises-file-handle-readFile.js +++ b/test/parallel/test-fs-promises-file-handle-readFile.js @@ -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(); + } } } diff --git a/test/parallel/test-fs-readfile.js b/test/parallel/test-fs-readfile.js index c0dd16255b44fd..1a7ddf45bebf64 100644 --- a/test/parallel/test-fs-readfile.js +++ b/test/parallel/test-fs-readfile.js @@ -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' }); + } } { diff --git a/test/pummel/test-fs-readfile-tostring-fail.js b/test/pummel/test-fs-readfile-tostring-fail.js index 0b594520d21aea..8428f1f15a0c22 100644 --- a/test/pummel/test-fs-readfile-tostring-fail.js +++ b/test/pummel/test-fs-readfile-tostring-fail.js @@ -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',