From 696fabd763967efa862e86edb3f8e5c4ddd7d864 Mon Sep 17 00:00:00 2001 From: 43081j <43081j@users.noreply.github.com> Date: Mon, 26 Feb 2024 19:02:16 +0000 Subject: [PATCH 1/3] fix: support some virtual contexts in `toThrow` This adds support for VM situations where we pass a `RegExp` from another process. Note that we don't have a full fix for this stuff until `check-error` also supports `Error` being from another origin. --- lib/chai/core/assertions.js | 6 +++--- lib/chai/utils/index.js | 4 ++++ test/assert.js | 3 +++ test/virtual-machines.js | 31 +++++++++++++++++++++++++++++++ web-test-runner.config.js | 5 ++++- 5 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 test/virtual-machines.js diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index 7d72b068..db7b2a94 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -2676,7 +2676,7 @@ function assertThrows (errorLike, errMsgMatcher, msg) { , negate = flag(this, 'negate') || false; new Assertion(obj, flagMsg, ssfi, true).is.a('function'); - if (errorLike instanceof RegExp || typeof errorLike === 'string') { + if (_.isRegExp(errorLike) || typeof errorLike === 'string') { errMsgMatcher = errorLike; errorLike = null; } @@ -2709,7 +2709,7 @@ function assertThrows (errorLike, errMsgMatcher, msg) { } this.assert( - caughtErr + caughtErr !== undefined , 'expected #{this} to throw ' + errorLikeString , 'expected #{this} to not throw an error but #{act} was thrown' , errorLike && errorLike.toString() @@ -2760,7 +2760,7 @@ function assertThrows (errorLike, errMsgMatcher, msg) { if (caughtErr && errMsgMatcher !== undefined && errMsgMatcher !== null) { // Here we check compatible messages var placeholder = 'including'; - if (errMsgMatcher instanceof RegExp) { + if (_.isRegExp(errMsgMatcher)) { placeholder = 'matching' } diff --git a/lib/chai/utils/index.js b/lib/chai/utils/index.js index 0602997a..099c22ad 100644 --- a/lib/chai/utils/index.js +++ b/lib/chai/utils/index.js @@ -94,3 +94,7 @@ export {isNaN} from './isNaN.js'; // getOperator method export {getOperator} from './getOperator.js'; + +export function isRegExp(obj) { + return Object.prototype.toString.call(obj) === '[object RegExp]'; +} diff --git a/test/assert.js b/test/assert.js index c469dda1..d22998a7 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1644,6 +1644,9 @@ describe('assert', function () { assert[throws](function() { throw new Error('bar'); }, Error, 'bar'); assert[throws](function() { throw new Error(''); }, Error, ''); assert[throws](function() { throw new Error('foo') }, ''); + assert[throws](function() { throw ''; }, ''); + assert[throws](function() { throw ''; }, /^$/); + assert[throws](function() { throw new Error(''); }, /^$/); var thrownErr = assert[throws](function() { throw new Error('foo'); }); assert(thrownErr instanceof Error, 'assert.' + throws + ' returns error'); diff --git a/test/virtual-machines.js b/test/virtual-machines.js new file mode 100644 index 00000000..92ac1978 --- /dev/null +++ b/test/virtual-machines.js @@ -0,0 +1,31 @@ +import vm from 'node:vm'; +import * as chai from '../index.js'; + +const {assert} = chai; +const vmContext = {assert}; +vm.createContext(vmContext); + +function runCodeInVm(code) { + vm.runInContext(code, vmContext); +} + +describe('node virtual machines', function () { + it('throws', function() { + const shouldNotThrow = [ + `assert.throws(function() { throw ''; }, /^$/);`, + `assert.throws(function() { throw new Error('bleepbloop'); });`, + `assert.throws(function() { throw new Error(''); });`, + // TODO (43081j): enable this test once check-error supports + // cross-vm `Error` objects + //`assert.throws(function() { throw new Error('swoosh'); }, /swoosh/);` + ]; + + for (const code of shouldNotThrow) { + assert.doesNotThrow( + () => { + runCodeInVm(code); + } + ); + } + }); +}); diff --git a/web-test-runner.config.js b/web-test-runner.config.js index 50d70191..b9b6cb25 100644 --- a/web-test-runner.config.js +++ b/web-test-runner.config.js @@ -5,7 +5,10 @@ const commonjs = fromRollup(rollupCommonjs); export default { nodeResolve: true, - files: ["test/*.js"], + files: [ + "test/*.js", + "!test/virtual-machines.js" + ], plugins: [ commonjs({ include: [ From 23b4dc29053f9899bf28e35e3492e58338d29241 Mon Sep 17 00:00:00 2001 From: 43081j <43081j@users.noreply.github.com> Date: Sat, 4 May 2024 18:52:57 +0100 Subject: [PATCH 2/3] fix: support throwing of unusual errors Adds support for throwing things like `undefined`, functions, etc. --- lib/chai/core/assertions.js | 24 +++++++++++++++++++----- lib/chai/utils/index.js | 6 ++++++ test/assert.js | 5 +++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index db7b2a94..a1664297 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -2681,10 +2681,12 @@ function assertThrows (errorLike, errMsgMatcher, msg) { errorLike = null; } - var caughtErr; + let caughtErr; + let errorWasThrown = false; try { obj(); } catch (err) { + errorWasThrown = true; caughtErr = err; } @@ -2708,14 +2710,26 @@ function assertThrows (errorLike, errMsgMatcher, msg) { errorLikeString = _.checkError.getConstructorName(errorLike); } + let actual = caughtErr; + if (caughtErr instanceof Error) { + actual = caughtErr.toString(); + } else if (typeof caughtErr === 'string') { + actual = caughtErr; + } else if (caughtErr && (typeof caughtErr === 'object' || typeof caughtErr === 'function')) { + try { + actual = _.checkError.getConstructorName(caughtErr); + } catch (_err) { + // somehow wasn't a constructor, maybe we got a function thrown + // or similar + } + } + this.assert( - caughtErr !== undefined + errorWasThrown , 'expected #{this} to throw ' + errorLikeString , 'expected #{this} to not throw an error but #{act} was thrown' , errorLike && errorLike.toString() - , (caughtErr instanceof Error ? - caughtErr.toString() : (typeof caughtErr === 'string' ? caughtErr : caughtErr && - _.checkError.getConstructorName(caughtErr))) + , actual ); } diff --git a/lib/chai/utils/index.js b/lib/chai/utils/index.js index 099c22ad..212fc168 100644 --- a/lib/chai/utils/index.js +++ b/lib/chai/utils/index.js @@ -95,6 +95,12 @@ export {isNaN} from './isNaN.js'; // getOperator method export {getOperator} from './getOperator.js'; +/** + * Determines if an object is a `RegExp` + * This is used since `instanceof` will not work in virtual contexts + * @param {*} obj Object to test + * @return {boolean} + */ export function isRegExp(obj) { return Object.prototype.toString.call(obj) === '[object RegExp]'; } diff --git a/test/assert.js b/test/assert.js index d22998a7..c10a6775 100644 --- a/test/assert.js +++ b/test/assert.js @@ -1635,6 +1635,8 @@ describe('assert', function () { }); it('throws / throw / Throw', function() { + class CustomError extends Error {} + ['throws', 'throw', 'Throw'].forEach(function (throws) { assert[throws](function() { throw new Error('foo'); }); assert[throws](function() { throw new Error(''); }, ''); @@ -1647,6 +1649,9 @@ describe('assert', function () { assert[throws](function() { throw ''; }, ''); assert[throws](function() { throw ''; }, /^$/); assert[throws](function() { throw new Error(''); }, /^$/); + assert[throws](function() { throw undefined; }); + assert[throws](function() { throw new CustomError('foo'); }); + assert[throws](function() { throw (() => {}); }); var thrownErr = assert[throws](function() { throw new Error('foo'); }); assert(thrownErr instanceof Error, 'assert.' + throws + ' returns error'); From c182e03bac97d9e677fef8e21cf920dca39963a0 Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Thu, 9 May 2024 08:22:13 +0100 Subject: [PATCH 3/3] chore: upgrade check-error --- lib/chai/utils/index.js | 3 +- package-lock.json | 8 ++--- package.json | 2 +- test/virtual-machines.js | 65 +++++++++++++++++++++------------------- 4 files changed, 41 insertions(+), 37 deletions(-) diff --git a/lib/chai/utils/index.js b/lib/chai/utils/index.js index 212fc168..80306f79 100644 --- a/lib/chai/utils/index.js +++ b/lib/chai/utils/index.js @@ -98,8 +98,9 @@ export {getOperator} from './getOperator.js'; /** * Determines if an object is a `RegExp` * This is used since `instanceof` will not work in virtual contexts + * * @param {*} obj Object to test - * @return {boolean} + * @returns {boolean} */ export function isRegExp(obj) { return Object.prototype.toString.call(obj) === '[object RegExp]'; diff --git a/package-lock.json b/package-lock.json index 4ce438d5..4848b60c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "MIT", "dependencies": { "assertion-error": "^2.0.1", - "check-error": "^2.0.0", + "check-error": "^2.1.1", "deep-eql": "^5.0.1", "loupe": "^3.1.0", "pathval": "^2.0.0" @@ -2027,9 +2027,9 @@ } }, "node_modules/check-error": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/check-error/-/check-error-2.0.0.tgz", - "integrity": "sha512-tjLAOBHKVxtPoHe/SA7kNOMvhCRdCJ3vETdeY0RuAc9popf+hyaSV6ZEg9hr4cpWF7jmo/JSWEnLDrnijS9Tog==", + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/check-error/-/check-error-2.1.1.tgz", + "integrity": "sha512-OAlb+T7V4Op9OwdkjmguYRqncdlx5JiofwOAUkmTF+jNdHwzTaTs4sRAGpzLF3oOz5xAyDGrPgeIDFQmDOTiJw==", "engines": { "node": ">= 16" } diff --git a/package.json b/package.json index f2f9aad1..a2cb11ff 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ }, "dependencies": { "assertion-error": "^2.0.1", - "check-error": "^2.0.0", + "check-error": "^2.1.1", "deep-eql": "^5.0.1", "loupe": "^3.1.0", "pathval": "^2.0.0" diff --git a/test/virtual-machines.js b/test/virtual-machines.js index 92ac1978..da5ec416 100644 --- a/test/virtual-machines.js +++ b/test/virtual-machines.js @@ -1,31 +1,34 @@ -import vm from 'node:vm'; -import * as chai from '../index.js'; - -const {assert} = chai; -const vmContext = {assert}; -vm.createContext(vmContext); - -function runCodeInVm(code) { - vm.runInContext(code, vmContext); -} - -describe('node virtual machines', function () { - it('throws', function() { - const shouldNotThrow = [ - `assert.throws(function() { throw ''; }, /^$/);`, - `assert.throws(function() { throw new Error('bleepbloop'); });`, - `assert.throws(function() { throw new Error(''); });`, - // TODO (43081j): enable this test once check-error supports - // cross-vm `Error` objects - //`assert.throws(function() { throw new Error('swoosh'); }, /swoosh/);` - ]; - - for (const code of shouldNotThrow) { - assert.doesNotThrow( - () => { - runCodeInVm(code); - } - ); - } - }); -}); +import vm from 'node:vm'; +import * as chai from '../index.js'; + +const {assert} = chai; +const vmContext = {assert}; +vm.createContext(vmContext); + +/** + * Run the code in a virtual context + * + * @param {string} code Code to run + */ +function runCodeInVm(code) { + vm.runInContext(code, vmContext); +} + +describe('node virtual machines', function () { + it('throws', function() { + const shouldNotThrow = [ + `assert.throws(function() { throw ''; }, /^$/);`, + `assert.throws(function() { throw new Error('bleepbloop'); });`, + `assert.throws(function() { throw new Error(''); });`, + `assert.throws(function() { throw new Error('swoosh'); }, /swoosh/);` + ]; + + for (const code of shouldNotThrow) { + assert.doesNotThrow( + () => { + runCodeInVm(code); + } + ); + } + }); +});