Skip to content

Test theta binary parity with Java fixtures - #532

Open
jaideeppyne wants to merge 2 commits into
apache:masterfrom
jaideeppyne:test/theta-java-binary-parity
Open

jaideeppyne wants to merge 2 commits into
apache:masterfrom
jaideeppyne:test/theta-java-binary-parity

Conversation

@jaideeppyne

Copy link
Copy Markdown
Contributor

Summary

  • deserialize the existing Java theta fixtures from their byte buffers
  • reserialize each sketch and require byte-for-byte parity
  • cover regular, compressed, and non-empty zero-retained sketches across 15 fixture cases

This turns the existing semantic compatibility test into a regression harness for the binary discrepancies investigated in #460, including flags, seed hashes, and compressed preambles.

Validation

  • mvn -t /private/tmp/jdk25-toolchains.xml -P generate_java_files -Dtest=org.apache.datasketches.theta.ThetaSketchCrossLanguageTest test in current datasketches-java
  • cmake -S . -B build -DSERDE_COMPAT=true
  • cmake --build build -j2
  • ctest --test-dir build -R ^'theta_test$' --output-on-failure (passes)
  • full CTest: all locally runnable targets pass; 10 other SerDe targets require their own Java fixture sets, which were not generated for this theta-only change

@leerho

leerho commented Oct 1, 2026

Copy link
Copy Markdown
Member

Thanks @jaideeppyne, this is a good addition: it turns the existing Java compatibility test into a byte-level regression check for the #460 fixes. I ran it locally against the Java snapshots in datasketches-tck, and all 15 cases match.

One request: the change switches these tests from the stream deserialize(is) to the byte-buffer deserialize(ptr, size), so the stream path loses its coverage against Java files. Could you deserialize both ways and check they agree?

We are planning on restructuring the C++ library for a 6.0.0 release, and as part of that we will be moving cross-language fixtures to datasketches-tck (https://github.com/apache/datasketches-tck), where each language's generators produce .sk snapshots that the other implementations consume. New cases (set-operation results, p < 1, alternate seeds, unordered) would be added as generators in datasketches-java and datasketches-cpp, with consumer tests here.
Since the test layout will change in 6.0.0, please hold off on rebuilding the matrix until we've set up the TCK structure. We'll ping you here when it's ready.

Very soon we will be creating a 5.3.0 release, so if you could respond quickly to the above request, we may be able to fit it in. In short: the #532 change now for 5.3.0, and the matrix after the restructure in master, before 6.0.0.

How does that sound?

@jaideeppyne

Copy link
Copy Markdown
Contributor Author

That sounds good. I pushed 641c3f4, which now deserializes every Java fixture through both APIs:

  • byte buffer: deserialize(bytes.data(), bytes.size())
  • stream: deserialize(std::istream&)

Each path independently reserializes to the exact source bytes, including all compressed fixtures and the non-empty zero-retained case. I ran it against the current Java snapshots from datasketches-tck: theta_test passes.

I’ll hold off on expanding the matrix until the 6.0.0 TCK structure is ready.

This branch has not been deployed

No deployments
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