Bug 1392323 - Correct use of pprint in action module. r?automatedtester draft
authorAndreas Tolfsen <ato@sny.no>
Mon, 21 Aug 2017 18:09:40 +0100
changeset 650509 f8e2736a2cf256f452b5e7fbe6117e89a4065a78
parent 650498 1867d7931c0a70ab90edf4aa84876525773a7139
child 727433 cd8ea738318a431691ed4208f18e073b6aa8fd41
push id75426
push userbmo:ato@sny.no
push dateTue, 22 Aug 2017 14:59:53 +0000
reviewersautomatedtester
bugs1392323
milestone57.0a1
Bug 1392323 - Correct use of pprint in action module. r?automatedtester In certain places, the Marionette action module calls error.pprint as if it is a function. pprint is a ES6 string template and should be used like pprint`${replacement}`. MozReview-Commit-ID: 29UoCNxkKa7
testing/marionette/action.js
testing/marionette/test_action.js
--- a/testing/marionette/action.js
+++ b/testing/marionette/action.js
@@ -361,17 +361,17 @@ action.PointerOrigin.get = function(obj)
   if (typeof obj == "undefined") {
     origin = this.Viewport;
   } else if (typeof obj == "string") {
     let name = capitalize(obj);
     assert.in(name, this, pprint`Unknown pointer-move origin: ${obj}`);
     origin = this[name];
   } else if (!element.isWebElementReference(obj)) {
     throw new InvalidArgumentError("Expected 'origin' to be a string or a " +
-      `web element reference, got: ${obj}`);
+      pprint`web element reference, got ${obj}`);
   }
   return origin;
 };
 
 /** Represents possible subtypes for a pointer input source. */
 action.PointerType = {
   Mouse: "mouse",
   // TODO For now, only mouse is supported
@@ -460,17 +460,17 @@ class InputState {
   static fromJson(obj) {
     let type = obj.type;
     assert.in(type, ACTIONS, pprint`Unknown action type: ${type}`);
     let name = type == "none" ? "Null" : capitalize(type);
     if (name == "Pointer") {
       if (!obj.pointerType &&
           (!obj.parameters || !obj.parameters.pointerType)) {
         throw new InvalidArgumentError(
-            pprint`Expected obj to have pointerType, got: ${obj}`);
+            pprint`Expected obj to have pointerType, got ${obj}`);
       }
       let pointerType = obj.pointerType || obj.parameters.pointerType;
       return new action.InputState[name](pointerType);
     }
     return new action.InputState[name]();
   }
 }
 
