-
Notifications
You must be signed in to change notification settings - Fork 522
[import-bytes] add initial tests for import bytes proposal #4648
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
base: main
Are you sure you want to change the base?
Changes from all commits
5eeb3ca
3c59ad9
4c53bb7
a55b2a2
895b983
5441e96
df2f48e
17662ce
25407a1
7d42a35
fdc288c
ac5338b
66f5f1c
4028c63
8eda568
c7ec727
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| // Copyright (C) 2025 @styfle. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| /*--- | ||
| esid: sec-create-bytes-module | ||
| description: Creates bytes module from txt file | ||
| flags: [module] | ||
| features: [import-attributes, immutable-arraybuffer, import-bytes] | ||
| includes: [compareArray.js] | ||
| ---*/ | ||
|
|
||
| import value from './bytes-from-empty_FIXTURE.bin' with { type: 'bytes' }; | ||
|
|
||
| assert(value instanceof Uint8Array); | ||
| assert(value.buffer instanceof ArrayBuffer); | ||
|
|
||
| assert.sameValue(value.length, 0); | ||
| assert.sameValue(value.buffer.byteLength, 0); | ||
| assert.sameValue(value.buffer.immutable, true); | ||
|
|
||
| assert.compareArray(Array.from(value), []); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.resize(0); | ||
| }); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.transfer(); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| // Copyright (C) 2025 @styfle. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| /*--- | ||
| esid: sec-create-bytes-module | ||
| description: Creates bytes module from js file | ||
| flags: [module] | ||
| features: [import-attributes, immutable-arraybuffer, import-bytes] | ||
| includes: [compareArray.js] | ||
| ---*/ | ||
|
|
||
| import value from './bytes-from-js_FIXTURE.js' with { type: 'bytes' }; | ||
|
|
||
| assert(value instanceof Uint8Array); | ||
| assert(value.buffer instanceof ArrayBuffer); | ||
|
|
||
| assert.sameValue(value.length, 16); | ||
| assert.sameValue(value.buffer.byteLength, 16); | ||
| assert.sameValue(value.buffer.immutable, true); | ||
|
|
||
| assert.compareArray( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fail, no? Comments won't be stripped from the JS file?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments are only here for the human reader to make it clear that each byte represents a character in the js file that was imported, in this case: var foo = 'bar'It should work just fine. Am I misunderstanding you?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By my reading, the imported byte array should contain the UTF-8 encoding of and not but this test expects the latter.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand now, thanks! You're right, I pushed commit fdc288c to remove it but then lint fails. Is there a recommended way to disable lint on the FIXTURE files?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I figured it out. I pushed ac5338b to disable license checks on files containing
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be nicer to go the other way and include the bytes of the comment here instead of messing with the lint rules.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered that but I figured I would fix it for everyone since fixture files are not source code. It seems like this was the first fixture file to be js perhaps?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the contrary, fixture files are source code and this is not the first JS fixture file. JS fixture files are used extensively in other importing tests. I think the best course of action is to keep the license header and include the bytes of the comment in the test, as annoying as that may be. (You could say that it's ridiculous to license
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I reverted those commits and pushed a change to read all the bytes 👍 Please take another look. |
||
| Array.from(value), | ||
| [ | ||
| 47, | ||
| 47, | ||
| 32, | ||
| 67, | ||
| 111, | ||
| 112, | ||
| 121, | ||
| 114, | ||
| 105, | ||
| 103, | ||
| 104, | ||
| 116, | ||
| 32, | ||
| 40, | ||
| 67, | ||
| 41, | ||
| 32, | ||
| 50, | ||
| 48, | ||
| 50, | ||
| 53, | ||
| 32, | ||
| 64, | ||
| 115, | ||
| 116, | ||
| 121, | ||
| 102, | ||
| 108, | ||
| 101, | ||
| 46, | ||
| 32, | ||
| 65, | ||
| 108, | ||
| 108, | ||
| 32, | ||
| 114, | ||
| 105, | ||
| 103, | ||
| 104, | ||
| 116, | ||
| 115, | ||
| 32, | ||
| 114, | ||
| 101, | ||
| 115, | ||
| 101, | ||
| 114, | ||
| 118, | ||
| 101, | ||
| 100, | ||
| 46, | ||
| 10, | ||
| 47, | ||
| 47, | ||
| 32, | ||
| 84, | ||
| 104, | ||
| 105, | ||
| 115, | ||
| 32, | ||
| 99, | ||
| 111, | ||
| 100, | ||
| 101, | ||
| 32, | ||
| 105, | ||
| 115, | ||
| 32, | ||
| 103, | ||
| 111, | ||
| 118, | ||
| 101, | ||
| 114, | ||
| 110, | ||
| 101, | ||
| 100, | ||
| 32, | ||
| 98, | ||
| 121, | ||
| 32, | ||
| 116, | ||
| 104, | ||
| 101, | ||
| 32, | ||
| 66, | ||
| 83, | ||
| 68, | ||
| 32, | ||
| 108, | ||
| 105, | ||
| 99, | ||
| 101, | ||
| 110, | ||
| 115, | ||
| 101, | ||
| 32, | ||
| 102, | ||
| 111, | ||
| 117, | ||
| 110, | ||
| 100, | ||
| 32, | ||
| 105, | ||
| 110, | ||
| 32, | ||
| 116, | ||
| 104, | ||
| 101, | ||
| 32, | ||
| 76, | ||
| 73, | ||
| 67, | ||
| 69, | ||
| 78, | ||
| 83, | ||
| 69, | ||
| 32, | ||
| 102, | ||
| 105, | ||
| 108, | ||
| 101, | ||
| 46, | ||
| 10, | ||
| 118, | ||
| 97, | ||
| 114, | ||
| 32, | ||
| 102, | ||
| 111, | ||
| 111, | ||
| 32, | ||
| 61, | ||
| 32, | ||
| 39, | ||
| 98, | ||
| 97, | ||
| 114, | ||
| 39, | ||
| 10 | ||
| ] | ||
| ); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.resize(0); | ||
| }); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.transfer(); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| // Copyright (C) 2025 @styfle. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| var foo = 'bar' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright (C) 2025 @styfle. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| /*--- | ||
| esid: sec-create-bytes-module | ||
| description: Creates bytes module from json file | ||
| flags: [module] | ||
| features: [import-attributes, immutable-arraybuffer, import-bytes] | ||
| includes: [compareArray.js] | ||
| ---*/ | ||
|
|
||
| import value from './bytes-from-json_FIXTURE.json' with { type: 'bytes' }; | ||
|
|
||
| assert(value instanceof Uint8Array); | ||
| assert(value.buffer instanceof ArrayBuffer); | ||
|
|
||
| assert.sameValue(value.length, 12); | ||
| assert.sameValue(value.buffer.byteLength, 12); | ||
| assert.sameValue(value.buffer.immutable, true); | ||
|
|
||
| assert.compareArray( | ||
| Array.from(value), | ||
| [ | ||
| 0x7b, // { | ||
| 0x20, // (space) | ||
| 0x22, // " | ||
| 0x61, // a | ||
| 0x22, // " | ||
| 0x3a, // : | ||
| 0x20, // (space) | ||
| 0x34, // 4 | ||
| 0x32, // 2 | ||
| 0x20, // (space) | ||
| 0x7d, // } | ||
| 0x0a, // (newline) | ||
| ] | ||
| ); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.resize(0); | ||
| }); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.transfer(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| { "a": 42 } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Copyright (C) 2025 @styfle. All rights reserved. | ||
| // This code is governed by the BSD license found in the LICENSE file. | ||
| /*--- | ||
| esid: sec-create-bytes-module | ||
| description: Creates bytes module from png file | ||
| flags: [module] | ||
| features: [import-attributes, immutable-arraybuffer, import-bytes] | ||
| includes: [compareArray.js] | ||
| ---*/ | ||
|
|
||
| import value from './bytes-from-png_FIXTURE.png' with { type: 'bytes' }; | ||
|
|
||
| assert(value instanceof Uint8Array); | ||
| assert(value.buffer instanceof ArrayBuffer); | ||
|
|
||
| assert.sameValue(value.length, 67); | ||
| assert.sameValue(value.buffer.byteLength, 67); | ||
| assert.sameValue(value.buffer.immutable, true); | ||
|
|
||
| assert.compareArray( | ||
| Array.from(value), | ||
| [ | ||
| 0x89, | ||
| 0x50, | ||
| 0x4e, | ||
| 0x47, | ||
| 0xd, | ||
| 0xa, | ||
| 0x1a, | ||
| 0xa, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0xd, | ||
| 0x49, | ||
| 0x48, | ||
| 0x44, | ||
| 0x52, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x1, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x1, | ||
| 0x1, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x37, | ||
| 0x6e, | ||
| 0xf9, | ||
| 0x24, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0xa, | ||
| 0x49, | ||
| 0x44, | ||
| 0x41, | ||
| 0x54, | ||
| 0x78, | ||
| 0x1, | ||
| 0x63, | ||
| 0x60, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x2, | ||
| 0x0, | ||
| 0x1, | ||
| 0x73, | ||
| 0x75, | ||
| 0x1, | ||
| 0x18, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x0, | ||
| 0x49, | ||
| 0x45, | ||
| 0x4e, | ||
| 0x44, | ||
| 0xae, | ||
| 0x42, | ||
| 0x60, | ||
| 0x82, | ||
| ] | ||
| ); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.resize(0); | ||
| }); | ||
|
|
||
| assert.throws(TypeError, function() { | ||
| value.buffer.transfer(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be some kind of assertion that it's immutable (not sure what that'd look like tho)
same throughout
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lines below are checking for immutable (asserts that an error is thrown on resize and also on transfer)
Let me know if there is a better way to check for immutability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly call the
immutableaccessor property https://tc39.es/proposal-immutable-arraybuffer/#sec-get-arraybuffer.prototype.immutable ?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I don't see any existing tests for
ArrayBuffer.prototype.immutable.I was basing this new test on the existing tests here:
test262/test/built-ins/ArrayBuffer/prototype/transfer/this-is-immutable-arraybuffer.js
Lines 27 to 30 in 3fd4ec2
test262/test/built-ins/ArrayBuffer/prototype/resize/this-is-immutable-arraybuffer-object.js
Lines 20 to 23 in 3fd4ec2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went ahead and added the new assertion in cafcddc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still in PR, alas.