Repository navigation
Reformat JSON test vector files, verify formatting in CI - #183
Merged
Merged
Conversation
For now we use Python since the original files were generated with the Python `json` module and its convenient for reformatting w/o affecting field order. In the future we may revisit this. The `aes_ff1_*` vector files are skipped for now since they have properties that make automated reformatting more challenging.
Using `./tools/reformat_json.py`.
Using: `sed -i 's/" : /": /g' testvectors_v1/aes_ff1_*.json`
By running `tools/reformat_json.py` and verifying no diff is produced.
davidben
approved these changes
Oct 5, 2025
davidben
left a comment
davidben
left a comment
Contributor
There was a problem hiding this comment.
LGTM
Verified the formatting by spot-checking a few and then confirming that json.dumps(json.load(...)) before and after this PR produced the same output. (Am assuming that the GHA workflow will confirm that the formatting itself matches expectations.)
botovq
approved these changes
Oct 7, 2025
Member
Author
|
Thanks for the reviews 🙇 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a simple
tools/reformat_json.pyPython script to ensure consistent and uniform formatting, and to switch away from the less conventional use of " : " separators. We useensure_ascii=Falseto preserve unicode content in comments (e.g.2⁻¹⁶inmlkem_512_test.json). CI is updated to run this script and fail if a diff is produced to prevent regression.The script avoids formatting
aes_ff1_*.jsonfiles for the reasons outlined in this comment. The delimiters in this file are fixed manually withsedin a separate commit, and we'll have to be careful to avoid regressions since it's excluded from CI checks. It seems unlikely these files will change anytime soon, so let's not get hung up on this while we can do better for the majority of files.Python is used here because unfortunately Go makes it quite difficult to accomplish the same thing without reordering fields. I'm personally of the mindset that sorting the field order would make this easier and our primary consumers are machines and not humans, but an alpha sort has the disadvantage of moving fields like
tcIdthat are currently up-front into a more or less random position. For now let's avoid this complication and use Python.Updates #162