DataView.prototype.getBigInt64 tests #1284
Conversation
| sample.setUint8(1, 2); | ||
| sample.setUint8(2, 6); | ||
| sample.setUint8(3, 2); | ||
| sample.setUint8(4, 128); |
There was a problem hiding this comment.
Not sure why you're passing 128 instead of 0 here.
| sample.setUint8(11, 2); | ||
|
|
||
| testCoercibleToIndexZero(function (x) { | ||
| assert.sameValue(sample.getBigInt64(x), 2810815725239828481n); |
There was a problem hiding this comment.
Verified these and following tests by converting these numbers to hex in Python and seeing that they make sense. If you wanted to make these tests a little cleaner and more obvious to debug, you could use hex syntax instead.
|
|
||
| $DETACHBUFFER(buffer); | ||
|
|
||
| assert.throws(TypeError, () => sample.getBigInt64(13), "13"); |
There was a problem hiding this comment.
Not sure why the message is "13", it could be something more descriptive, like "detached DataView access should throw".
| var sample = new DataView(buffer, 0); | ||
|
|
||
| $DETACHBUFFER(buffer); | ||
| assert.throws(TypeError, () => sample.getBigInt64(0), "0"); |
There was a problem hiding this comment.
Not sure why the message is "0", it could be something more descriptive, like "detached DataView access should throw".
|
The tests overall seem correct, but they could be cleaner. Would you mind making these small changes? (We're discussing whether code reviews should block on cleanups like this, but to stick with my understanding of current conventions, I'm requesting these minor changes.) |
cxielarko
force-pushed
the
getbigint64
branch
from
682533f
to
c23f229
Compare
|
I updated the tests to use hex literals and added better error messages |
|
LGTM! |
No description provided.