Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support some virtual contexts in toThrow #1609

Merged
merged 3 commits into from May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 21 additions & 7 deletions lib/chai/core/assertions.js
Expand Up @@ -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;
}

Expand All @@ -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
);
}

Expand Down Expand Up @@ -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'
}

Expand Down
11 changes: 11 additions & 0 deletions lib/chai/utils/index.js
Expand Up @@ -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]';
}
43081j marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -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"
Expand Down
8 changes: 8 additions & 0 deletions test/assert.js
Expand Up @@ -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(''); }, '');
Expand All @@ -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');
Expand Down
34 changes: 34 additions & 0 deletions 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);
}
);
}
});
});
5 changes: 4 additions & 1 deletion web-test-runner.config.js
Expand Up @@ -5,7 +5,10 @@ const commonjs = fromRollup(rollupCommonjs);

export default {
nodeResolve: true,
files: ["test/*.js"],
files: [
"test/*.js",
"!test/virtual-machines.js"
43081j marked this conversation as resolved.
Show resolved Hide resolved
],
plugins: [
commonjs({
include: [
Expand Down