Bug 1285530 - Reps: Off by one error in grip-array max length. r=Honza
authorSteve Chung <schung@mozilla.com>
Wed, 13 Jul 2016 16:44:31 +0800
changeset 330631 b99fa4e2c82986e9462c61bc5b8c7fd4297ef362
parent 330630 c96249f34046211c1f9884479eb5e53e07ff976f
child 330632 30b6f751cdcefc35226c771650782e987c66a649
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1285530
milestone50.0a1
Bug 1285530 - Reps: Off by one error in grip-array max length. r=Honza MozReview-Commit-ID: 3rBiraUWiY0
devtools/client/shared/components/reps/array.js
devtools/client/shared/components/reps/grip-array.js
devtools/client/shared/components/test/mochitest/test_reps_array.html
devtools/client/shared/components/test/mochitest/test_reps_grip-array.html
--- a/devtools/client/shared/components/reps/array.js
+++ b/devtools/client/shared/components/reps/array.js
@@ -55,17 +55,17 @@ define(function (require, exports, modul
           items.push(ItemRep({
             object: exc,
             delim: delim,
             key: i
           }));
         }
       }
 
-      if (array.length > max + 1) {
+      if (array.length > max) {
         items.pop();
 
         let objectLink = this.props.objectLink || DOM.span;
         items.push(Caption({
           key: "more",
           object: objectLink({
             object: this.props.object
           }, "more…")
--- a/devtools/client/shared/components/reps/grip-array.js
+++ b/devtools/client/shared/components/reps/grip-array.js
@@ -82,17 +82,17 @@ define(function (require, exports, modul
           items.push(GripArrayItem(Object.assign({}, this.props, {
             object: exc,
             delim: delim,
             key: i}
           )));
         }
       }
 
-      if (array.length > max + 1) {
+      if (array.length > max) {
         items.pop();
         let objectLink = this.props.objectLink || span;
         items.push(Caption({
           key: "more",
           object: objectLink({
             object: this.props.object
           }, "more…")
         }));
--- a/devtools/client/shared/components/test/mochitest/test_reps_array.html
+++ b/devtools/client/shared/components/test/mochitest/test_reps_array.html
@@ -14,23 +14,28 @@ Test ArrayRep rep
 <pre id="test">
 <script src="head.js" type="application/javascript;version=1.8"></script>
 <script type="application/javascript;version=1.8">
 window.onload = Task.async(function* () {
   let { Rep } = browserRequire("devtools/client/shared/components/reps/rep");
   let { ArrayRep } = browserRequire("devtools/client/shared/components/reps/array");
 
   let componentUnderTest = ArrayRep;
+  const maxLength = {
+    short: 3,
+    long: 300
+  };
 
   try {
     yield testBasic();
 
     // Test property iterator
     yield testMaxProps();
-    yield testMoreThanMaxProps();
+    yield testMoreThanShortMaxProps();
+    yield testMoreThanLongMaxProps();
     yield testRecursiveArray();
 
     // Test that properties are rendered as expected by ItemRep
     yield testNested();
   } catch(e) {
     ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
   } finally {
     SimpleTest.finish();
@@ -88,36 +93,63 @@ window.onload = Task.async(function* () 
         mode: "long",
         expectedOutput: defaultOutput,
       }
     ];
 
     testRepRenderModes(modeTests, "testMaxProps", componentUnderTest, stub);
   }
 
-  function testMoreThanMaxProps() {
-    const stub = Array(302).fill("foo");
-    const defaultOutput = `["foo", "foo", "foo", more…]`;
+  function testMoreThanShortMaxProps() {
+    const stub = Array(maxLength.short + 1).fill("foo");
+    const defaultShortOutput = `[${Array(maxLength.short).fill("\"foo\"").join(", ")}, more…]`;
 
     const modeTests = [
       {
         mode: undefined,
-        expectedOutput: defaultOutput,
+        expectedOutput: defaultShortOutput,
       },
       {
         mode: "tiny",
-        expectedOutput: `[302]`,
+        expectedOutput: `[${maxLength.short + 1}]`,
       },
       {
         mode: "short",
-        expectedOutput: defaultOutput,
+        expectedOutput: defaultShortOutput,
       },
       {
         mode: "long",
-        expectedOutput: `[${Array(300).fill("\"foo\"").join(", ")}, more…]`,
+        expectedOutput: `[${Array(maxLength.short + 1).fill("\"foo\"").join(", ")}]`,
+      }
+    ];
+
+    testRepRenderModes(modeTests, "testMoreThanMaxProps", componentUnderTest, stub);
+  }
+
+  function testMoreThanLongMaxProps() {
+    const stub = Array(maxLength.long + 1).fill("foo");
+    const defaultShortOutput = `[${Array(maxLength.short).fill("\"foo\"").join(", ")}, more…]`;
+    const defaultLongOutput = `[${Array(maxLength.long).fill("\"foo\"").join(", ")}, more…]`;
+
+    const modeTests = [
+      {
+        mode: undefined,
+        expectedOutput: defaultShortOutput,
+      },
+      {
+        mode: "tiny",
+        expectedOutput: `[${maxLength.long + 1}]`,
+      },
+      {
+        mode: "short",
+        expectedOutput: defaultShortOutput,
+      },
+      {
+        mode: "long",
+        expectedOutput: defaultLongOutput,
       }
     ];
 
     testRepRenderModes(modeTests, "testMoreThanMaxProps", componentUnderTest, stub);
   }
 
   function testRecursiveArray() {
     let stub = [1];
--- a/devtools/client/shared/components/test/mochitest/test_reps_grip-array.html
+++ b/devtools/client/shared/components/test/mochitest/test_reps_grip-array.html
@@ -14,23 +14,28 @@ Test GripArray rep
 <pre id="test">
 <script src="head.js" type="application/javascript;version=1.8"></script>
 <script type="application/javascript;version=1.8">
 window.onload = Task.async(function* () {
   let { Rep } = browserRequire("devtools/client/shared/components/reps/rep");
   let { GripArray } = browserRequire("devtools/client/shared/components/reps/grip-array");
 
   let componentUnderTest = GripArray;
+  const maxLength = {
+    short: 3,
+    long: 300
+  };
 
   try {
     yield testBasic();
 
     // Test property iterator
     yield testMaxProps();
-    yield testMoreThanMaxProps();
+    yield testMoreThanShortMaxProps();
+    yield testMoreThanLongMaxProps();
     yield testRecursiveArray();
   } catch(e) {
     ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
   } finally {
     SimpleTest.finish();
   }
 
   function testBasic() {
@@ -90,38 +95,67 @@ window.onload = Task.async(function* () 
         mode: "long",
         expectedOutput: defaultOutput,
       }
     ];
 
     testRepRenderModes(modeTests, testName, componentUnderTest, getGripStub(testName));
   }
 
-  function testMoreThanMaxProps() {
-    // Test array = `["test string"…] //301 items`
-    const testName = "testMoreThanMaxProps";
+  function testMoreThanShortMaxProps() {
+    // Test array = `["test string"…] //4 items`
+    const testName = "testMoreThanShortMaxProps";
 
-    const defaultOutput = `[${Array(3).fill("\"test string\"").join(", ")}, more…]`;
+    const defaultOutput = `[${Array(maxLength.short).fill("\"test string\"").join(", ")}, more…]`;
 
     const modeTests = [
       {
         mode: undefined,
         expectedOutput: defaultOutput,
       },
       {
         mode: "tiny",
-        expectedOutput: `[302]`,
+        expectedOutput: `[${maxLength.short + 1}]`,
       },
       {
         mode: "short",
         expectedOutput: defaultOutput,
       },
       {
         mode: "long",
-        expectedOutput: `[${Array(300).fill("\"test string\"").join(", ")}, more…]`,
+        expectedOutput: `[${Array(maxLength.short + 1).fill("\"test string\"").join(", ")}]`,
+      }
+    ];
+
+    testRepRenderModes(modeTests, testName, componentUnderTest, getGripStub(testName));
+  }
+
+  function testMoreThanLongMaxProps() {
+    // Test array = `["test string"…] //301 items`
+    const testName = "testMoreThanLongMaxProps";
+
+    const defaultShortOutput = `[${Array(maxLength.short).fill("\"test string\"").join(", ")}, more…]`;
+    const defaultLongOutput = `[${Array(maxLength.long).fill("\"test string\"").join(", ")}, more…]`;
+
+    const modeTests = [
+      {
+        mode: undefined,
+        expectedOutput: defaultShortOutput,
+      },
+      {
+        mode: "tiny",
+        expectedOutput: `[${maxLength.long + 1}]`,
+      },
+      {
+        mode: "short",
+        expectedOutput: defaultShortOutput,
+      },
+      {
+        mode: "long",
+        expectedOutput: defaultLongOutput
       }
     ];
 
     testRepRenderModes(modeTests, testName, componentUnderTest, getGripStub(testName));
   }
 
   function testRecursiveArray() {
     // @TODO This is not how this feature should actually work
@@ -195,39 +229,63 @@ window.onload = Task.async(function* () 
                 "frozen": false,
                 "sealed": false,
                 "ownPropertyLength": 0
               }
             ]
           }
         };
 
-      case "testMoreThanMaxProps":
-        let grip = {
+      case "testMoreThanShortMaxProps":
+        let shortArrayGrip = {
           "type": "object",
           "class": "Array",
           "actor": "server1.conn1.obj35",
           "extensible": true,
           "frozen": false,
           "sealed": false,
           "ownPropertyLength": 4,
           "preview": {
             "kind": "ArrayLike",
-            "length": 302,
+            "length": maxLength.short + 1,
             "items": []
           }
         };
 
-        // Generate 101 properties, which is more that the maximum
-        // limit in case of the 'long' mode.
-        for (let i = 0; i < 302; i++) {
-          grip.preview.items.push("test string");
+        // Generate array grip with length 4, which is more that the maximum
+        // limit in case of the 'short' mode.
+        for (let i = 0; i < maxLength.short + 1; i++) {
+          shortArrayGrip.preview.items.push("test string");
         }
 
-        return grip;
+        return shortArrayGrip;
+
+      case "testMoreThanLongMaxProps":
+        let longArrayGrip = {
+          "type": "object",
+          "class": "Array",
+          "actor": "server1.conn1.obj35",
+          "extensible": true,
+          "frozen": false,
+          "sealed": false,
+          "ownPropertyLength": 4,
+          "preview": {
+            "kind": "ArrayLike",
+            "length": maxLength.long + 1,
+            "items": []
+          }
+        };
+
+        // Generate array grip with length 301, which is more that the maximum
+        // limit in case of the 'long' mode.
+        for (let i = 0; i < maxLength.long + 1; i++) {
+          longArrayGrip.preview.items.push("test string");
+        }
+
+        return longArrayGrip;
 
       case "testRecursiveArray":
         return {
           "type": "object",
           "class": "Array",
           "actor": "server1.conn3.obj42",
           "extensible": true,
           "frozen": false,