Bug 1452706 - Make the 'expected' argument to Assert.throws/rejects required rather than optional. r=mikedeboer

MozReview-Commit-ID: 25flGw5sWga

--HG--
extra : rebase_source : 5212db5e4f940906646078542641f449d46a1684

testing/modules/Assert.jsm

@@ -327,6 +327,19 @@ 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;
@@ -356,27 +369,21 @@ function expectedException(actual, expected) {
  * 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();
@@ -384,14 +391,14 @@ proto.throws = function(block, expected, message) {
     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;
   }
 
@@ -410,15 +417,12 @@ proto.throws = function(block, expected, message) {
  *        (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;
         }

testing/modules/tests/xpcshell/test_assert.js

@@ -81,15 +81,15 @@ function run_test() {
   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");
@@ -158,10 +158,10 @@ function run_test() {
   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;
@@ -183,7 +183,7 @@ function run_test() {
   }
   assert.throws(function() {
     ifError(new Error("test error"));
-  });
+  }, /test error/);
 
   // make sure that validating using constructor really works
   threw = false;
@@ -253,7 +253,7 @@ function run_test() {
     });
   } 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);
 
@@ -355,9 +355,6 @@ add_task(async function test_rejects() {
   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.