Bug 144188 – Set#forEach does not pass "key" or "set" arguments to callback.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jordan Harband]

Per https://people.mozilla.org/~jorendorff/es6-draft.html#sec-set.prototype.foreach, the Set#forEach callback should pass 3 arguments to the callback.

Currently, it passes only 1.

[Jordan Harband]

Created attachment 251617 [details]
Patch

[Build Bot]

Comment on attachment 251617 [details]
Patch

Attachment 251617 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4715717987401728

New failing tests:
js/set-foreach-calls-back-with-right-args.html

[Build Bot]

Created attachment 251620 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 251617 [details]
Patch

Attachment 251617 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6619609880854528

New failing tests:
js/set-foreach-calls-back-with-right-args.html

[Build Bot]

Created attachment 251623 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Darin Adler]

Comment on attachment 251617 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251617&action=review

> LayoutTests/js/script-tests/set-foreach-calls-back-with-right-args.js:11
> +    print(value === 'value' ? 'PASS value is "value"' : 'FAIL value is ' + value);
> +    print(key === 'value' ? 'PASS key is "value"' : 'FAIL key is ' + key);
> +    print(set === s ? 'PASS set arg is set' : 'FAIL set arg is ' + set);
> +    print(this === receiver ? 'PASS "this" is "receiver"' : 'FAIL "this" is ' + this);

Not sure where you’re expecting the "print" function to come from. I don’t  think there is such a function. Instead the code should be more like this:

    shouldBe("value", "'value'");
    shouldBe("key", "'value'");
    shouldBe("set", "s");
    shouldBe("this", "receiver");

[Jordan Harband]

Thanks for the review! I found "print" in another test, and I was unable to get `shouldBe` to pass properly. `print`, however, passed just fine locally. It appears to be like `debug`.

I'll give it another try with `shouldBe`.

(In reply to comment #6)
> Comment on attachment 251617 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251617&action=review
> 
> > LayoutTests/js/script-tests/set-foreach-calls-back-with-right-args.js:11
> > +    print(value === 'value' ? 'PASS value is "value"' : 'FAIL value is ' + value);
> > +    print(key === 'value' ? 'PASS key is "value"' : 'FAIL key is ' + key);
> > +    print(set === s ? 'PASS set arg is set' : 'FAIL set arg is ' + set);
> > +    print(this === receiver ? 'PASS "this" is "receiver"' : 'FAIL "this" is ' + this);
> 
> Not sure where you’re expecting the "print" function to come from. I don’t 
> think there is such a function. Instead the code should be more like this:
> 
>     shouldBe("value", "'value'");
>     shouldBe("key", "'value'");
>     shouldBe("set", "s");
>     shouldBe("this", "receiver");

[Darin Adler]

(In reply to comment #7)
> I found "print" in another test

Which test?

[Jordan Harband]

Created attachment 251706 [details]
Patch

[Jordan Harband]

(In reply to comment #8)
> (In reply to comment #7)
> > I found "print" in another test
> 
> Which test?

map-iterate-first-entry-is-a-hole.js. It looks like since the callback is never actually called, that the line might not be testing things correctly, and should instead throw an exception or something.

[WebKit Commit Bot]

Comment on attachment 251706 [details]
Patch

Clearing flags on attachment: 251706

Committed r183371: <http://trac.webkit.org/changeset/183371>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.