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

Update tests for TypedArrays, DataView and ArrayBuffer #669

Closed
wants to merge 3 commits into from

Conversation

leobalter
Copy link
Member


sample.setUint8(0, 0);
sample.setUint8(-0.1, 42);
assert.sameValue(typedArray[0], 42, "-0.1");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little surprising, but it's definitely to-spec.

@jugglinmike
Copy link
Contributor

Leo, hello!

This is looking good. I left a few comments in-line, but a couple others apply to multiple files:

  • DataView/prototype/*/negative-byteoffset-throws.js the info field of
    these files still documents the old semantics (pretty minor, but it confused
    me when reviewing this change set)

  • DataView/prototype/*/toindex-byteoffset.js
    These tests re-set the buffer value to 0 immediately before each assertion.
    Some of the "setup" steps introduced in this patch set the value at the wrong
    offset, making them susceptible to false positives. For example:

     sample.setInt32(0, 0);
     sample.setInt32(1.1, 42);
     assert.sameValue(sample.getInt32(1), 42, "1.1");
    
     sample.setInt32(0, 0);
     sample.setInt32(1.9, 42);
     assert.sameValue(sample.getInt32(1), 42, "1.9");
    

@leobalter
Copy link
Member Author

DataView/prototype/*/negative-byteoffset-throws.js the info field of these files still documents the old semantics (pretty minor, but it confused me when reviewing this change set)

They look fine here, or I couldn't find anything. Can you point me to an example?

@leobalter
Copy link
Member Author

17e3115 fixes the resets and the different values for {}.toString and {}.valueOf

@jugglinmike
Copy link
Contributor

They look fine here, or I couldn't find anything. Can you point me to an example?

Looks like I referenced the wrong file name--sorry for the confusion. The files
I had in mind actually match the pattern
DataView/prototype/*/detached-buffer-after-integer-byteoffset.js. For
example:

--- a/test/built-ins/DataView/prototype/getFloat32/detached-buffer-after-integer-byteoffset.js
+++ b/test/built-ins/DataView/prototype/getFloat32/detached-buffer-after-integer-byteoffset.js
@@ -17,22 +17,19 @@ info: |
   24.2.1.1 GetViewValue ( view, requestIndex, isLittleEndian, type )

   ...
   6. If numberIndex ≠ getIndex or getIndex < 0, throw a RangeError exception.
   ...
   8. Let buffer be the value of view's [[ViewedArrayBuffer]] internal slot.
   9. If IsDetachedBuffer(buffer) is true, throw a TypeError exception.
   ...
 includes: [detachArrayBuffer.js]
 ---*/

 var buffer = new ArrayBuffer(6);
 var sample = new DataView(buffer, 0);

 $DETACHBUFFER(buffer);
-assert.throws(RangeError, function() {
-  sample.getFloat32(1.1);
-});

Here, the quoted steps in GetViewValue are no longer accurate. I don't think
this would be worth correcting if it were just about the step numbers, though
(that would border on busywork). In this case, the source of the expected
RangeError has changed, and this could confuse people reading the test.

@jugglinmike
Copy link
Contributor

Merged to master at commit cfc77c8. Nice going, Leo!

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.

None yet

2 participants