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

additional BigInt JSON tests #1235

Merged
merged 3 commits into from
Oct 3, 2017
Merged

additional BigInt JSON tests #1235

merged 3 commits into from
Oct 3, 2017

Conversation

cxielarko
Copy link
Contributor

Additional JSON tests for BigInt, for JSON.stringify with a replacer function and the toJSON method.

@littledan
Copy link
Member

These tests look valid to me. While you're at it, it'd also be good to have a few tests that fully verify the ordering,

  1. toJSON
  2. replacer
  3. unwrap
  4. TypeError if it's a BigInt

(this was a recent spec change)

}

assert.sameValue(JSON.stringify(0n, replacer), "\"bigint\"");
assert.sameValue(JSON.stringify({x: 0n}, replacer), "{\"x\":\"bigint\"}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are updates to be made anyway (per @littledan's request), make all of these strings with single quotes so you can get rid of the escapes (both files)

@cxielarko
Copy link
Contributor Author

This now includes an ordering test for JSON.stringify, and also uses single quotes to avoid string escaping in some places.

@rwaldron rwaldron merged commit 184a37f into tc39:master Oct 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants