From 8186ba411902defccde0524eec2e38da8dc9ff83 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Tue, 25 May 2021 20:42:54 +0530 Subject: [PATCH 01/20] fs: stop fixing EPERM on Windows Originally, when rimraf faced an EPERM on Windows, it would try to change the file permissions of the entry and attempt to remove it again. However, rm -rf doesn't change any file permissions, so rimraf shouldn't either. This change also updates test/common/tmpdir.js to repeatedly try changing the file permissions of its contents to 0o777 when an EACCES or an EPERM is faced while attempting to remove them. Signed-off-by: Darshan Sen --- lib/internal/fs/rimraf.js | 83 ++------------- test/common/tmpdir.js | 79 +++++++++++--- test/parallel/test-fs-rm.js | 205 ++++++++++++++++++++++++++++-------- 3 files changed, 228 insertions(+), 139 deletions(-) diff --git a/lib/internal/fs/rimraf.js b/lib/internal/fs/rimraf.js index 84f32dc6bb99db..e886973f799099 100644 --- a/lib/internal/fs/rimraf.js +++ b/lib/internal/fs/rimraf.js @@ -1,9 +1,4 @@ -// This file is a modified version of the rimraf module on npm. It has been -// modified in the following ways: -// - Use of the assert module has been replaced with core's error system. -// - All code related to the glob dependency has been removed. -// - Bring your own custom fs module is not currently supported. -// - Some basic code cleanup. +// This file is a modified version of the rimraf module on npm. 'use strict'; const { @@ -15,16 +10,12 @@ const { const { Buffer } = require('buffer'); const fs = require('fs'); const { - chmod, - chmodSync, lstat, lstatSync, readdir, readdirSync, rmdir, rmdirSync, - stat, - statSync, unlink, unlinkSync } = fs; @@ -34,9 +25,6 @@ const { sleep } = require('internal/util'); const notEmptyErrorCodes = new SafeSet(['ENOTEMPTY', 'EEXIST', 'EPERM']); const retryErrorCodes = new SafeSet( ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']); -const isWindows = process.platform === 'win32'; -const epermHandler = isWindows ? fixWinEPERM : _rmdir; -const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync; const readdirEncoding = 'buffer'; const separator = Buffer.from(sep); @@ -69,10 +57,6 @@ function _rimraf(path, options, callback) { if (err) { if (err.code === 'ENOENT') return callback(null); - - // Windows can EPERM on stat. - if (isWindows && err.code === 'EPERM') - return fixWinEPERM(path, options, err, callback); } else if (stats.isDirectory()) { return _rmdir(path, options, err, callback); } @@ -81,11 +65,8 @@ function _rimraf(path, options, callback) { if (err) { if (err.code === 'ENOENT') return callback(null); - if (err.code === 'EISDIR') + if (err.code === 'EISDIR' || err.code === 'EPERM') return _rmdir(path, options, err, callback); - if (err.code === 'EPERM') { - return epermHandler(path, options, err, callback); - } } return callback(err); @@ -94,24 +75,6 @@ function _rimraf(path, options, callback) { } -function fixWinEPERM(path, options, originalErr, callback) { - chmod(path, 0o666, (err) => { - if (err) - return callback(err.code === 'ENOENT' ? null : originalErr); - - stat(path, (err, stats) => { - if (err) - return callback(err.code === 'ENOENT' ? null : originalErr); - - if (stats.isDirectory()) - _rmdir(path, options, originalErr, callback); - else - unlink(path, callback); - }); - }); -} - - function _rmdir(path, options, originalErr, callback) { rmdir(path, (err) => { if (err) { @@ -181,10 +144,6 @@ function rimrafSync(path, options) { } catch (err) { if (err.code === 'ENOENT') return; - - // Windows can EPERM on stat. - if (isWindows && err.code === 'EPERM') - fixWinEPERMSync(path, options, err); } try { @@ -194,14 +153,11 @@ function rimrafSync(path, options) { else _unlinkSync(path, options); } catch (err) { - if (err.code === 'ENOENT') - return; - if (err.code === 'EPERM') - return epermHandlerSync(path, options, err); - if (err.code !== 'EISDIR') - throw err; + if (err.code === 'EISDIR' || err.code === 'EPERM') + return _rmdirSync(path, options, err); - _rmdirSync(path, options, err); + if (err.code !== 'ENOENT') + throw err; } } @@ -280,31 +236,4 @@ function _rmdirSync(path, options, originalErr) { } -function fixWinEPERMSync(path, options, originalErr) { - try { - chmodSync(path, 0o666); - } catch (err) { - if (err.code === 'ENOENT') - return; - - throw originalErr; - } - - let stats; - - try { - stats = statSync(path, { throwIfNoEntry: false }); - } catch { - throw originalErr; - } - - if (stats === undefined) return; - - if (stats.isDirectory()) - _rmdirSync(path, options, originalErr); - else - _unlinkSync(path, options); -} - - module.exports = { rimraf, rimrafPromises, rimrafSync }; diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index d911feb460a2f3..177f543c98056a 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -6,7 +6,45 @@ const path = require('path'); const { isMainThread } = require('worker_threads'); function rmSync(pathname) { - fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true }); + let err = null; + const maxRetries = 10; + + for (let retryNumber = 0; retryNumber < maxRetries; ++retryNumber) { + err = null; + + try { + fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true }); + } catch (_err) { + err = _err; + } + + if (err === null) { + return; + } + + const errPath = err.path; + const errCode = err.code; + + if (errCode === 'EACCES' || errCode === 'EPERM') { + const surroundingDir = path.join(errPath, '..'); + + try { + fs.chmodSync(surroundingDir, 0o777); + } catch { + } + + try { + fs.chmodSync(errPath, 0o777); + } catch { + } + } + } + + if (err === null) { + return; + } + + throw err; } const testRoot = process.env.NODE_TEST_DIR ? @@ -31,6 +69,28 @@ function refresh() { } } +function logErrorInfo(err) { + console.error('Can\'t clean tmpdir:', tmpPath); + + const files = fs.readdirSync(tmpPath); + console.error('Files blocking:', files); + + if (files.some((f) => f.startsWith('.nfs'))) { + // Warn about NFS "silly rename" + console.error('Note: ".nfs*" might be files that were open and ' + + 'unlinked but not closed.'); + console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); + } + + console.error(); + + // Additionally logging err, just in case throwing err gets overshadowed by + // an error from a test failure. + console.error(err); + + throw err; +} + function onexit() { // Change directory to avoid possible EBUSY if (isMainThread) @@ -38,21 +98,8 @@ function onexit() { try { rmSync(tmpPath); - } catch (e) { - console.error('Can\'t clean tmpdir:', tmpPath); - - const files = fs.readdirSync(tmpPath); - console.error('Files blocking:', files); - - if (files.some((f) => f.startsWith('.nfs'))) { - // Warn about NFS "silly rename" - console.error('Note: ".nfs*" might be files that were open and ' + - 'unlinked but not closed.'); - console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); - } - - console.error(); - throw e; + } catch (err) { + logErrorInfo(err); } } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 3fdfc8426248ac..4da5751c40ad67 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -5,7 +5,6 @@ const tmpdir = require('../common/tmpdir'); const assert = require('assert'); const fs = require('fs'); const path = require('path'); -const { execSync } = require('child_process'); const { validateRmOptionsSync } = require('internal/fs/utils'); @@ -287,24 +286,51 @@ function removeAsync(dir) { // IBMi has a different access permission mechanism // This test should not be run as `root` if (!common.isIBMi && (common.isWindows || process.getuid() !== 0)) { - function makeDirectoryReadOnly(dir, mode) { - let accessErrorCode = 'EACCES'; - if (common.isWindows) { - accessErrorCode = 'EPERM'; - execSync(`icacls ${dir} /deny "everyone:(OI)(CI)(DE,DC)"`); - } else { - fs.chmodSync(dir, mode); + // On Windows, we are allowed to access and modify the contents of a + // read-only folder. + { + // Check that deleting a file that cannot be accessed using rmsync throws + // https://github.com/nodejs/node/issues/38683 + const dirname = nextDirPath(); + const filePath = path.join(dirname, 'text.txt'); + + fs.mkdirSync(dirname, { recursive: true }); + fs.writeFileSync(filePath, 'hello'); + + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); + + let err = null; + + try { + fs.rmSync(filePath, { force: true }); + } catch (_err) { + err = _err; } - return accessErrorCode; - } - function makeDirectoryWritable(dir) { - if (fs.existsSync(dir)) { - if (common.isWindows) { - execSync(`icacls ${dir} /remove:d "everyone"`); - } else { - fs.chmodSync(dir, 0o777); - } + try { + fs.chmodSync(dirname, 0o777); + } catch { + } + + try { + fs.chmodSync(filePath, 0o777); + } catch { + } + + let isValidState = true; + const exists = fs.existsSync(filePath); + + if (common.isWindows && + !(exists === false && err === null)) { + isValidState = false; + } else if (!common.isWindows && + !(exists === true && err?.code === 'EACCES')) { + isValidState = false; + } + + if (!isValidState) { + throw err; } } @@ -313,18 +339,90 @@ function removeAsync(dir) { // https://github.com/nodejs/node/issues/38683 const dirname = nextDirPath(); const filePath = path.join(dirname, 'text.txt'); + + fs.mkdirSync(dirname, { recursive: true }); + fs.writeFileSync(filePath, 'hello'); + + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); + + fs.rm(filePath, { force: true }, common.mustCall((err) => { + try { + fs.chmodSync(dirname, 0o777); + } catch { + } + + try { + fs.chmodSync(filePath, 0o777); + } catch { + } + + let isValidState = true; + const exists = fs.existsSync(filePath); + + if (common.isWindows && + !(exists === false && err === null)) { + isValidState = false; + } else if (!common.isWindows && + !(exists === true && err?.code === 'EACCES')) { + isValidState = false; + } + + if (!isValidState) { + throw err; + } + })); + } + + // On Windows, we are not allowed to delete a read-only directory. + { + // Check endless recursion. + // https://github.com/nodejs/node/issues/34580 + const dirname = nextDirPath(); + fs.mkdirSync(dirname, { recursive: true }); + const root = fs.mkdtempSync(path.join(dirname, 'fs-')); + const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); + + fs.mkdirSync(middle); + fs.mkdirSync(leaf); + + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); + + let err = null; + try { - fs.mkdirSync(dirname, { recursive: true }); - fs.writeFileSync(filePath, 'hello'); - const code = makeDirectoryReadOnly(dirname, 0o444); - assert.throws(() => { - fs.rmSync(filePath, { force: true }); - }, { - code, - name: 'Error', - }); - } finally { - makeDirectoryWritable(dirname); + fs.rmSync(root, { recursive: true }); + } catch (_err) { + err = _err; + } + + try { + fs.chmodSync(middle, 0o777); + } catch { + } + + try { + fs.chmodSync(leaf, 0o777); + } catch { + } + + let isValidState = true; + const exists = fs.existsSync(root); + + if (common.isWindows && + !(exists === true && err?.code === 'EPERM')) { + // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: + // https://github.com/libuv/libuv/pull/3193 + isValidState = false; + } else if (!common.isWindows && + !(exists === true && err?.code === 'EACCES')) { + isValidState = false; + } + + if (!isValidState) { + throw err; } } @@ -335,27 +433,42 @@ function removeAsync(dir) { fs.mkdirSync(dirname, { recursive: true }); const root = fs.mkdtempSync(path.join(dirname, 'fs-')); const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); + fs.mkdirSync(middle); - fs.mkdirSync(path.join(middle, 'leaf')); // Make `middle` non-empty - try { - const code = makeDirectoryReadOnly(middle, 0o555); + fs.mkdirSync(leaf); + + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); + + fs.rm(root, { recursive: true }, common.mustCall((err) => { try { - assert.throws(() => { - fs.rmSync(root, { recursive: true }); - }, { - code, - name: 'Error', - }); - } catch (err) { - // Only fail the test if the folder was not deleted. - // as in some cases rmSync succesfully deletes read-only folders. - if (fs.existsSync(root)) { - throw err; - } + fs.chmodSync(middle, 0o777); + } catch { } - } finally { - makeDirectoryWritable(middle); - } + + try { + fs.chmodSync(leaf, 0o777); + } catch { + } + + let isValidState = true; + const exists = fs.existsSync(root); + + if (common.isWindows && + !(exists === true && err?.code === 'EPERM')) { + // TODO(RaisinTen): Replace the error code with 'EACCES' if this + // lands: https://github.com/libuv/libuv/pull/3193 + isValidState = false; + } else if (!common.isWindows && + !(exists === true && err?.code === 'EACCES')) { + isValidState = false; + } + + if (!isValidState) { + throw err; + } + })); } } } From abdadd37ce8e27aeee6c99e263d4033712055684 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:36:59 +0530 Subject: [PATCH 02/20] move checks into functions --- test/parallel/test-fs-rm.js | 86 +++++++++++-------------------------- 1 file changed, 26 insertions(+), 60 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 4da5751c40ad67..cfda9804489adb 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -289,8 +289,15 @@ function removeAsync(dir) { // On Windows, we are allowed to access and modify the contents of a // read-only folder. { - // Check that deleting a file that cannot be accessed using rmsync throws - // https://github.com/nodejs/node/issues/38683 + // Check that deleting a file that cannot be accessed using rmsync + // throws: https://github.com/nodejs/node/issues/38683 + function isValidState(exists, err) { + return common.isWindows ? + (exists === false && err === null)) : + (exists === true && err?.code === 'EACCES')); + } + + { const dirname = nextDirPath(); const filePath = path.join(dirname, 'text.txt'); @@ -318,25 +325,12 @@ function removeAsync(dir) { } catch { } - let isValidState = true; - const exists = fs.existsSync(filePath); - - if (common.isWindows && - !(exists === false && err === null)) { - isValidState = false; - } else if (!common.isWindows && - !(exists === true && err?.code === 'EACCES')) { - isValidState = false; - } - - if (!isValidState) { + if (!isValidState(fs.existsSync(filePath), err)) { throw err; } } { - // Check that deleting a file that cannot be accessed using rmsync throws - // https://github.com/nodejs/node/issues/38683 const dirname = nextDirPath(); const filePath = path.join(dirname, 'text.txt'); @@ -357,27 +351,26 @@ function removeAsync(dir) { } catch { } - let isValidState = true; - const exists = fs.existsSync(filePath); - - if (common.isWindows && - !(exists === false && err === null)) { - isValidState = false; - } else if (!common.isWindows && - !(exists === true && err?.code === 'EACCES')) { - isValidState = false; - } - - if (!isValidState) { + if (!isValidState(fs.existsSync(filePath), err)) { throw err; } })); } + } // On Windows, we are not allowed to delete a read-only directory. { - // Check endless recursion. - // https://github.com/nodejs/node/issues/34580 + // Check endless recursion. + // https://github.com/nodejs/node/issues/34580 + function isValidState(exists, err) { + // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: + // https://github.com/libuv/libuv/pull/3193 + return common.isWindows ? + (exists === true && err?.code === 'EPERM') : + (exists === true && err?.code === 'EACCES'); + } + + { const dirname = nextDirPath(); fs.mkdirSync(dirname, { recursive: true }); const root = fs.mkdtempSync(path.join(dirname, 'fs-')); @@ -408,27 +401,12 @@ function removeAsync(dir) { } catch { } - let isValidState = true; - const exists = fs.existsSync(root); - - if (common.isWindows && - !(exists === true && err?.code === 'EPERM')) { - // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: - // https://github.com/libuv/libuv/pull/3193 - isValidState = false; - } else if (!common.isWindows && - !(exists === true && err?.code === 'EACCES')) { - isValidState = false; - } - - if (!isValidState) { + if (!isValidState(fs.existsSync(root), err)) { throw err; } } { - // Check endless recursion. - // https://github.com/nodejs/node/issues/34580 const dirname = nextDirPath(); fs.mkdirSync(dirname, { recursive: true }); const root = fs.mkdtempSync(path.join(dirname, 'fs-')); @@ -452,23 +430,11 @@ function removeAsync(dir) { } catch { } - let isValidState = true; - const exists = fs.existsSync(root); - - if (common.isWindows && - !(exists === true && err?.code === 'EPERM')) { - // TODO(RaisinTen): Replace the error code with 'EACCES' if this - // lands: https://github.com/libuv/libuv/pull/3193 - isValidState = false; - } else if (!common.isWindows && - !(exists === true && err?.code === 'EACCES')) { - isValidState = false; - } - - if (!isValidState) { + if (!isValidState(fs.existsSync(root), err)) { throw err; } })); } + } } } From ea8e0a57dbe1bd80acf19810bdc25c3987918e56 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:39:11 +0530 Subject: [PATCH 03/20] reindent --- test/parallel/test-fs-rm.js | 196 ++++++++++++++++++------------------ 1 file changed, 98 insertions(+), 98 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index cfda9804489adb..07fc018aec0d8b 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -289,58 +289,32 @@ function removeAsync(dir) { // On Windows, we are allowed to access and modify the contents of a // read-only folder. { - // Check that deleting a file that cannot be accessed using rmsync - // throws: https://github.com/nodejs/node/issues/38683 - function isValidState(exists, err) { - return common.isWindows ? - (exists === false && err === null)) : - (exists === true && err?.code === 'EACCES')); - } - - { - const dirname = nextDirPath(); - const filePath = path.join(dirname, 'text.txt'); - - fs.mkdirSync(dirname, { recursive: true }); - fs.writeFileSync(filePath, 'hello'); - - fs.chmodSync(filePath, 0o444); - fs.chmodSync(dirname, 0o444); - - let err = null; - - try { - fs.rmSync(filePath, { force: true }); - } catch (_err) { - err = _err; + // Check that deleting a file that cannot be accessed using rmsync throws: + // https://github.com/nodejs/node/issues/38683 + function isValidState(exists, err) { + return common.isWindows ? + (exists === false && err === null)) : + (exists === true && err?.code === 'EACCES')); } - try { - fs.chmodSync(dirname, 0o777); - } catch { - } - - try { - fs.chmodSync(filePath, 0o777); - } catch { - } + { + const dirname = nextDirPath(); + const filePath = path.join(dirname, 'text.txt'); - if (!isValidState(fs.existsSync(filePath), err)) { - throw err; - } - } + fs.mkdirSync(dirname, { recursive: true }); + fs.writeFileSync(filePath, 'hello'); - { - const dirname = nextDirPath(); - const filePath = path.join(dirname, 'text.txt'); + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); - fs.mkdirSync(dirname, { recursive: true }); - fs.writeFileSync(filePath, 'hello'); + let err = null; - fs.chmodSync(filePath, 0o444); - fs.chmodSync(dirname, 0o444); + try { + fs.rmSync(filePath, { force: true }); + } catch (_err) { + err = _err; + } - fs.rm(filePath, { force: true }, common.mustCall((err) => { try { fs.chmodSync(dirname, 0o777); } catch { @@ -354,72 +328,69 @@ function removeAsync(dir) { if (!isValidState(fs.existsSync(filePath), err)) { throw err; } - })); - } - } + } - // On Windows, we are not allowed to delete a read-only directory. - { - // Check endless recursion. - // https://github.com/nodejs/node/issues/34580 - function isValidState(exists, err) { - // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: - // https://github.com/libuv/libuv/pull/3193 - return common.isWindows ? - (exists === true && err?.code === 'EPERM') : - (exists === true && err?.code === 'EACCES'); - } + { + const dirname = nextDirPath(); + const filePath = path.join(dirname, 'text.txt'); - { - const dirname = nextDirPath(); - fs.mkdirSync(dirname, { recursive: true }); - const root = fs.mkdtempSync(path.join(dirname, 'fs-')); - const middle = path.join(root, 'middle'); - const leaf = path.join(middle, 'leaf'); + fs.mkdirSync(dirname, { recursive: true }); + fs.writeFileSync(filePath, 'hello'); - fs.mkdirSync(middle); - fs.mkdirSync(leaf); + fs.chmodSync(filePath, 0o444); + fs.chmodSync(dirname, 0o444); - fs.chmodSync(leaf, 0o555); - fs.chmodSync(middle, 0o555); + fs.rm(filePath, { force: true }, common.mustCall((err) => { + try { + fs.chmodSync(dirname, 0o777); + } catch { + } - let err = null; + try { + fs.chmodSync(filePath, 0o777); + } catch { + } - try { - fs.rmSync(root, { recursive: true }); - } catch (_err) { - err = _err; + if (!isValidState(fs.existsSync(filePath), err)) { + throw err; + } + })); } + } - try { - fs.chmodSync(middle, 0o777); - } catch { + // On Windows, we are not allowed to delete a read-only directory. + { + // Check endless recursion. + // https://github.com/nodejs/node/issues/34580 + function isValidState(exists, err) { + // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: + // https://github.com/libuv/libuv/pull/3193 + return common.isWindows ? + (exists === true && err?.code === 'EPERM') : + (exists === true && err?.code === 'EACCES'); } - try { - fs.chmodSync(leaf, 0o777); - } catch { - } + { + const dirname = nextDirPath(); + fs.mkdirSync(dirname, { recursive: true }); + const root = fs.mkdtempSync(path.join(dirname, 'fs-')); + const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); - if (!isValidState(fs.existsSync(root), err)) { - throw err; - } - } + fs.mkdirSync(middle); + fs.mkdirSync(leaf); - { - const dirname = nextDirPath(); - fs.mkdirSync(dirname, { recursive: true }); - const root = fs.mkdtempSync(path.join(dirname, 'fs-')); - const middle = path.join(root, 'middle'); - const leaf = path.join(middle, 'leaf'); + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); - fs.mkdirSync(middle); - fs.mkdirSync(leaf); + let err = null; - fs.chmodSync(leaf, 0o555); - fs.chmodSync(middle, 0o555); + try { + fs.rmSync(root, { recursive: true }); + } catch (_err) { + err = _err; + } - fs.rm(root, { recursive: true }, common.mustCall((err) => { try { fs.chmodSync(middle, 0o777); } catch { @@ -433,8 +404,37 @@ function removeAsync(dir) { if (!isValidState(fs.existsSync(root), err)) { throw err; } - })); - } + } + + { + const dirname = nextDirPath(); + fs.mkdirSync(dirname, { recursive: true }); + const root = fs.mkdtempSync(path.join(dirname, 'fs-')); + const middle = path.join(root, 'middle'); + const leaf = path.join(middle, 'leaf'); + + fs.mkdirSync(middle); + fs.mkdirSync(leaf); + + fs.chmodSync(leaf, 0o555); + fs.chmodSync(middle, 0o555); + + fs.rm(root, { recursive: true }, common.mustCall((err) => { + try { + fs.chmodSync(middle, 0o777); + } catch { + } + + try { + fs.chmodSync(leaf, 0o777); + } catch { + } + + if (!isValidState(fs.existsSync(root), err)) { + throw err; + } + })); + } } } } From 9596fdc526b75cc81d25bf12908beda23f500249 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:45:19 +0530 Subject: [PATCH 04/20] update comments --- test/parallel/test-fs-rm.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 07fc018aec0d8b..c4142f0cebd3f0 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -286,12 +286,12 @@ function removeAsync(dir) { // IBMi has a different access permission mechanism // This test should not be run as `root` if (!common.isIBMi && (common.isWindows || process.getuid() !== 0)) { - // On Windows, we are allowed to access and modify the contents of a - // read-only folder. + // Check that deleting a file that cannot be accessed using rmsync throws: + // https://github.com/nodejs/node/issues/38683 { - // Check that deleting a file that cannot be accessed using rmsync throws: - // https://github.com/nodejs/node/issues/38683 function isValidState(exists, err) { + // On Windows, we are allowed to access and modify the contents of a + // read-only folder. return common.isWindows ? (exists === false && err === null)) : (exists === true && err?.code === 'EACCES')); @@ -358,12 +358,12 @@ function removeAsync(dir) { } } - // On Windows, we are not allowed to delete a read-only directory. + // Check endless recursion. + // https://github.com/nodejs/node/issues/34580 { - // Check endless recursion. - // https://github.com/nodejs/node/issues/34580 function isValidState(exists, err) { - // TODO(RaisinTen): Replace the error code with 'EACCES' if this lands: + // On Windows, we are not allowed to delete a read-only folder yet. + // TODO(RaisinTen): Remove Windows special-casing if this lands: // https://github.com/libuv/libuv/pull/3193 return common.isWindows ? (exists === true && err?.code === 'EPERM') : From 8975f3e85ab931a980685b2c151f588ae420db07 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:45:59 +0530 Subject: [PATCH 05/20] tmpdir: remove empty call --- test/common/tmpdir.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index 177f543c98056a..4b1e866e99474e 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -82,8 +82,6 @@ function logErrorInfo(err) { console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); } - console.error(); - // Additionally logging err, just in case throwing err gets overshadowed by // an error from a test failure. console.error(err); From aedf3a9fa297ca276049c2f5b095176ee3392893 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:50:49 +0530 Subject: [PATCH 06/20] fixup! tmpdir: remove empty call --- test/parallel/test-fs-rm.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index c4142f0cebd3f0..a1511459717a9d 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -293,8 +293,8 @@ function removeAsync(dir) { // On Windows, we are allowed to access and modify the contents of a // read-only folder. return common.isWindows ? - (exists === false && err === null)) : - (exists === true && err?.code === 'EACCES')); + (exists === false && err === null) : + (exists === true && err?.code === 'EACCES'); } { From 838cad0441e5c26f56fe5af67291aaee902ab5a2 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 20:55:20 +0530 Subject: [PATCH 07/20] lint fix --- test/parallel/test-fs-rm.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index a1511459717a9d..aa4ffc19949f8d 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -293,8 +293,8 @@ function removeAsync(dir) { // On Windows, we are allowed to access and modify the contents of a // read-only folder. return common.isWindows ? - (exists === false && err === null) : - (exists === true && err?.code === 'EACCES'); + (exists === false && err === null) : + (exists === true && err?.code === 'EACCES'); } { @@ -366,8 +366,8 @@ function removeAsync(dir) { // TODO(RaisinTen): Remove Windows special-casing if this lands: // https://github.com/libuv/libuv/pull/3193 return common.isWindows ? - (exists === true && err?.code === 'EPERM') : - (exists === true && err?.code === 'EACCES'); + (exists === true && err?.code === 'EPERM') : + (exists === true && err?.code === 'EACCES'); } { From ecac8c638190c8c1ca77010727c2c5a2d9ae3e95 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 21:09:05 +0530 Subject: [PATCH 08/20] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/parallel/test-fs-rm.js | 44 +++++++------------------------------ 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index aa4ffc19949f8d..dca0ccb0e54a2e 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -315,15 +315,8 @@ function removeAsync(dir) { err = _err; } - try { - fs.chmodSync(dirname, 0o777); - } catch { - } - - try { - fs.chmodSync(filePath, 0o777); - } catch { - } + try { fs.chmodSync(dirname, 0o777); } catch {} + try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { throw err; @@ -341,15 +334,8 @@ function removeAsync(dir) { fs.chmodSync(dirname, 0o444); fs.rm(filePath, { force: true }, common.mustCall((err) => { - try { - fs.chmodSync(dirname, 0o777); - } catch { - } - - try { - fs.chmodSync(filePath, 0o777); - } catch { - } + try { fs.chmodSync(dirname, 0o777); } catch {} + try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { throw err; @@ -391,15 +377,8 @@ function removeAsync(dir) { err = _err; } - try { - fs.chmodSync(middle, 0o777); - } catch { - } - - try { - fs.chmodSync(leaf, 0o777); - } catch { - } + try { fs.chmodSync(middle, 0o777); } catch {} + try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { throw err; @@ -420,15 +399,8 @@ function removeAsync(dir) { fs.chmodSync(middle, 0o555); fs.rm(root, { recursive: true }, common.mustCall((err) => { - try { - fs.chmodSync(middle, 0o777); - } catch { - } - - try { - fs.chmodSync(leaf, 0o777); - } catch { - } + try { fs.chmodSync(middle, 0o777); } catch {} + try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { throw err; From f48b819abcc7af41f433c33995c7533113385a51 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 21:11:04 +0530 Subject: [PATCH 09/20] Apply suggestions from code review --- test/common/tmpdir.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index 4b1e866e99474e..2a22172bca7efd 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -28,15 +28,8 @@ function rmSync(pathname) { if (errCode === 'EACCES' || errCode === 'EPERM') { const surroundingDir = path.join(errPath, '..'); - try { - fs.chmodSync(surroundingDir, 0o777); - } catch { - } - - try { - fs.chmodSync(errPath, 0o777); - } catch { - } + try { fs.chmodSync(surroundingDir, 0o777); } catch {} + try { fs.chmodSync(errPath, 0o777); } catch {} } } From f67b959291bc09582a990800bd6967c5890382bb Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 21:13:56 +0530 Subject: [PATCH 10/20] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/parallel/test-fs-rm.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index dca0ccb0e54a2e..b89091eb047c59 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -351,9 +351,8 @@ function removeAsync(dir) { // On Windows, we are not allowed to delete a read-only folder yet. // TODO(RaisinTen): Remove Windows special-casing if this lands: // https://github.com/libuv/libuv/pull/3193 - return common.isWindows ? - (exists === true && err?.code === 'EPERM') : - (exists === true && err?.code === 'EACCES'); + return exists === true && + err?.code === common.isWindows ? 'EPERM' : 'EACCES'; } { From d4e6c6e98a9345c29eb1b98c772b040c7b4cfdc2 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Jun 2021 21:17:35 +0530 Subject: [PATCH 11/20] lint fix --- test/parallel/test-fs-rm.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index b89091eb047c59..ffa212c2440a8a 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -334,8 +334,8 @@ function removeAsync(dir) { fs.chmodSync(dirname, 0o444); fs.rm(filePath, { force: true }, common.mustCall((err) => { - try { fs.chmodSync(dirname, 0o777); } catch {} - try { fs.chmodSync(filePath, 0o777); } catch {} + try { fs.chmodSync(dirname, 0o777); } catch {} + try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { throw err; @@ -398,8 +398,8 @@ function removeAsync(dir) { fs.chmodSync(middle, 0o555); fs.rm(root, { recursive: true }, common.mustCall((err) => { - try { fs.chmodSync(middle, 0o777); } catch {} - try { fs.chmodSync(leaf, 0o777); } catch {} + try { fs.chmodSync(middle, 0o777); } catch {} + try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { throw err; From 3e52248434868345192848baa1e5aa918421a3b6 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Jun 2021 18:53:10 +0530 Subject: [PATCH 12/20] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/common/tmpdir.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index 2a22172bca7efd..b7df5389a7e4a4 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -10,18 +10,13 @@ function rmSync(pathname) { const maxRetries = 10; for (let retryNumber = 0; retryNumber < maxRetries; ++retryNumber) { - err = null; - try { fs.rmSync(pathname, { maxRetries: 3, recursive: true, force: true }); + return; } catch (_err) { err = _err; } - if (err === null) { - return; - } - const errPath = err.path; const errCode = err.code; @@ -33,11 +28,7 @@ function rmSync(pathname) { } } - if (err === null) { - return; - } - - throw err; + if (err) throw err; } const testRoot = process.env.NODE_TEST_DIR ? From bd817e2785ccaa4301902c3e046ef23077fe8f1a Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Jun 2021 19:12:59 +0530 Subject: [PATCH 13/20] Apply suggestions from code review --- test/common/tmpdir.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index b7df5389a7e4a4..a5285709130a53 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -28,7 +28,7 @@ function rmSync(pathname) { } } - if (err) throw err; + if (err !== null) throw err; } const testRoot = process.env.NODE_TEST_DIR ? From d0173e54e9329b23e4301a4992f32966fbf74e20 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Jun 2021 19:42:26 +0530 Subject: [PATCH 14/20] tmpdir: remove `if` around `throw err` --- test/common/tmpdir.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index a5285709130a53..d404f8d3c23ae3 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -28,7 +28,7 @@ function rmSync(pathname) { } } - if (err !== null) throw err; + throw err; } const testRoot = process.env.NODE_TEST_DIR ? From e88bdbb0c1284cea7a21a583a0010129158817fe Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 5 Jun 2021 20:11:03 +0530 Subject: [PATCH 15/20] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- test/parallel/test-fs-rm.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index ffa212c2440a8a..5cd61a82a6cdc6 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -319,7 +319,9 @@ function removeAsync(dir) { try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { - throw err; + const e = new Error('Invalid state'); + e.cause = err; + throw e; } } @@ -338,7 +340,9 @@ function removeAsync(dir) { try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { - throw err; + const e = new Error('Invalid state'); + e.cause = err; + throw e; } })); } @@ -380,7 +384,9 @@ function removeAsync(dir) { try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { - throw err; + const e = new Error('Invalid state'); + e.cause = err; + throw e; } } @@ -402,7 +408,9 @@ function removeAsync(dir) { try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { - throw err; + const e = new Error('Invalid state'); + e.cause = err; + throw e; } })); } From a766a75bd72c7a0019aae25bfcb8e6679d7ff989 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 5 Jun 2021 20:12:21 +0530 Subject: [PATCH 16/20] tmpdir: inline error logging function --- test/common/tmpdir.js | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index d404f8d3c23ae3..a80bbf1633a1d5 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -53,26 +53,6 @@ function refresh() { } } -function logErrorInfo(err) { - console.error('Can\'t clean tmpdir:', tmpPath); - - const files = fs.readdirSync(tmpPath); - console.error('Files blocking:', files); - - if (files.some((f) => f.startsWith('.nfs'))) { - // Warn about NFS "silly rename" - console.error('Note: ".nfs*" might be files that were open and ' + - 'unlinked but not closed.'); - console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); - } - - // Additionally logging err, just in case throwing err gets overshadowed by - // an error from a test failure. - console.error(err); - - throw err; -} - function onexit() { // Change directory to avoid possible EBUSY if (isMainThread) @@ -81,7 +61,23 @@ function onexit() { try { rmSync(tmpPath); } catch (err) { - logErrorInfo(err); + console.error('Can\'t clean tmpdir:', tmpPath); + + const files = fs.readdirSync(tmpPath); + console.error('Files blocking:', files); + + if (files.some((f) => f.startsWith('.nfs'))) { + // Warn about NFS "silly rename" + console.error('Note: ".nfs*" might be files that were open and ' + + 'unlinked but not closed.'); + console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); + } + + // Additionally logging err, just in case throwing err gets overshadowed by + // an error from a test failure. + console.error(err); + + throw err; } } From 8c76097a2a4aec5271a241672ab51469e71b712f Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 5 Jun 2021 20:26:18 +0530 Subject: [PATCH 17/20] use InvalidStateError class --- test/parallel/test-fs-rm.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 5cd61a82a6cdc6..ca6b29078659cb 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -286,6 +286,13 @@ function removeAsync(dir) { // IBMi has a different access permission mechanism // This test should not be run as `root` if (!common.isIBMi && (common.isWindows || process.getuid() !== 0)) { + class InvalidStateError extends Error { + constructor(err) { + super('Invalid state'); + this.cause = err; + } + } + // Check that deleting a file that cannot be accessed using rmsync throws: // https://github.com/nodejs/node/issues/38683 { @@ -319,9 +326,7 @@ function removeAsync(dir) { try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { - const e = new Error('Invalid state'); - e.cause = err; - throw e; + throw new InvalidStateError(err); } } @@ -340,9 +345,7 @@ function removeAsync(dir) { try { fs.chmodSync(filePath, 0o777); } catch {} if (!isValidState(fs.existsSync(filePath), err)) { - const e = new Error('Invalid state'); - e.cause = err; - throw e; + throw new InvalidStateError(err); } })); } @@ -384,9 +387,7 @@ function removeAsync(dir) { try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { - const e = new Error('Invalid state'); - e.cause = err; - throw e; + throw new InvalidStateError(err); } } @@ -408,9 +409,7 @@ function removeAsync(dir) { try { fs.chmodSync(leaf, 0o777); } catch {} if (!isValidState(fs.existsSync(root), err)) { - const e = new Error('Invalid state'); - e.cause = err; - throw e; + throw new InvalidStateError(err); } })); } From 65657f2faa79c554694ed2302d106be2aaad5af0 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 5 Jun 2021 20:37:43 +0530 Subject: [PATCH 18/20] isValidState -> validateState --- test/parallel/test-fs-rm.js | 40 +++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index ca6b29078659cb..d930832ea780fc 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -296,12 +296,17 @@ function removeAsync(dir) { // Check that deleting a file that cannot be accessed using rmsync throws: // https://github.com/nodejs/node/issues/38683 { - function isValidState(exists, err) { + function validateState(exists, err) { // On Windows, we are allowed to access and modify the contents of a // read-only folder. - return common.isWindows ? - (exists === false && err === null) : - (exists === true && err?.code === 'EACCES'); + const isValidState = + common.isWindows ? + (exists === false && err === null) : + (exists === true && err?.code === 'EACCES'); + + if (!isValidState) { + throw new InvalidStateError(err); + } } { @@ -325,9 +330,7 @@ function removeAsync(dir) { try { fs.chmodSync(dirname, 0o777); } catch {} try { fs.chmodSync(filePath, 0o777); } catch {} - if (!isValidState(fs.existsSync(filePath), err)) { - throw new InvalidStateError(err); - } + validateState(fs.existsSync(filePath), err); } { @@ -344,9 +347,7 @@ function removeAsync(dir) { try { fs.chmodSync(dirname, 0o777); } catch {} try { fs.chmodSync(filePath, 0o777); } catch {} - if (!isValidState(fs.existsSync(filePath), err)) { - throw new InvalidStateError(err); - } + validateState(fs.existsSync(filePath), err); })); } } @@ -354,12 +355,17 @@ function removeAsync(dir) { // Check endless recursion. // https://github.com/nodejs/node/issues/34580 { - function isValidState(exists, err) { + function validateState(exists, err) { // On Windows, we are not allowed to delete a read-only folder yet. // TODO(RaisinTen): Remove Windows special-casing if this lands: // https://github.com/libuv/libuv/pull/3193 - return exists === true && - err?.code === common.isWindows ? 'EPERM' : 'EACCES'; + const isValidState = + exists === true && + err?.code === common.isWindows ? 'EPERM' : 'EACCES'; + + if (!isValidState) { + throw new InvalidStateError(err); + } } { @@ -386,9 +392,7 @@ function removeAsync(dir) { try { fs.chmodSync(middle, 0o777); } catch {} try { fs.chmodSync(leaf, 0o777); } catch {} - if (!isValidState(fs.existsSync(root), err)) { - throw new InvalidStateError(err); - } + validateState(fs.existsSync(root), err); } { @@ -408,9 +412,7 @@ function removeAsync(dir) { try { fs.chmodSync(middle, 0o777); } catch {} try { fs.chmodSync(leaf, 0o777); } catch {} - if (!isValidState(fs.existsSync(root), err)) { - throw new InvalidStateError(err); - } + validateState(fs.existsSync(root), err); })); } } From befac4bc7874d2f9c5eeeae6904633efa27e2cd7 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Mon, 7 Jun 2021 19:20:16 +0530 Subject: [PATCH 19/20] log error from tmpdir only once at max --- test/common/tmpdir.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index a80bbf1633a1d5..dc7d85f2101fd5 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -73,11 +73,13 @@ function onexit() { console.error('See http://nfs.sourceforge.net/#faq_d2 for details.'); } - // Additionally logging err, just in case throwing err gets overshadowed by - // an error from a test failure. + // Manually logging err instead of throwing it, so that it doesn't get + // overshadowed by an error from a test failure. console.error(err); - throw err; + // Setting the process exit code to a non-zero exit code, so that this gets + // marked as `not ok` during a CI run. + process.exitCode ||= 1; } } From 3fe174f7642c329b90170035cdb00e143b5eb9fd Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Mon, 7 Jun 2021 19:56:47 +0530 Subject: [PATCH 20/20] ||= ~> if --- test/common/tmpdir.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js index dc7d85f2101fd5..3fba1e54ba60b0 100644 --- a/test/common/tmpdir.js +++ b/test/common/tmpdir.js @@ -79,7 +79,9 @@ function onexit() { // Setting the process exit code to a non-zero exit code, so that this gets // marked as `not ok` during a CI run. - process.exitCode ||= 1; + if (!process.exitCode) { + process.exitCode = 1; + } } }