@@ -500,18 +500,20 @@ action.InputState.Key = class Key extend
    *
    * @throws {InvalidArgumentError}
    *     If |key| is not a modifier.
    */
   setModState(key, value) {
     if (key in MODIFIER_NAME_LOOKUP) {
       this[MODIFIER_NAME_LOOKUP[key]] = value;
     } else {
-      throw new InvalidArgumentError("Expected 'key' to be one of " +
-          `${Object.keys(MODIFIER_NAME_LOOKUP)}; got: ${key}`);
+      throw new InvalidArgumentError(
+          "Expected 'key' to be one of " +
+          Object.keys(MODIFIER_NAME_LOOKUP) +
+          pprint`, got ${key}`);
     }
   }
 
   /**
    * Check whether |key| is pressed.
    *
    * @param {string} key
    *     Normalized key value.
@@ -569,17 +571,17 @@ action.InputState.Null = class Null exte
  * @throws {InvalidArgumentError}
  *     If subtype is undefined or an invalid pointer type.
  */
 action.InputState.Pointer = class Pointer extends InputState {
   constructor(subtype) {
     super();
     this.pressed = new Set();
     assert.defined(subtype,
-        pprint`Expected subtype to be defined, got: ${subtype}`);
+        pprint`Expected subtype to be defined, got ${subtype}`);
     this.subtype = action.PointerType.get(subtype);
     this.x = 0;
     this.y = 0;
   }
 
   /**
    * Check whether |button| is pressed.
    *
@@ -641,17 +643,17 @@ action.InputState.Pointer = class Pointe
  *      If any parameters are undefined.
  */
 action.Action = class {
   constructor(id, type, subtype) {
     if ([id, type, subtype].includes(undefined)) {
       throw new InvalidArgumentError("Missing id, type or subtype");
     }
     for (let attr of [id, type, subtype]) {
-      assert.string(attr, pprint`Expected string, got: ${attr}`);
+      assert.string(attr, pprint`Expected string, got ${attr}`);
     }
     this.id = id;
     this.type = type;
     this.subtype = subtype;
   }
 
   toString() {
     return `[action ${this.type}]`;
@@ -694,18 +696,18 @@ action.Action = class {
     switch (item.subtype) {
       case action.KeyUp:
       case action.KeyDown:
         let key = actionItem.value;
         // TODO countGraphemes
         // TODO key.value could be a single code point like "\uE012"
         // (see rawKey) or "grapheme cluster"
         assert.string(key,
-            pprint("Expected 'value' to be a string that represents single code point " +
-                `or grapheme cluster, got: ${key}`));
+            "Expected 'value' to be a string that represents single code point " +
+            pprint`or grapheme cluster, got ${key}`);
         item.value = key;
         break;
 
       case action.PointerDown:
       case action.PointerUp:
         assert.positiveInteger(actionItem.button,
             pprint`Expected 'button' (${actionItem.button}) to be >= 0`);
         item.button = actionItem.button;
@@ -764,17 +766,18 @@ action.Chain = class extends Array {
    *     Transpose of |actions| such that actions to be performed in a
    *     single tick are grouped together.
    *
    * @throws {InvalidArgumentError}
    *     If |actions| is not an Array.
    */
   static fromJson(actions) {
     assert.array(actions,
-        pprint`Expected 'actions' to be an Array, got: ${actions}`);
+        pprint`Expected 'actions' to be an array, got ${actions}`);
+
     let actionsByTick = new action.Chain();
     //  TODO check that each actionSequence in actions refers to a
     // different input ID
     for (let actionSequence of actions) {
       let inputSourceActions = action.Sequence.fromJson(actionSequence);
       for (let i = 0; i < inputSourceActions.length; i++) {
         // new tick
         if (actionsByTick.length < (i + 1)) {
@@ -808,27 +811,29 @@ action.Sequence = class extends Array {
    *     to an |action.InputState} incompatible with |actionSequence.type|.
    *     If |actionSequence.actions| is not an Array.
    */
   static fromJson(actionSequence) {
     // used here to validate 'type' in addition to InputState type below
     let inputSourceState = InputState.fromJson(actionSequence);
     let id = actionSequence.id;
     assert.defined(id, "Expected 'id' to be defined");
-    assert.string(id, pprint`Expected 'id' to be a string, got: ${id}`);
+    assert.string(id, pprint`Expected 'id' to be a string, got ${id}`);
     let actionItems = actionSequence.actions;
-    assert.array(actionItems,
-        pprint("Expected 'actionSequence.actions' to be an Array, " +
-            `got: ${actionSequence.actions}`));
+    assert.array(
+        actionItems,
+        "Expected 'actionSequence.actions' to be an array, " +
+        pprint`got ${actionSequence.actions}`);
+
     if (!action.inputStateMap.has(id)) {
       action.inputStateMap.set(id, inputSourceState);
     } else if (!action.inputStateMap.get(id).is(inputSourceState)) {
       throw new InvalidArgumentError(
           `Expected ${id} to be mapped to ${inputSourceState}, ` +
-          `got: ${action.inputStateMap.get(id)}`);
+          `got ${action.inputStateMap.get(id)}`);
     }
     let actions = new action.Sequence();
     for (let actionItem of actionItems) {
       actions.push(action.Action.fromJson(actionSequence, actionItem));
     }
     return actions;
   }
 };
@@ -879,24 +884,26 @@ action.PointerParameters = class {
  *     If |id| is already mapped to an |action.InputState| that is
  *     not compatible with |act.type| or |pointerParams.pointerType|.
  */
 action.processPointerAction = function(id, pointerParams, act) {
   if (action.inputStateMap.has(id) &&
       action.inputStateMap.get(id).type !== act.type) {
     throw new InvalidArgumentError(
         `Expected 'id' ${id} to be mapped to InputState whose type is ` +
-        `${action.inputStateMap.get(id).type}, got: ${act.type}`);
+        action.inputStateMap.get(id).type +
+        pprint` , got ${act.type}`);
   }
   let pointerType = pointerParams.pointerType;
   if (action.inputStateMap.has(id) &&
       action.inputStateMap.get(id).subtype !== pointerType) {
     throw new InvalidArgumentError(
         `Expected 'id' ${id} to be mapped to InputState whose subtype is ` +
-        `${action.inputStateMap.get(id).subtype}, got: ${pointerType}`);
+        action.inputStateMap.get(id).subtype +
+        pprint` , got ${pointerType}`);
   }
   act.pointerType = pointerParams.pointerType;
 };
 
 /** Collect properties associated with KeyboardEvent */
 action.Key = class {
   constructor(rawKey) {
     this.key = NORMALIZED_KEY_LOOKUP[rawKey] || rawKey;
--- a/testing/marionette/test_action.js
+++ b/testing/marionette/test_action.js
@@ -348,17 +348,17 @@ add_test(function test_processInputSourc
   actionSequence.id = undefined;
   check(`actionSequence.id: ${getTypeString(actionSequence.id)}`,
       /Expected 'id' to be defined/);
   action.inputStateMap.clear();
 
   actionSequence.id = "some_id";
   actionSequence.actions = -1;
   check(`actionSequence.actions: ${getTypeString(actionSequence.actions)}`,
-      /Expected 'actionSequence.actions' to be an Array/);
+      /Expected 'actionSequence.actions' to be an array/);
   action.inputStateMap.clear();
 
   run_next_test();
 });
 
 add_test(function test_processInputSourceActionSequence() {
   let actionItem = { type: "pause", duration: 5};
   let actionSequence = {
@@ -487,17 +487,17 @@ add_test(function test_createInputState(
 });
 
 add_test(function test_extractActionChainValidation() {
   for (let actions of [-1, "a", undefined, null]) {
     let message = `actions: ${getTypeString(actions)}`;
     Assert.throws(() => action.Chain.fromJson(actions),
         InvalidArgumentError, message);
     Assert.throws(() => action.Chain.fromJson(actions),
-        /Expected 'actions' to be an Array/, message);
+        /Expected 'actions' to be an array/, message);
   }
   run_next_test();
 });
 
 add_test(function test_extractActionChainEmpty() {
   deepEqual(action.Chain.fromJson([]), []);
   run_next_test();
 });