address my own comments

Author: michaelficarra

harness/iteratorZipUtils.js

@@ -90,8 +90,8 @@ function _assertIsNullProtoMutableObject(object, label) {
   }
 }
 
-// Assert that the `zipped` iterator results as for Iterator.zip for the first `count` elements.
-// Assumes inputs is an array of arrays of length >= count.
+// Assert that the `zipped` iterator yields the first `count` outputs of Iterator.zip.
+// Assumes `inputs` is an array of arrays, each with length >= `count`.
 // Advances `zipped` by `count` steps.
 function assertZipped(zipped, inputs, count, label) {
   // Last returned elements array.
@@ -117,14 +117,12 @@ function assertZipped(zipped, inputs, count, label) {
     assert.compareArray(value, expected, itemLabel + ": values");
 
     // Ensure value is a packed array with default data properties.
-    //
-    // This operation is destructive, so it has to happen last.
     assertIsPackedArray(value, itemLabel);
   }
 }
 
-// Assert that the `zipped` iterator results as for Iterator.zipKeyed for the first `count` elements.
-// Assumes inputs is an object whose values are arrays of length >= count.
+// Assert that the `zipped` iterator yields the first `count` outputs of Iterator.zipKeyed.
+// Assumes `inputs` is an object whose values are arrays, each with length >= `count`.
 // Advances `zipped` by `count` steps.
 function assertZippedKeyed(zipped, inputs, count, label) {
   // Last returned elements array.
@@ -154,8 +152,6 @@ function assertZippedKeyed(zipped, inputs, count, label) {
     assert.compareArray(Object.values(value), expectedValues, itemLabel + ": result object values");
 
     // Ensure resulting object is a null-prototype mutable object with default data properties.
-    //
-    // This operation is destructive, so it has to happen last.
     _assertIsNullProtoMutableObject(value, itemLabel);
   }
 }

test/built-ins/Iterator/zip/iterables-iteration-after-reading-options.js

@@ -49,3 +49,25 @@ assert.compareArray(log, [
   "get padding",
   "get iterator",
 ]);
+
+for (var mode of [undefined, "shortest", "strict"]) {
+  log.length = 0;
+
+  options = {
+    get mode() {
+      log.push("get mode");
+      return mode;
+    },
+    get padding() {
+      log.push("unexpected get padding");
+      return [];
+    }
+  };
+
+  Iterator.zip(iterables, options);
+
+  assert.compareArray(log, [
+    "get mode",
+    "get iterator",
+  ]);
+}

test/built-ins/Iterator/zip/iterables-iteration.js

@@ -70,7 +70,7 @@ var iterableReturningThrowingIterator = {
 
 // "iterables" argument must be an iterable.
 assert.throws(TypeError, function() {
-  Iterator.zip({});
+  Iterator.zip(Object.create(null));
 });
 
 // GetIteratorFlattenable accepts both iterables and iterators.
@@ -116,7 +116,7 @@ var elements = [
   makeProxyWithGetHandler("second", iterableReturningThrowingIterator),
 
   // An object without any iteration methods.
-  makeProxyWithGetHandler("third", {}),
+  makeProxyWithGetHandler("third", Object.create(null)),
 ];
 
 var elementsIter = elements.values();

test/built-ins/Iterator/zip/iterator-non-iterable.js

@@ -9,9 +9,11 @@ features: [joint-iteration]
 ---*/
 
 var invalidIterables = [
-  {},
-  { next() {}, return() {} },
-  function() {}
+  Object.create(null),
+  Object.create(null, {
+    next: { value: function(){} },
+    return: { value: function(){} },
+  }),
 ];
 
 // Throws a TypeError for invalid iterables values.

test/built-ins/Iterator/zip/non-constructible.js

@@ -12,10 +12,6 @@ description: >
 features: [joint-iteration]
 ---*/
 
-assert.throws(TypeError, () => {
-  new Iterator.zip();
-});
-
 assert.throws(TypeError, () => {
   new Iterator.zip([]);
 });

test/built-ins/Iterator/zip/options-padding.js

@@ -19,6 +19,7 @@ features: [joint-iteration]
 var validPadding = [
   undefined,
   [],
+  Object("string"),
 ];
 
 var invalidPadding = [
@@ -45,7 +46,7 @@ for (var padding of invalidPadding) {
   });
 }
 
-// Invalid padding options are ignored when mode is not "longest".
+// Invalid padding options are okay when mode is not "longest" because the padding option is not read.
 for (var padding of invalidPadding) {
   Iterator.zip([], {padding});
   Iterator.zip([], {mode: undefined, padding});

test/built-ins/Iterator/zipKeyed/iterables-iteration-after-reading-options.js

@@ -47,3 +47,25 @@ assert.compareArray(log, [
   "get padding",
   "own-keys",
 ]);
+
+for (var mode of [undefined, "shortest", "strict"]) {
+  log.length = 0;
+
+  options = {
+    get mode() {
+      log.push("get mode");
+      return mode;
+    },
+    get padding() {
+      log.push("unexpected get padding");
+      return [];
+    }
+  };
+
+  Iterator.zipKeyed(iterables, options);
+
+  assert.compareArray(log, [
+    "get mode",
+    "own-keys",
+  ]);
+}

test/built-ins/Iterator/zipKeyed/iterables-iteration-get-abrupt-completion.js

@@ -77,12 +77,40 @@ var second = {
   },
 };
 
+var symbol = {
+  next() {
+    log.push("unexpected call to next method");
+  },
+  return() {
+    log.push("unexpected call to return method");
+  },
+};
+
+var arrayIndex = {
+  next() {
+    log.push("unexpected call to next method");
+  },
+  return() {
+    // Called with the correct receiver and no arguments.
+    assert.sameValue(this, arrayIndex);
+    assert.sameValue(arguments.length, 0);
+
+    // NB: Log after above asserts, because failures aren't propagated.
+    log.push("close array-indexed iterator");
+
+    // IteratorClose ignores new exceptions when called with a Throw completion.
+    throw new Test262Error();
+  },
+};
+
 var iterables = {
+  [Symbol()]: symbol,
   first,
   second,
   get third() {
     throw new ExpectedError();
-  }
+  },
+  5: arrayIndex,
 };
 
 assert.throws(ExpectedError, function() {
@@ -93,4 +121,5 @@ assert.throws(ExpectedError, function() {
 assert.compareArray(log, [
   "close second iterator",
   "close first iterator",
+  "close array-indexed iterator",
 ]);

test/built-ins/Iterator/zipKeyed/non-constructible.js

@@ -12,10 +12,6 @@ description: >
 features: [joint-iteration]
 ---*/
 
-assert.throws(TypeError, () => {
-  new Iterator.zipKeyed();
-});
-
 assert.throws(TypeError, () => {
   new Iterator.zipKeyed({});
 });

test/built-ins/Iterator/zipKeyed/options-padding.js

@@ -18,7 +18,7 @@ features: [joint-iteration]
 
 var validPadding = [
   undefined,
-  [],
+  {},
 ];
 
 var invalidPadding = [
@@ -45,7 +45,7 @@ for (var padding of invalidPadding) {
   });
 }
 
-// Invalid padding options are ignored when mode is not "longest".
+// Invalid padding options are okay when mode is not "longest" because the padding option is not read.
 for (var padding of invalidPadding) {
   Iterator.zipKeyed({}, {padding});
   Iterator.zipKeyed({}, {mode: undefined, padding});