add BigInt tests #1148

cxielarko · 2017-07-11T16:10:38Z

These are tests for the BigInt proposal

cxielarko · 2017-07-11T16:29:14Z

This patch primarily tests integration with the rest of the language (syntax, type conversions, library methods, etc.) and doesn't include detailed arithmetic tests. Tests for the typed arrays and DataViews are also missing. I'm planning to submit more arithmetic tests in a separate patch.

leobalter · 2017-07-11T16:51:13Z

it would help a lot to set a checklist - here on Test262 or in the proposal issues - of parts that need coverage. I'll need to write it anyway if not available when I find time to review so we can check what is missing to be covered in follow up patches.

…

On Tue, Jul 11, 2017 at 12:29 PM, cxielarko ***@***.***> wrote: This patch primarily tests integration with the rest of the language (syntax, type conversions, library methods, etc.) and doesn't include detailed arithmetic tests. Tests for the typed arrays and DataViews are also missing. I'm planning to submit more arithmetic tests in a separate patch. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1148 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASYkcJtvpcr5MLrzn92wLzdmVypHWYqks5sM6LbgaJpZM4OUeIO> .

littledan

These are a good start. I don't see any bugs, just room for more tests.

Generally, there's a lot more Number tests than BigInt tests in this patch. You could write a lot of tests by copying over the relevant ones and updating them for BigInt semantics.

You can also include an info: section in the tests which copies the relevant block of spec text.

littledan · 2017-07-11T18:25:12Z

test/built-ins/BigInt/asIntN.js

+features: [BigInt]
+---*/
+
+assert.sameValue(BigInt.asIntN(1, 3n), -1n);


Add some more tests for:

Passing a non-Index argument for the first argument (e.g., -1, or "foo")

Passing a non-BigInt argument for the second argument

If both are objects with valueOf methods, then the index is first (sorry, just changed this Normative: Reverse the order of casts for asIntN/asUintN proposal-bigint#52)

Testing a wider range of values to get a little more confidence in the arithmetic

Superficial things like, what's the length of the function, check that it's a writable, non-enumerable and configurable property.

littledan · 2017-07-11T18:29:48Z

test/built-ins/BigInt/constructor.js

+// This code is governed by the BSD license found in the LICENSE file.
+
+/*---
+description: BigInt constructor


You could write something here like, "if the argument to the BigInt constructor is not a safe integer, throw a RangeError."

littledan · 2017-07-11T18:30:37Z

test/built-ins/BigInt/constructor.js

+    assert.throws(RangeError, () => BigInt(-x));
+}
+assert.sameValue(BigInt(9007199254740991), 9007199254740991n);
+assert.sameValue(BigInt(-9007199254740991), -9007199254740991n);


Some more tests you can write for the constructor:

Taking a string as an argument (should parse similarly to the parseInt tests below, except there is no radix argument)

An object (should call ToPrimitive appropriately)

The length and property attributes of how it's installed on the global object, as described above.

littledan · 2017-07-11T18:34:23Z

test/built-ins/BigInt/parseInt.js

+assert.throws(SyntaxError, () => BigInt.parseInt("1", 37));
+assert.sameValue(BigInt.parseInt("0xf", 0), 0xfn);
+assert.sameValue(BigInt.parseInt("-0"), 0n);
+assert.sameValue(BigInt.parseInt(" 0@"), 0n);


Why is this suffix allowed?

littledan · 2017-07-11T18:36:23Z

test/built-ins/BigInt/parseInt.js

+assert.sameValue(BigInt.parseInt("-0"), 0n);
+assert.sameValue(BigInt.parseInt(" 0@"), 0n);
+assert.sameValue(BigInt.parseInt("kf12oikf12oikf12oi", 36),
+                 5849853453554480289462428370n);


How about adding a test that one more than this parses as a distinct BigInt?

littledan · 2017-07-11T18:58:20Z

test/language/expressions/addition/bigint-to-string.js

+features: [BigInt]
+---*/
+
+const ToString = (x) => x + "";


Test that BigInt + String works the other way around as well, and with non-empty strings, and that it also works with these things in object wrappers.

littledan · 2017-07-11T18:58:41Z

test/language/expressions/division/bigint.js

+features: [BigInt]
+---*/
+
+assert.throws(RangeError, () => 1n / 0n);


0n/0n as well?

littledan · 2017-07-11T19:12:06Z

test/language/expressions/does-not-equals/bigint.js

+assert(1n != false);
+assert(0n != NaN);
+assert(0n != Infinity);
+assert(0n != -Infinity);


Rather than just assert(), use assert.sameValue(true, ...) to be completely sure that this outputs a boolean.

Share the tests from the "equals" assertions, except asserting that the output is false

Include a test which requires some calculation to compare the BigInt to the Number

there's a lot of observation parts here that I can't get an exact reference for all of these cases combined.

https://tc39.github.io/proposal-bigint/#sec-abstract-equality-comparison

littledan · 2017-07-11T19:13:17Z

test/language/expressions/greater-than-or-equal/bigint.js

+assert(1n >= 0);
+assert(1 >= 0n);
+assert(1n >= 1);
+assert(1 >= 1n);


Include some test which requires a little bit more calculation, and include a comparison between some non-numeric types and a BigInt.

littledan · 2017-07-11T19:13:52Z

test/language/expressions/logical-not/bigint.js

+---*/
+
+assert.sameValue(!!0n, false, "Expected ToBoolean(0n) to be false");
+assert.sameValue(!!1n, true, "Expected ToBoolean(1n) to be true");


Also test this through if, ||, &&, ?:.

cxielarko · 2017-07-13T16:07:13Z

I used a checklist like the following, based on the proposal's TOC, to develop the initial tests. (It could probably be more detailed, e.g. listing which interactions between features are tested)

(List moved to issue #1056)

leobalter · 2017-08-24T18:13:09Z

I'm working on a follow up for this today.

littledan · 2017-08-29T20:49:57Z

Landed as #1198

add BigInt tests

5202fd0

littledan reviewed Jul 11, 2017

View reviewed changes

littledan mentioned this pull request Jul 12, 2017

BigInt (Stage 3) babel/proposals#2

Open

leobalter added the awaiting stage 2.7 Supports a "Stage 2" feature label Jul 13, 2017

leobalter removed the awaiting stage 2.7 Supports a "Stage 2" feature label Aug 24, 2017

leobalter mentioned this pull request Aug 24, 2017

Basic tests for BigInt #1198

Merged

littledan closed this Aug 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add BigInt tests #1148

add BigInt tests #1148

cxielarko commented Jul 11, 2017

cxielarko commented Jul 11, 2017

leobalter commented Jul 11, 2017 via email

littledan left a comment

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

leobalter Aug 24, 2017

littledan Jul 11, 2017

littledan Jul 11, 2017

cxielarko commented Jul 13, 2017 •

edited

Loading

leobalter commented Aug 24, 2017

littledan commented Aug 29, 2017

add BigInt tests #1148

add BigInt tests #1148

Conversation

cxielarko commented Jul 11, 2017

cxielarko commented Jul 11, 2017

leobalter commented Jul 11, 2017 via email

littledan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cxielarko commented Jul 13, 2017 • edited Loading

leobalter commented Aug 24, 2017

littledan commented Aug 29, 2017

cxielarko commented Jul 13, 2017 •

edited

Loading