Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests that deal with applying number format specifiers on Symbols #9488

Conversation

Projects
None yet
4 participants
@domfarolino
Copy link
Member

commented Feb 13, 2018

This PR is a follow-up to #9008 which now deals only with the application of the %s format specifier on Symbols.

This PR now includes tests for the application of %i, %d, and %f format specifiers on Symbols. The approval of this PR is pending on further discussion between implementations. At the time of creating, the spec and these tests mandate that Console implementations throw an Error when applying the aforementioned format specifiers on Symbols, though many browsers do not behave in this way currently.

@w3c-bots

This comment has been minimized.

Copy link

commented Feb 13, 2018

Build PASSED

Started: 2018-03-21 22:10:57
Finished: 2018-03-21 22:16:55

View more information about this build on:

domenic added a commit that referenced this pull request Feb 13, 2018

@domenic
Copy link
Member

left a comment

These tests LGTM and test the spec correctly, so I'm marking approved. But, we should probably delay merging until we have more clarity on the spec questions in whatwg/console#113.

@domfarolino

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2018

Sounds good, I've added a comment there to discuss two possible routes we could take with that part of the spec (one throwing on Symbol evaluation, the other not).

domfarolino added some commits Feb 13, 2018

Manually test number format specifiers applied to Symbols
Adds manual tests that apply number format specifiers on Symbols
for each Console method that uses Formatter. This also updates a
previous test of the same nature to call console.groupEnd after
calls to group() and groupCollapsed() to clean up the console output
and prevent unnecessary nesting. See whatwg/console#113.

@domfarolino domfarolino force-pushed the domfarolino:console-format-number-specifiers branch from 4a63035 to 13436eb Mar 21, 2018

@domfarolino

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2018

I've added tests to reflect the most recent spec PR following discussion in whatwg/console#113. Since we've opted to not throw when applying %i/%d/%f format specifiers on Symbols, I've added a manual test instead of an an -any.js test, so testers can see the output NaN in the developer tools.

I'll merge the spec PR and, Domenic can review/merge this upon signing off when he's back!

for (method of methods) {
console[method]("%i", Symbol.for("description"));
if (method == "group" || method == "groupCollapsed") console.groupEnd();

This comment has been minimized.

Copy link
@domfarolino

domfarolino Mar 21, 2018

Author Member

This isn't the prettiest thing ever, but I'm kinda OK with it unless someone wants me to change it.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 1, 2018

Bug 1437737 [wpt PR 9008] - Add format-specifier-applied-to-Symbol co…
…nsole tests, a=testonly

Automatic update from web-platform-testsAdd format-specifier-applied-to-Symbol console tests

Follows whatwg/console#123. See whatwg/console#113 and web-platform-tests/wpt#9488 for follow-up work.

wpt-commits: 54c8d719084e1200481df813f6e01d385ea3b07e
wpt-pr: 9008
wpt-commits: 54c8d719084e1200481df813f6e01d385ea3b07e
wpt-pr: 9008

mykmelez pushed a commit to mozilla/gecko that referenced this pull request Apr 2, 2018

Bug 1437737 [wpt PR 9008] - Add format-specifier-applied-to-Symbol co…
…nsole tests, a=testonly

Automatic update from web-platform-testsAdd format-specifier-applied-to-Symbol console tests

Follows whatwg/console#123. See whatwg/console#113 and web-platform-tests/wpt#9488 for follow-up work.

wpt-commits: 54c8d719084e1200481df813f6e01d385ea3b07e
wpt-pr: 9008
wpt-commits: 54c8d719084e1200481df813f6e01d385ea3b07e
wpt-pr: 9008

@domfarolino domfarolino merged commit a21cb02 into web-platform-tests:master Jun 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@domfarolino domfarolino deleted the domfarolino:console-format-number-specifiers branch Jun 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.