Add JsonInputArchive overrides to XDRCereal#5130
Open
drebelsky wants to merge 2 commits intostellar:masterfrom
Open
Add JsonInputArchive overrides to XDRCereal#5130drebelsky wants to merge 2 commits intostellar:masterfrom
drebelsky wants to merge 2 commits intostellar:masterfrom
Conversation
ab727fc to
92f99fb
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.
Resolves #5041. Note that the overrides are intentionally hidden behind
BUILD_TESTSbecause we shouldn't use the input overrides on production code (we should store XDR as XDR, not our custom JSON overrides that break every so often). The overloads also don't get used anywhere else, but I think it's probably still worth having them + the tests in the repo, so we can quickly write an ad-hoc tool to parse core JSON if we need to again.Testing notes
The tests right now are written to check that we can roundtrip 1000 random values of most of our XDR types. Unfortunately, due to the heavy template instantiation, compilation takes ~1 minute on my laptop. With the current configurable values (1000 random values per type, use 1 for
levelsforxdr::generator_t, limit theautocheck::arbitraryto generating things of at most size 10), the tests take about a minute to run. For reference, 1000 random values for type, limitingautocheck::arbitraryto 5, and using thexdrppautocheck::generatoroverride took ~17 seconds. Limiting the nesting depth is probably fine since we're checking the nested structures.Alternatively, we could try roundtripping (by hand or generation) a subset of the particular overloads. This has the benefit of compiling faster. However, it also requires us to actively keep the tests up-to-date (the class list can be updated more intermittently, especially since including things like
BucketEntryhelp gives provide coverage for future types).There's also a basic set of unit tests that check that the overrides get called.