serde/json/test: add JSONTestSuite parsing conformance suite#30211
Merged
nvartolomei merged 2 commits intoredpanda-data:devfrom Apr 21, 2026
Merged
serde/json/test: add JSONTestSuite parsing conformance suite#30211nvartolomei merged 2 commits intoredpanda-data:devfrom
nvartolomei merged 2 commits intoredpanda-data:devfrom
Conversation
nvartolomei
requested review from
Copilot,
rockwotj and
wdberkeley
and removed request for
Copilot
Contributor
|
Can we instead pull it in via bazel instead of vendoring in a bunch of files? |
Contributor
Author
|
@rockwotj didn't want to add extra (flaky) dependency in test run. |
Contributor
You could vendor into our s3 bucket. There are already lots of things in github we depend on and we really need to have some ability to automatically mirror |
Contributor
Author
|
Why not vendor (source control) a bunch of files instead? 300 KB of blobs. |
Contributor
|
Why not vendor everything 😄 Personally I think not vendoring is more consistent, and practically speaking these will never get updated if we vendor, where I think if all you have to do is bump a sha it's much less work to update |
Collaborator
nvartolomei
force-pushed
the
nv/json-compliance
branch
from
4a683b4 to
16595c3
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds automated JSON parsing conformance coverage by integrating the upstream JSONTestSuite corpus into the existing serde/json gtest-based test suite, and refactors local jsonchecker test-case discovery into a reusable helper.
Changes:
- Extend
test_utils::get_runfile_pathto support locating runfiles from external Bazel repositories via arepoparameter. - Add a shared helper to enumerate
.jsontest vectors and use it from existing/new parameterized tests. - Add a new parameterized gtest that runs the
nst/JSONTestSuitetest_parsingcorpus via a newhttp_archive.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/v/test_utils/runfiles.h | Update runfiles helper API to accept an optional repo name and document external repo usage. |
| src/v/test_utils/runfiles.cc | Implement repo-aware Rlocation path construction. |
| src/v/serde/json/tests/test_cases.h | New helper to collect and sort .json test cases from a directory. |
| src/v/serde/json/tests/json_test_suite_test.cc | New parameterized test running JSONTestSuite test_parsing corpus. |
| src/v/serde/json/tests/json_checker_test.cc | Refactor to use shared test-case collection helper and path-based params. |
| src/v/serde/json/tests/BUILD | Add test_cases test library and new json_test_suite_test target; wire data/deps. |
| MODULE.bazel | Add nst_json_test_suite http_archive with a filegroup exposing test_parsing JSON corpus. |
Split the testdata directory scan out of json_checker_test.cc so that future parameterized JSON parser test suites can reuse it. The helper returns full file paths — callers derive the filename locally rather than re-joining the test data directory.
nvartolomei
force-pushed
the
nv/json-compliance
branch
from
16595c3 to
70b44bf
Compare
Fetch the test_parsing corpus from https://github.com/nst/JSONTestSuite (MIT, Nicolas Seriot) via a pinned http_archive and run it as a parameterized gtest. File-name prefixes encode the expected parser behavior per RFC 8259: y_ must be accepted, n_ must be rejected, i_ is implementation-defined (the parser just must not crash). The current parser passes all 318 cases with default config. get_runfile_path gains an optional repo parameter (default _main) so callers can resolve data from external Bazel repositories.
nvartolomei
force-pushed
the
nv/json-compliance
branch
from
70b44bf to
9b91321
Compare
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.
Vendor the test_parsing corpus from https://github.com/nst/JSONTestSuite
(MIT, Nicolas Seriot) and run it as a parameterized gtest. File-name
prefixes encode the expected parser behavior per RFC 8259: y_ must be
accepted, n_ must be rejected, i_ is implementation-defined (the parser
just must not crash).
The current parser passes all 318 cases with default config.
Backports Required
Release Notes