test(commons): add golden fixture corpus for LongEncoding - #3146
Conversation
Lock down the current encode/decode behavior of LongEncoding as a compatibility contract: exact outputs across the full value range including the wrapping arithmetic around Long.MIN_VALUE, accepted non-canonical inputs, and the exception class for each rejected input. Exception messages are deliberately not part of the contract. The corpus is generated by running the current implementation (LongEncodingGoldenGenerator, deterministic seed) and verified two ways: LongEncodingGoldenTest replays every line against the current implementation, and regenerates the corpus in memory to check the committed file matches the generator, so the two cannot drift apart. Both tests are registered in UnitTestSuite so commons CI runs them.
dosubot
Bot
added
size:XS
|
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
|
@imbajin seeing your interest in my proposal wdyt here? |
|
@dpol1 Sorry I haven't replied to your proposal directly sooner. We've been focused on getting 1.8.0 released as soon as possible, and we're looking to take the Rust work forward in the 1.9–2.0 timeframe. As the release work wraps up, we'll discuss the proposal together with the other PMC members and follow up here and in the related issues. A Rust refactor touches quite a few areas, so we do need some additional time to research the options and confirm the scope and compatibility requirements. Thanks for the proposal and the groundwork in this PR. The community is very interested in your proposal and supportive of your involvement and this direction. As the release progresses, we'll follow up as soon as possible to exchange ideas and discuss the next steps, including through IM or Discord for quicker back-and-forth. |
- pin the corpus to LF across checkouts - preserve byte-exact generator comparisons - prevent autocrlf false failures
|
Completed six independent review lanes and pushed a small portability fix in c6cf10b: pin the golden corpus to LF in Java 11 validation passed: 19 focused tests and all 345 commons Review score: 9.0/10. No remaining actionable code findings within the six-operation contract; final-head CI is running, so approval is pending those results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3146 +/- ##
============================================
+ Coverage 39.30% 41.15% +1.84%
- Complexity 264 7217 +6953
============================================
Files 771 802 +31
Lines 65915 69393 +3478
Branches 8754 9237 +483
============================================
+ Hits 25910 28558 +2648
- Misses 37241 37572 +331
- Partials 2764 3263 +499 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Consolidate early wraps in the golden fixture Java sources. - Preserve aligned multiline arguments and wrap the regeneration command. - Keep Java tokens and golden corpus bytes unchanged.
There was a problem hiding this comment.
+1, approved. @dpol1, thank you for the compatibility groundwork in this PR.
The repository's updated AGENTS.md and .editorconfig specify a 120-column maximum for Java. I have adjusted the two new Java files in ed67283c2 accordingly, keeping multiline arguments aligned. The aim is to make the code easier for both people and LLMs to read and work on; 120 is an upper limit, not a target every line must reach. The formatting change preserves the Java tokens and corpus bytes; all 19 focused tests and the full clean compile passed.
If useful, a separate formatting-only PR can apply this convention more broadly across the repository, starting with the core code and keeping functional changes separate.
We very much welcome your continued work on testing, correctness, compatibility fixtures, and the foundations for a future Rust refactor in #3145. This groundwork is important to the community regardless of which components eventually move to Rust. We would be glad to see you build deeper expertise and take an ongoing ownership role in this area, with other contributors participating in the design and reviews.
2 participants
Visual summary
Purpose of the PR
LongEncodingencodes ids and sortable keys, so its exact behavior is a compatibility contract, including the intentional overflow wrapping aroundLong.MIN_VALUEand inputs the decoder accepts but the encoder never produces. Today that contract exists only implicitly in the implementation. This PR pins it in a committed, implementation-independent corpus that any port or refactor can be checked against line by line.Main Changes
LongEncodingGoldenGenerator(test scope): produces the corpus by running the current implementation and recording, for each input, the exact output or the exception class. Deterministic: fixed value sets plus a fixed random seed, duplicate lines dropped.longencoding-golden.txt(test resource): 3940 cases, plain ASCII, tab separated with\uXXXXescapes. Covers full-range round trips includingLong.MIN_VALUE/MAX_VALUE, non-canonical decode inputs (00decodes to MIN while encode produces010), arbitrary UTF-16 decode input including surrogates, the error taxonomy (IllegalArgumentExceptionvsNumberFormatExceptionvs NPE), and length limits.LongEncodingGoldenTest: two checks. It replays every corpus line against the implementation, and it regenerates the corpus in memory to assert the committed file matches the generator output exactly, so generator and resource cannot drift apart.UnitTestSuiteso commons CI executes them.Review guide: the review surface is the generator (case selection, ~370 lines), the corpus format documented in the file header, and the two-way check mechanics. The corpus file is generated output verified by
testCorpusMatchesGeneratorand does not need line-by-line review.Contract boundaries: six operations (
encodeSortable/decodeSortable,encodeB64/decodeB64,encodeSignedB64/decodeSignedB64). Exact output strings and decoded values are contract; the exception class is contract, the exception message deliberately is not. No production code changes.Verifying these changes
mvn test -pl hugegraph-commons/hugegraph-common -Dtest=UnitTestSuite -DskipCommonsTests=falseon JDK 11 (the commons CI toolchain): 345 tests, the only errors are 2 environment NPEs inMachineInfoTestthat reproduce identically on a clean master checkout in the same environment.apache-rat:check,checkstyle:checkandgit diff --checkare clean.Does this PR potentially affect the following parts?
Documentation Status
Doc - No Need(user docs unaffected; the technical contract is documented in [Proposal] Java compatibility fixtures and PoC selection matrix for the Rust modernization groundwork #3145, the corpus file header, and this PR)