Add 262 JSON tests #662
Add 262 JSON tests #662
Conversation
This commit introduces the `javy-test-macros` crate to the existing
family of crates.
This crate is intended to be a dev-dependency crate used to test javy
through the embedding API.
As of this commit, this crate exposes the bare minimum functionality to
test the implementation of JSON.{parse,stringify} introduced in
bytecodealliance#651.
This commit adds `javy-test-macros` as a development dependency in the
`javy` crate and adds 262 test for Javy's custom implementation of
`JSON.{parse,stringigy}`.
The 262 suite is pulled in as a submodule.
In order to test the custom implementation, this commit explicitly calls
`config.override_json_parse_and_stringify` when creating the default
runtime used in Javy. The override configuration was introduced in
bytecodealliance#651.
To pull-in 262 test suite.
This was added for development purposes.
There's nothing in the spec stating that the key must be a string.
Ah yeah, good question. To ensure that it's correct, we'd probably need to hook the spec test for msgpack, similar to what's done in this PR for JSON. I suspect that the in-the-wild usage for the msgpack ser/deserializer is close to none, so my preference would be to deprecate this feature instead. Also, these serializers have been in "experimental" state since I initially wrote them, until now, in which we're upgrading the status of JSON. I'll do the deprecation in a follow up though. |
There was a problem hiding this comment.
Mostly looks good to me! Just noticed the error messages not including the type of error while seeing what the error message with the unbalanced double-quote looked like.
Re non-JSON formats, I agree that the msgpack transcoding probably isn't used widely. However, we were publicly exporting the Serializer and Deserializer from the quickjs-wasm-rs crate and someone could've been using those for any format (I also don't have any evidence they were in use) but it also looks like we're not exporting them from the Javy crate so I guess we're probably fine since they wouldn't be able to continue to use the serializer or deserializer anyway. I guess if someone complains, we can figure out options to support their use case at if that happens.
Description of the change
This PR is a follow up to #651
This PR is best reviewed commit by commit.
In general, this PR introduces tests (and updates) to Javy's custom JSON.{parse,stringify} implementation to ensure compliance with the official JSON test suite. There are a couple of tests, 3 to be exact, that are marked as ignored, refer to the comments on each test for more details.
Why am I making this change?
To ensure compliance with the official JSON test suite for JavaScript.
Checklist
javy-cliandjavy-coredo not require updating CHANGELOG files.