Bug 1355471 - Test timeout field before value's typing; r=whimboo

The error message returned when unmarshalling the timeout configuration
object with invalid input is misleading, because it checks the typing
of the value before the field name.

This patch changes Marionette to run the type assertion for the value
after each case in the switch statement has been evaluated, ensuring
that the field is valid before asserting its value.

It also adds a few unit tests to verify this behaviour.

Fixes: https://github.com/mozilla/geckodriver/issues/633
MozReview-Commit-ID: LVjTyUacD0s

--HG--
extra : rebase_source : f8a215aedfa5edf8ddbd037cae583ec07626de27

testing/marionette/session.js

@@ -53,23 +53,21 @@ session.Timeouts = class {
     let t = new session.Timeouts();
 
     for (let [typ, ms] of Object.entries(json)) {
-      assert.positiveInteger(ms);
-
       switch (typ) {
         case "implicit":
-          t.implicit = ms;
+          t.implicit = assert.positiveInteger(ms);
           break;
 
         case "script":
-          t.script = ms;
+          t.script = assert.positiveInteger(ms);
           break;
 
         case "pageLoad":
-          t.pageLoad = ms;
+          t.pageLoad = assert.positiveInteger(ms);
           break;
 
         default:
-          throw new InvalidArgumentError();
+          throw new InvalidArgumentError("Unrecognised timeout: " + typ);
       }
     }
 

testing/marionette/test_session.js

@@ -48,6 +48,43 @@ add_test(function test_Timeouts_fromJSON() {
   run_next_test();
 });
 
+add_test(function test_Timeouts_fromJSON_unrecognised_field() {
+  let json = {
+    sessionId: "foobar",
+    script: 42,
+  };
+  try {
+    session.Timeouts.fromJSON(json);
+  } catch (e) {
+    equal(e.name, InvalidArgumentError.name);
+    equal(e.message, "Unrecognised timeout: sessionId");
+  }
+
+  run_next_test();
+});
+
+add_test(function test_Timeouts_fromJSON_invalid_type() {
+  try {
+    session.Timeouts.fromJSON({script: "foobar"});
+  } catch (e) {
+    equal(e.name, InvalidArgumentError.name);
+    equal(e.message, "Expected [object String] \"foobar\" to be an integer");
+  }
+
+  run_next_test();
+});
+
+add_test(function test_Timeouts_fromJSON_bounds() {
+  try {
+    session.Timeouts.fromJSON({script: -42});
+  } catch (e) {
+    equal(e.name, InvalidArgumentError.name);
+    equal(e.message, "Expected [object Number] -42 to be >= 0");
+  }
+
+  run_next_test();
+});
+
 add_test(function test_PageLoadStrategy() {
   equal(session.PageLoadStrategy.None, "none");
   equal(session.PageLoadStrategy.Eager, "eager");