From 37263c08b85f1f7a55d9522607a9b40f2ee651cd Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Thu, 9 May 2024 11:14:13 +0100 Subject: [PATCH] fix: support some virtual contexts in `toThrow` (#1609) * 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. * fix: support throwing of unusual errors Adds support for throwing things like `undefined`, functions, etc. * chore: upgrade check-error --- lib/chai/core/assertions.js | 28 +++++++++++++++++++++------- lib/chai/utils/index.js | 11 +++++++++++ package-lock.json | 8 ++++---- package.json | 2 +- test/assert.js | 8 ++++++++ test/virtual-machines.js | 34 ++++++++++++++++++++++++++++++++++ web-test-runner.config.js | 5 ++++- 7 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 test/virtual-machines.js diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js index 7d72b068..a1664297 100644 --- a/lib/chai/core/assertions.js +++ b/lib/chai/core/assertions.js @@ -2676,15 +2676,17 @@ 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; } - 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 + 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 ); } @@ -2760,7 +2774,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..80306f79 100644 --- a/lib/chai/utils/index.js +++ b/lib/chai/utils/index.js @@ -94,3 +94,14 @@ 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 + * @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/assert.js b/test/assert.js index c469dda1..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(''); }, ''); @@ -1644,6 +1646,12 @@ 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(''); }, /^$/); + 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'); diff --git a/test/virtual-machines.js b/test/virtual-machines.js new file mode 100644 index 00000000..da5ec416 --- /dev/null +++ b/test/virtual-machines.js @@ -0,0 +1,34 @@ +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); + } + ); + } + }); +}); 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: [