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

javy-fuzz: Compare values instead of strings. #706

Merged

Conversation

saulecabrera
Copy link
Member

@saulecabrera saulecabrera commented Jul 24, 2024

This commit introduces value comparison for the json-differential target.

The motivation behind this change is to overcome the complexity of float value formatting.

JavaScript has very complex rules for Number formatting, for example, numbers that have magnitude of 10^21, independent of the sign, will be formatted via scientific notation. I was able to achieve certain format parity by writing a custom serde serializer formatter and relying on crates like ryu and lexical. Even though this worked to an extent, there are some unwritten rules to my knowledge that make it harder to achieve full formatting parity. For example, in some cases the scientific notation would include the exponent with a sign e.g., e+ or e- depending on how big the number is. In the case of lexical or ryu even though the numerical value is the same, no signs are used, which makes it harder to perform differential testing.

Having to post-process the resulting string, will most likely incur in a performance penalty that I'm not sure is worth, especially given that no issues have been reported regarding the numerical value during my testing.

Due to all these reasons, this commit switches toward value comparison rahter than format/string comparison by parsing the stringified JSON into serde_json::Value and asserting equality on top of it.

Checklist

  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli and javy-core do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

This commit introduces value comparison for the json-differential
target.

The motivation behind this change is to overcome the complexity of float
value formatting.

JavaScript has very complex rules for Number formatting, for example,
numbers that have magnitude of 10^21, independent of the sign, will be
formatted via scientific notation. I was able to achieve certain format
parity by writing a custom serde serializer formatter and relying on
crates like `ryu` and `lexical`. Even though this worked to an extent,
there are some unwritten rules to my knowledge that make it harder to
achieve full formatting parity. For example, in some cases the
scientific notation would include the exponent with a sign e.g., `e+` or
`e-` depending on how big the number is. In the case of `lexical` or
`ryu` even though the numerical value is the same, no signs are used,
which makes it harder to perform differential testing.

Having to post-process the resulting string, will most likely incur in
a performance penalty that I'm not sure is worth, especially given that
no issues have been reported regarding the numerical value during my
testing.

Due to all this reasons, this commit switches toward value comparison
rahter than format/string comparison by parsing the stringified JSON
into `serde_json::Value` and asserting equality on top of it.
@saulecabrera saulecabrera requested a review from jeffcharles July 24, 2024 18:50
@saulecabrera saulecabrera merged commit dc232b9 into bytecodealliance:main Jul 24, 2024
16 checks passed
@saulecabrera saulecabrera deleted the tidy-up-json-fuzzing branch July 24, 2024 20:58
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.

2 participants