From 66dde0020bf3031065928460846ad802259d1fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Sun, 1 Nov 2026 16:27:55 +0000 Subject: [PATCH 1/2] test: support unit tests next to source files Unit tests live next to the code they test as `lib/**/*.test.js`, while `test/` keeps the end-to-end tests. Test files are excluded from the npm package and from coverage, and are linted with the same config as `test/`. ESLint 5 doesn't support `extends` in `overrides`, so the config moves to .eslintrc.js, extends the jest addon (which only applies to test files) and applies what es9 adds on top of es6 to `lib/**/*.test.js`. --- .eslintrc | 7 ------- .eslintrc.js | 24 ++++++++++++++++++++++++ package.json | 10 +++++++++- 3 files changed, 33 insertions(+), 8 deletions(-) delete mode 100644 .eslintrc create mode 100644 .eslintrc.js diff --git a/.eslintrc b/.eslintrc deleted file mode 100644 index 3124fb8..0000000 --- a/.eslintrc +++ /dev/null @@ -1,7 +0,0 @@ -{ - "root": true, - "extends": [ - "eslint-config-moxy/es6", - "eslint-config-moxy/addons/node" - ] -} diff --git a/.eslintrc.js b/.eslintrc.js new file mode 100644 index 0000000..e14bf57 --- /dev/null +++ b/.eslintrc.js @@ -0,0 +1,24 @@ +'use strict'; + +// Unit tests in `lib/` must get the same config as `test/.eslintrc`, but ESLint 5 doesn't support +// `extends` in `overrides`, so we apply what `es9` adds on top of `es6` manually +const es7 = require('eslint-config-moxy/es7'); +const es8 = require('eslint-config-moxy/es8'); +const es9 = require('eslint-config-moxy/es9'); + +module.exports = { + root: true, + extends: [ + 'eslint-config-moxy/es6', + 'eslint-config-moxy/addons/node', + // Only enabled for test files, such as `lib/**/*.test.js` + 'eslint-config-moxy/addons/jest', + ], + overrides: [ + { + files: ['lib/**/*.test.js'], + parserOptions: es9.parserOptions, + rules: Object.assign({}, es7.rules, es8.rules, es9.rules), + }, + ], +}; diff --git a/package.json b/package.json index 24b2eb4..43564b9 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "license": "MIT", "main": "index.js", "files": [ - "lib" + "lib", + "!lib/**/*.test.js" ], "scripts": { "lint": "eslint .", @@ -36,6 +37,13 @@ "pre-commit": "lint-staged" } }, + "jest": { + "collectCoverageFrom": [ + "index.js", + "lib/**/*.js", + "!lib/**/*.test.js" + ] + }, "lint-staged": { "*.js": [ "eslint --fix", From bd535d724e3a0d24dd461cb3a09e9679c2288c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Cruz?= Date: Sun, 1 Nov 2026 16:27:56 +0000 Subject: [PATCH 2/2] fix: close the file descriptor when reading a shebang fails readShebang only closed the file descriptor after a successful read, so it leaked when fs.readSync threw, e.g.: when the file is a directory. Based on https://github.com/moxystudio/node-cross-spawn/pull/184. Co-authored-by: Huanyi Xie <74809554+xiehuanyi@users.noreply.github.com> --- lib/util/readShebang.js | 10 ++++-- lib/util/readShebang.test.js | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 lib/util/readShebang.test.js diff --git a/lib/util/readShebang.js b/lib/util/readShebang.js index 5e83733..ac28aba 100644 --- a/lib/util/readShebang.js +++ b/lib/util/readShebang.js @@ -13,8 +13,14 @@ function readShebang(command) { try { fd = fs.openSync(command, 'r'); fs.readSync(fd, buffer, 0, size, 0); - fs.closeSync(fd); - } catch (e) { /* Empty */ } + } catch (e) { /* Empty */ } finally { + // Always close the file descriptor, even if reading failed + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch (e) { /* Empty */ } + } + } // Attempt to extract shebang (null is returned if not a shebang) return shebangCommand(buffer.toString()); diff --git a/lib/util/readShebang.test.js b/lib/util/readShebang.test.js new file mode 100644 index 0000000..df15d02 --- /dev/null +++ b/lib/util/readShebang.test.js @@ -0,0 +1,64 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const readShebang = require('./readShebang'); + +describe('readShebang', () => { + afterEach(() => jest.restoreAllMocks()); + + it('should read the shebang command', () => { + const close = jest.spyOn(fs, 'closeSync'); + + expect(readShebang(path.join(__dirname, '../../test/fixtures/shebang'))).toBe('node'); + expect(close).toHaveBeenCalledTimes(1); + }); + + it('should return null if the file has no shebang', () => { + expect(readShebang(path.join(__dirname, '../../test/fixtures/echo.js'))).toBe(null); + }); + + it('should close the file descriptor when reading fails, including descriptor zero', () => { + jest.spyOn(fs, 'openSync').mockReturnValue(0); + jest.spyOn(fs, 'readSync').mockImplementation(() => { + throw new Error('read failed'); + }); + const close = jest.spyOn(fs, 'closeSync').mockImplementation(() => {}); + + expect(readShebang('command')).toBe(null); + expect(close).toHaveBeenCalledTimes(1); + expect(close).toHaveBeenCalledWith(0); + }); + + it('should close the file descriptor when the file is a directory', () => { + const open = jest.spyOn(fs, 'openSync'); + const close = jest.spyOn(fs, 'closeSync'); + + expect(readShebang(__dirname)).toBe(null); + + // Opening a directory fails on Windows, but succeeds on other platforms and then reading fails + expect(close).toHaveBeenCalledTimes(open.mock.results.filter((result) => result.type === 'return').length); + }); + + it('should not close a file descriptor when opening fails', () => { + jest.spyOn(fs, 'openSync').mockImplementation(() => { + throw new Error('open failed'); + }); + const close = jest.spyOn(fs, 'closeSync').mockImplementation(() => {}); + + expect(readShebang('command')).toBe(null); + expect(close).not.toHaveBeenCalled(); + }); + + it('should return null when closing also fails', () => { + jest.spyOn(fs, 'openSync').mockReturnValue(0); + jest.spyOn(fs, 'readSync').mockImplementation(() => { + throw new Error('read failed'); + }); + jest.spyOn(fs, 'closeSync').mockImplementation(() => { + throw new Error('close failed'); + }); + + expect(readShebang('command')).toBe(null); + }); +});