[test] Check for illegal uses of mjsunit methods

The assertThrows and assertDoesNotThrow methods expect either a
function to execute, or a string to eval. In several tests however we
accidentally passed the *result* of the statement to be tested instead
of the code.
This CL adds check to catch such error early, and removes wrong uses.
In most places, we do not need to use assertDoesNotThrow anyway,
because exceptions are handled as test failures.

Drive-by: Unify catch syntax in mjsunit.js and make sure to propagate
MjsUnitAssertionErrors correctly.

R=mathias@chromium.org

Bug: v8:8562
Change-Id: I88894a667cbe0570774f748a9a23e8a527887a49
Reviewed-on: https://chromium-review.googlesource.com/c/1439238Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59277}

test/mjsunit/mjsunit.js

@@ -213,7 +213,7 @@ var prettyPrinted;
   // TODO(neis): Remove try-catch once BigInts are enabled by default.
   try {
     BigIntPrototypeValueOf = BigInt.prototype.valueOf;
-  } catch(e) {}
+  } catch (e) {}
 
   function classOf(object) {
     // Argument must not be null or undefined.
@@ -480,14 +480,17 @@ var prettyPrinted;
     }
   };
 
+  function executeCode(code) {
+    if (typeof code === 'function')  return code();
+    if (typeof code === 'string') return eval(code);
+    failWithMessage(
+        'Given code is neither function nor string, but ' + (typeof code) +
+        ': <' + prettyPrinted(code) + '>');
+  }
 
   assertThrows = function assertThrows(code, type_opt, cause_opt) {
     try {
-      if (typeof code === 'function') {
-        code();
-      } else {
-        eval(code);
-      }
+      executeCode(code);
     } catch (e) {
       if (typeof type_opt === 'function') {
         assertInstanceof(e, type_opt);
@@ -508,11 +511,10 @@ var prettyPrinted;
     failWithMessage("Did not throw exception");
   };
 
-
   assertThrowsEquals = function assertThrowsEquals(fun, val) {
     try {
       fun();
-    } catch(e) {
+    } catch (e) {
       assertSame(val, e);
       return;
     }
@@ -533,15 +535,11 @@ var prettyPrinted;
     }
   };
 
-
-   assertDoesNotThrow = function assertDoesNotThrow(code, name_opt) {
+  assertDoesNotThrow = function assertDoesNotThrow(code, name_opt) {
     try {
-      if (typeof code === 'function') {
-        return code();
-      } else {
-        return eval(code);
-      }
+      executeCode(code);
     } catch (e) {
+      if (e instanceof MjsUnitAssertionError) throw e;
       failWithMessage("threw an exception: " + (e.message || e));
     }
   };
@@ -772,7 +770,7 @@ var prettyPrinted;
         return frame;
       });
       return "" + error.message + "\n" + ArrayPrototypeJoin.call(stack, "\n");
-    } catch(e) {};
+    } catch (e) {};
     return error.stack;
   }
 })();

test/mjsunit/regress/regress-3218530.js

@@ -33,7 +33,7 @@ var p = "floor";
 
 function test() {
   var bignumber = 31363200000;
-  assertDoesNotThrow(assertEquals(m[p](Math.round(bignumber/864E5)/7)+1, 52));
+  assertEquals(m[p](Math.round(bignumber/864E5)/7)+1, 52);
 }
 
 test();

test/mjsunit/regress/regress-687.js

@@ -29,9 +29,8 @@
 // update a accessor property to a data property using Object.defineProperty.
 
 var obj = { get value() {}, set value (v) { throw "Error";} };
-assertDoesNotThrow(
-    Object.defineProperty(obj, "value",
-                          { value: 5, writable:true, configurable: true }));
+Object.defineProperty(obj, "value",
+                      { value: 5, writable:true, configurable: true });
 var desc = Object.getOwnPropertyDescriptor(obj, "value");
 assertEquals(obj.value, 5);
 assertTrue(desc.configurable);
@@ -49,7 +48,7 @@ var proto = {
 var create = Object.create(proto);
 
 assertEquals(create.value, undefined);
-assertDoesNotThrow(create.value = 4);
+create.value = 4;
 assertEquals(create.value, 4);
 
 // These tests where provided in bug 959, but are all related to the this issue.

test/mjsunit/regress/regress-crbug-715455.js

@@ -20,6 +20,6 @@ for (var i = 0; i < test_set.length; ++i) {
   src = src.replace(/MODULE/g, "Module" + i);
   src = src.replace(/LIMIT/g, test_set[i]);
   var module = eval("(" + src + ")");
-  assertDoesNotThrow(module(this).f());
+  module(this).f();
   assertFalse(%IsAsmWasmCode(module));
 }

test/mjsunit/wasm/errors.js

@@ -26,8 +26,7 @@ function instance(bytes, imports = {}) {
 
 // instantiate should succeed but run should fail.
 function instantiateAndFailAtRuntime(bytes, imports = {}) {
-  var instance =
-      assertDoesNotThrow(new WebAssembly.Instance(module(bytes), imports));
+  var instance = new WebAssembly.Instance(module(bytes), imports);
   instance.exports.run();
 }