[js-api] Add spec tests for proposed API additions #155
Conversation
This has already been updated in the upstream spec, but also needs to be fixed in proposals.
|
This PR is now updated to match the recent event to tag renaming in the spec. |
There was a problem hiding this comment.
Thanks for doing this! Sorry for the late reply. Is there any reason file names have 'tentative' in them, even if they are tentative? (Aren't all files tentative in a sense, unless it is standardized..?) Is that a rule in the spec repo?
Anyway can you wait for a little more while before merging?
| // META: script=/wasm/jsapi/assertions.js | ||
|
|
||
| function assert_type(argument) { | ||
| const tag = new WebAssembly.Tag(argument); |
There was a problem hiding this comment.
It looks some functions use two spaces while others use four spaces. What's our rule for other JS spec tests? It'd better be consistent.
There was a problem hiding this comment.
Thanks for noticing that, I think 2-space is the norm based on other tests. I've just reformatted all the files I added with prettier to be consistent with 2-space. @Ms2ger is that fine for spec test style? (would be happy to apply a different linter too if there's a recommended one)
I'm not sure exactly what the rules are, @Ms2ger would know better what the standard practice is here I think, but I think the idea was to keep it tentative until the proposal spec is merged into the main spec. The test framework docs says this about the
BTW, I just noticed it's also possible to set the whole directory as tentative. Would it be better if I did that for these tests?
That's ok with me. I guess it would make sense to wait until the JS API spec text is merged too, is there anything else it should block on? |
|
These tests are all going into WPT as well, as soon as they're merged here, so are marked as tentative until phase 5. I forgot about marking the directory as tentative; no preference either way on that. |
There was a problem hiding this comment.
Sorry for the delayed reply and thanks for your patience. Also sorry I'm not very familiar with the spec tests convention, so I might ask easy questions.
- For the usage of 'tentative': Then are other wasm proposal repos all using 'tentative' for their JS API tests until they reach Phase 5?
- How can I run these JS API tests?
| test(() => { | ||
| const argument = { parameters: [] }; | ||
| assert_throws_js(TypeError, () => WebAssembly.Tag(argument)); | ||
| }, "Calling"); |
There was a problem hiding this comment.
Why does this throw? Are we not allowed to have a tag with zero argument? I don't recall such a restriction though.
| test(() => { | ||
| const argument = new WebAssembly.Tag({ parameters: [] }); | ||
| assert_throws_js(TypeError, () => WebAssembly.Exception(argument)); | ||
| }, "Calling"); |
There was a problem hiding this comment.
The issue here is the absence of the new keyword, which leads to NewTarget being undefined, which causes an exception in step 1.2 of the "create an interface object" algorithm.
| // META: script=/wasm/jsapi/assertions.js | ||
|
|
||
| function assert_type(argument) { | ||
| const exception = new WebAssembly.Exception(argument); |
There was a problem hiding this comment.
The indentation for this file is 4 spaces, unlike other files
There was a problem hiding this comment.
For the usage of 'tentative': Then are other wasm proposal repos all using 'tentative' for their JS API tests until they reach Phase 5?
If I did the review, they should. I can try to find a place to document this as well.
How can I run these JS API tests?
Once they're in WPT, they'll be published at wpt.live (example).
| test(() => { | ||
| const argument = new WebAssembly.Tag({ parameters: [] }); | ||
| assert_throws_js(TypeError, () => WebAssembly.Exception(argument)); | ||
| }, "Calling"); |
There was a problem hiding this comment.
The issue here is the absence of the new keyword, which leads to NewTarget being undefined, which causes an exception in step 1.2 of the "create an interface object" algorithm.
| test(() => { | ||
| const argument = { parameters: [] }; | ||
| assert_throws_js(TypeError, () => WebAssembly.Tag(argument)); | ||
| }, "Calling"); |
This PR adds preliminary web-platform / JS API spec tests for the proposed exception handling API from #154 and discussed in #150.