Bug 1452706 - Make the 'expected' argument to Assert.throws/rejects required rather than optional. r?mikedeboer
MozReview-Commit-ID: 25flGw5sWga
--- a/testing/modules/Assert.jsm
+++ b/testing/modules/Assert.jsm
@@ -322,16 +322,29 @@ proto.strictEqual = function strictEqual
* (mixed) Test reference to evaluate against `actual`
* @param message (optional)
* (string) Short explanation of the expected result
*/
proto.notStrictEqual = function notStrictEqual(actual, expected, message) {
this.report(actual === expected, actual, expected, message, "!==");
};
+function checkExpectedArgument(instance, funcName, expected) {
+ if (!expected) {
+ instance.ok(false, `Error: The 'expected' argument was not supplied to Assert.${funcName}()`);
+ }
+
+ if (!instanceOf(expected, "RegExp") &&
+ typeof expected !== "function" &&
+ typeof expected !== "object") {
+ instance.ok(false,
+ `Error: The 'expected' argument to Assert.${funcName}() must be a RegExp, function or an object`);
+ }
+}
+
function expectedException(actual, expected) {
if (!actual || !expected) {
return false;
}
if (instanceOf(expected, "RegExp")) {
return expected.test(actual);
// We need to guard against the right hand parameter of "instanceof" lacking
@@ -351,52 +364,46 @@ function expectedException(actual, expec
* assert.throws(block, Error_opt, message_opt);
*
* Example:
* ```js
* // The following will verify that an error of type TypeError was thrown:
* Assert.throws(() => testBody(), TypeError);
* // The following will verify that an error was thrown with an error message matching "hello":
* Assert.throws(() => testBody(), /hello/);
- * // The following will verify that any error was thrown and will use "hello" in the test report:
- * Assert.throws(() => testBody(), "hello");
* ```
*
* @param block
* (function) Function block to evaluate and catch eventual thrown errors
* @param expected (optional)
- * (mixed) This parameter can be either a RegExp, a function, or a string. The
+ * (mixed) This parameter can be either a RegExp or a function. The
* function is either the error type's constructor, or it's a method that returns a boolean
- * that describes the test outcome. When string value is provided, it will be used as if it
- * was provided as the message parameter.
+ * that describes the test outcome.
* @param message (optional)
* (string) Short explanation of the expected result
*/
proto.throws = function(block, expected, message) {
- let actual;
+ checkExpectedArgument(this, "throws", expected);
- if (typeof expected === "string") {
- message = expected;
- expected = null;
- }
+ let actual;
try {
block();
} catch (e) {
actual = e;
}
- message = (expected && expected.name ? " (" + expected.name + ")." : ".") +
+ message = (expected.name ? " (" + expected.name + ")." : ".") +
(message ? " " + message : ".");
if (!actual) {
this.report(true, actual, expected, "Missing expected exception" + message);
}
- if ((actual && expected && !expectedException(actual, expected))) {
+ if ((actual && !expectedException(actual, expected))) {
throw actual;
}
this.report(false, expected, expected, message);
};
/**
* A promise that is expected to reject:
@@ -405,25 +412,22 @@ proto.throws = function(block, expected,
* @param promise
* (promise) A promise that is expected to reject
* @param expected (optional)
* (mixed) Test reference to evaluate against the rejection result
* @param message (optional)
* (string) Short explanation of the expected result
*/
proto.rejects = function(promise, expected, message) {
+ checkExpectedArgument(this, "rejects", expected);
return new Promise((resolve, reject) => {
- if (typeof expected === "string") {
- message = expected;
- expected = null;
- }
return promise.then(
() => this.report(true, null, expected, "Missing expected exception " + message),
err => {
- if (expected && !expectedException(err, expected)) {
+ if (!expectedException(err, expected)) {
reject(err);
return;
}
this.report(false, err, expected, message);
resolve();
}
).catch(reject);
});
--- a/testing/modules/tests/xpcshell/test_assert.js
+++ b/testing/modules/tests/xpcshell/test_assert.js
@@ -76,25 +76,25 @@ function run_test() {
"deepEqual date");
// 7.3
assert.deepEqual(/a/, /a/);
assert.deepEqual(/a/g, /a/g);
assert.deepEqual(/a/i, /a/i);
assert.deepEqual(/a/m, /a/m);
assert.deepEqual(/a/igm, /a/igm);
- assert.throws(makeBlock(assert.deepEqual, /ab/, /a/));
- assert.throws(makeBlock(assert.deepEqual, /a/g, /a/));
- assert.throws(makeBlock(assert.deepEqual, /a/i, /a/));
- assert.throws(makeBlock(assert.deepEqual, /a/m, /a/));
- assert.throws(makeBlock(assert.deepEqual, /a/igm, /a/im));
+ assert.throws(makeBlock(assert.deepEqual, /ab/, /a/), ns.Assert.AssertionError);
+ assert.throws(makeBlock(assert.deepEqual, /a/g, /a/), ns.Assert.AssertionError);
+ assert.throws(makeBlock(assert.deepEqual, /a/i, /a/), ns.Assert.AssertionError);
+ assert.throws(makeBlock(assert.deepEqual, /a/m, /a/), ns.Assert.AssertionError);
+ assert.throws(makeBlock(assert.deepEqual, /a/igm, /a/im), ns.Assert.AssertionError);
let re1 = /a/;
re1.lastIndex = 3;
- assert.throws(makeBlock(assert.deepEqual, re1, /a/));
+ assert.throws(makeBlock(assert.deepEqual, re1, /a/), ns.Assert.AssertionError);
// 7.4
assert.deepEqual(4, "4", "deepEqual == check");
assert.deepEqual(true, 1, "deepEqual == check");
assert.throws(makeBlock(assert.deepEqual, 4, "5"),
ns.Assert.AssertionError,
"deepEqual == check");
@@ -153,20 +153,20 @@ function run_test() {
}
makeBlock(thrower, ns.Assert.AssertionError);
makeBlock(thrower, ns.Assert.AssertionError);
// the basic calls work
assert.throws(makeBlock(thrower, ns.Assert.AssertionError),
ns.Assert.AssertionError, "message");
assert.throws(makeBlock(thrower, ns.Assert.AssertionError), ns.Assert.AssertionError);
- assert.throws(makeBlock(thrower, ns.Assert.AssertionError));
+ assert.throws(makeBlock(thrower, ns.Assert.AssertionError), ns.Assert.AssertionError);
// if not passing an error, catch all.
- assert.throws(makeBlock(thrower, TypeError));
+ assert.throws(makeBlock(thrower, TypeError), TypeError);
// when passing a type, only catch errors of the appropriate type
let threw = false;
try {
assert.throws(makeBlock(thrower, TypeError), ns.Assert.AssertionError);
} catch (e) {
threw = true;
assert.ok(e instanceof TypeError, "type");
@@ -178,17 +178,17 @@ function run_test() {
function ifError(err) {
if (err) {
throw err;
}
}
assert.throws(function() {
ifError(new Error("test error"));
- });
+ }, /test error/);
// make sure that validating using constructor really works
threw = false;
try {
assert.throws(
function() {
throw ({});
},
@@ -248,17 +248,17 @@ function run_test() {
// https://github.com/joyent/node/issues/2893
try {
assert.throws(function() {
ifError(null);
});
} catch (e) {
threw = true;
- assert.equal(e.message, "Missing expected exception..");
+ assert.equal(e.message, "Error: The 'expected' argument was not supplied to Assert.throws() - false == true");
}
assert.ok(threw);
// https://github.com/joyent/node/issues/5292
try {
assert.equal(1, 2);
} catch (e) {
assert.equal(e.toString().split("\n")[0], "AssertionError: 1 == 2");
@@ -350,19 +350,16 @@ add_task(async function test_rejects() {
deepEqual(ex, err, "Assert.rejects threw the original unexpected error");
}
}
// A "throwable" error that's not an actual Error().
let SomeErrorLikeThing = function() {};
// The actual tests...
- // No "expected" or "message" values supplied.
- await assert.rejects(Promise.reject(new Error("oh no")));
- await assert.rejects(Promise.reject("oh no"));
// An explicit error object:
// An instance to check against.
await assert.rejects(Promise.reject(new Error("oh no")), Error, "rejected");
// A regex to match against the message.
await assert.rejects(Promise.reject(new Error("oh no")), /oh no/, "rejected");
// Failure cases: