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 2 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
Original file line number Diff line number Diff line change
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
10 changes: 10 additions & 0 deletions lib/chai/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,13 @@ 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]';
}
43081j marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions test/assert.js
Original file line number Diff line number Diff line change
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
31 changes: 31 additions & 0 deletions test/virtual-machines.js
Original file line number Diff line number Diff line change
@@ -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
43081j marked this conversation as resolved.
Show resolved Hide resolved
//`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
Original file line number Diff line number Diff line change
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