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

Tests for NumericSeparatorLiteral. #1189

Merged
merged 3 commits into from Feb 8, 2018

Conversation

@leobalter
Copy link
Member

commented Aug 21, 2017

Closes #1051
Reverts #1188
Restores #1176

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2017

Cleaning up my own mess.

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2017

This ready to land label means the PR contents are reviewed. It still needs to await for stage 3.

@littledan

This comment has been minimized.

Copy link
Member

commented Aug 24, 2017

Are there any tests here on the behavior of functions like Number and parseFloat for invalid uses of _, e.g., at the beginning or end of a string?

@rwaldron rwaldron changed the title [Stage 2] Tests for NumericSeparatorLiteral. Tests for NumericSeparatorLiteral. Aug 28, 2017

@rwaldron rwaldron force-pushed the revert-1188-revert-1176-1051 branch from b8cabca to fbe73a1 Sep 19, 2017

@rwaldron

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2017

Rebased

1 similar comment
@rwaldron

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2017

Rebased

@rwaldron rwaldron force-pushed the revert-1188-revert-1176-1051 branch from fbe73a1 to 0fbe393 Nov 28, 2017

@anba

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2017

There are various tests for the Number constructor and numeric separators, but the proposal does not (or no longer?) extend Number to support numeric separators. It'd be nice to update the test, so we can use them to verify https://bugzilla.mozilla.org/show_bug.cgi?id=1421400 is correctly implemented.

@rwaldron

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2017

but the proposal does not (or no longer?) extend Number to support numeric separators

@anba an oversight, I will address this as soon as I can. Thanks for the report.

@adrianheine

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2017

btw, numeric separators reached stage3.

@littledan

This comment has been minimized.

Copy link
Member

commented Dec 20, 2017

@rwaldron It might be good to take a look at tc39/proposal-numeric-separator#25 (as well as tc39/proposal-numeric-separator#32) before removing these tests.

leobalter and others added some commits Aug 21, 2017

@leobalter leobalter force-pushed the revert-1188-revert-1176-1051 branch 2 times, most recently from 0133a59 to a2288b9 Jan 30, 2018

@leobalter leobalter force-pushed the revert-1188-revert-1176-1051 branch from a2288b9 to 48f7925 Jan 31, 2018

@leobalter

This comment has been minimized.

Copy link
Member Author

commented Feb 2, 2018

@anba I fixed the tests to match the current specs.

I still need to write something for escape sequences... tc39/proposal-numeric-separator#25

@rwaldron rwaldron merged commit 6d5a7ad into master Feb 8, 2018

2 checks passed

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

@leobalter leobalter deleted the revert-1188-revert-1176-1051 branch Feb 8, 2018

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