From 6d8e90d82e97b272f8c0be7f95a74d9f2d1faef5 Mon Sep 17 00:00:00 2001 From: David Huggins-Daines Date: Tue, 6 Aug 2024 07:12:29 -0400 Subject: [PATCH 1/5] fix: report syntax errors in package.json (fixes: #5141) --- lib/cli/options.js | 5 +++++ test/node-unit/cli/options.spec.js | 31 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lib/cli/options.js b/lib/cli/options.js index d238737d37..3faeec71f2 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -195,6 +195,11 @@ const loadPkgRc = (args = {}) => { `Unable to read/parse ${filepath}: ${err}`, filepath ); + } else if (err.toString().includes('SyntaxError')) { + throw createUnparsableFileError( + `Unable to parse ${filepath}: ${err}`, + filepath + ); } debug('failed to read default package.json at %s; ignoring', filepath); } diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 60357d12ae..cb8e630a6b 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -199,6 +199,37 @@ describe('options', function () { }); }); + describe('when path to package.json unspecified and package.json exists but is invalid', function () { + beforeEach(function () { + const filepath = '/some/package.json'; + readFileSync = sinon.stub(); + // package.json + readFileSync + .onFirstCall() + // Note the extra comma + .returns('{"mocha": {"retries": 3, "_": ["foobar.spec.js"],}}'); + findConfig = sinon.stub().returns('/some/.mocharc.json'); + loadConfig = sinon.stub().returns({}); + findupSync = sinon.stub().returns(filepath); + loadOptions = proxyLoadOptions({ + readFileSync, + findConfig, + loadConfig, + findupSync + }); + }); + + it('should throw', function () { + expect( + () => { + loadOptions(); + }, + 'to throw', + 'Unable to parse /some/package.json: SyntaxError: Expected double-quoted property name in JSON at position 49' + ); + }); + }); + describe('when called with package = false (`--no-package`)', function () { let result; beforeEach(function () { From f1fe34432d108a74f07f8465948fb02ae53544b3 Mon Sep 17 00:00:00 2001 From: David Huggins-Daines Date: Tue, 6 Aug 2024 07:14:17 -0400 Subject: [PATCH 2/5] fix(tests): incorrect test (should use existing result) Since `readFileSync` is only stubbed `onFirstCall` we get a different answer the second time around which is probably Not What You Want. But also we *already checked that config = false*. So you could just remove this test, it does nothing useful. --- test/node-unit/cli/options.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index cb8e630a6b..dbba4620cf 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -318,7 +318,7 @@ describe('options', function () { }); it('should set config = false', function () { - expect(loadOptions(), 'to have property', 'config', false); + expect(result, 'to have property', 'config', false); }); }); From a7f015a0227d7e344b8f88ce6585701099093444 Mon Sep 17 00:00:00 2001 From: David Huggins-Daines Date: Sat, 17 Aug 2024 09:07:04 -0400 Subject: [PATCH 3/5] fix: separate read and parse errors --- lib/cli/options.js | 35 +++++++++++++++++++----------- test/node-unit/cli/options.spec.js | 2 +- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/lib/cli/options.js b/lib/cli/options.js index 3faeec71f2..fc0c951a8c 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -181,8 +181,24 @@ const loadPkgRc = (args = {}) => { result = {}; const filepath = args.package || findUp.sync(mocharc.package); if (filepath) { + let configData; try { - const pkg = JSON.parse(fs.readFileSync(filepath, 'utf8')); + configData = fs.readFileSync(filepath, 'utf8'); + } catch (err) { + // If `args.package` was explicitly specified, throw an error + if (filepath == args.package) { + throw createUnparsableFileError( + `Unable to read ${filepath}: ${err}`, + filepath + ); + } else { + debug('failed to read default package.json at %s; ignoring', + filepath); + return result; + } + } + try { + const pkg = JSON.parse(configData); if (pkg.mocha) { debug('`mocha` prop of package.json parsed: %O', pkg.mocha); result = pkg.mocha; @@ -190,18 +206,11 @@ const loadPkgRc = (args = {}) => { debug('no config found in %s', filepath); } } catch (err) { - if (args.package) { - throw createUnparsableFileError( - `Unable to read/parse ${filepath}: ${err}`, - filepath - ); - } else if (err.toString().includes('SyntaxError')) { - throw createUnparsableFileError( - `Unable to parse ${filepath}: ${err}`, - filepath - ); - } - debug('failed to read default package.json at %s; ignoring', filepath); + // If JSON failed to parse, throw an error. + throw createUnparsableFileError( + `Unable to parse ${filepath}: ${err}`, + filepath + ); } } return result; diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index dbba4620cf..9ebb9e0542 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -149,7 +149,7 @@ describe('options', function () { loadOptions('--package /something/wherever --require butts'); }, 'to throw', - 'Unable to read/parse /something/wherever: bad file message' + 'Unable to read /something/wherever: bad file message' ); }); }); From 8c4103fd822e60f2dc5b7c38910fd13ecc6423da Mon Sep 17 00:00:00 2001 From: David Huggins-Daines Date: Tue, 8 Oct 2024 13:44:50 -0400 Subject: [PATCH 4/5] fix: clarify invalid JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Josh Goldberg ✨ --- test/node-unit/cli/options.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 9ebb9e0542..0ee516fc48 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -206,8 +206,7 @@ describe('options', function () { // package.json readFileSync .onFirstCall() - // Note the extra comma - .returns('{"mocha": {"retries": 3, "_": ["foobar.spec.js"],}}'); + .returns('{definitely-invalid'); findConfig = sinon.stub().returns('/some/.mocharc.json'); loadConfig = sinon.stub().returns({}); findupSync = sinon.stub().returns(filepath); From cd33f01ea8dc21ed3db67f12c31dc572fc77886a Mon Sep 17 00:00:00 2001 From: David Huggins-Daines Date: Tue, 8 Oct 2024 13:54:03 -0400 Subject: [PATCH 5/5] fix(test): expect only a SyntaxError nothing else --- test/node-unit/cli/options.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 0ee516fc48..7c846a37ed 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -224,7 +224,7 @@ describe('options', function () { loadOptions(); }, 'to throw', - 'Unable to parse /some/package.json: SyntaxError: Expected double-quoted property name in JSON at position 49' + /SyntaxError/, ); }); });