Skip to content

[WIP] Add Java conformance runner for the types surface - #16

Draft
nssalian wants to merge 1 commit into
apache:mainfrom
nssalian:runners
Draft

nssalian wants to merge 1 commit into
apache:mainfrom
nssalian:runners

Conversation

@nssalian

@nssalian nssalian commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Note: Still iterating

First conformance runner: Java, for the table-spec/types fixtures. Rust, Python, and Go runners follow.

  • runners/java/ parses each type input with Iceberg's own parser and grades it pass / fail (the only blocking state) / advisory_fail (a failed should) / skip (an unmodeled type). Contract in runners/README.md.
  • VerifyTypesTest self-tests the grader itself, so a grader bug - not just an
    implementation divergence - fails CI.
  • CI has two lanes: a blocking build job (spotlessCheck + the self-test) on Linux and macOS, and an advisory conformance-java job that runs the fixtures against the latest iceberg-api SNAPSHOT and reports without blocking (no release carries the v3 types yet).
  • runners/README.md carries a per-language support matrix with each runner's CI status; the root README links to it. --surface and table-spec/manifest.json let a consumer run only the surfaces it implements and vendor the fixtures as a git submodule.

@laskoviymishka laskoviymishka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really glad to see the first consumer of the types fixtures, and the grading logic is in good shape — the decoded-shape comparison, should→advisory_fail, and the setup-error exit split are what I'd want every consumer to copy.

My main concern is where this lives, and I'd push for moving it into apache/iceberg itself. With the runner here, conformance can only ever be checked against a published artifact — this PR resolves the latest iceberg-api SNAPSHOT and runs advisory. So a change in Iceberg that breaks a fixture merges green, and we find out afterwards, in another repo, as an annotation nobody owns. The whole value of the fixtures is catching the divergence on the PR that introduces it, and that only works if the check runs in the implementation's own CI on every commit. We could emulate it from here with cross-repo dispatch, building Iceberg from source, composite builds and so on (the --include-build / iceberg-src notes already point that way), but that's a lot of fragile infra to get back what we'd have for free with a plain test in the main repo.

It's also what we agreed on dev@: the proposal says no harness and no per-language code here, each implementation pins this repo and asserts in its own framework with its own skip list, and the central-CI model is set aside for V1. So I'd turn VerifyTypes into a JUnit test in apache/iceberg that pins this repo at a commit (the parquet-testing shape), and keep this PR to the fixture side: the types/README.md contract wording and manifest.json, if we want it. The root README's "and the runners that exercise them" line predates this PR, but it's worth fixing in the same pass.

Wherever the grader lands, a few semantics are worth fixing before Rust/Python/Go copy them: skip comes from a hard-coded keyword allowlist (v3 types hard-fail against 1.11.0 while unknown types silently skip), an all-skip run exits 0, canonical is compared via toString() rather than real serialization, and valid:false passes on any RuntimeException. The file literally named " " at the repo root should go too — it breaks globbing and Windows checkouts, credits a dev/build-status-matrix.py that isn't in the tree, and is a cross-implementation status matrix, which the proposal lists as a non-goal (GitHub won't let me comment inline on a path that's a single space).

Comment thread README.md
deliberately. There is no central conformance gate and no pass/fail matrix
across implementations.

## Runners

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd drop this section. Runners hosted here can only test published artifacts, so conformance never gates the Iceberg commit that breaks it. Per the proposal, each implementation pins this repo and runs the fixtures in its own CI, so I'd move the Java runner into apache/iceberg as a regular test.

: "fail";
}

private static final Set<String> EXACT_KEYWORDS =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth getting right wherever this lands: skip here means "the input string isn't in the runner's allowlist", not "the implementation doesn't model this type". So against the 1.11.0 floor every v3 case (variant, geometry, timestamp_ns) hard-fails instead of skipping, while a type that's missing from the list silently skips and exits 0. The prefix arm also matches fixedfoo / decimalx.

This is really the per-consumer skip list the proposal describes, so I'd make it an explicit, reviewed list that lives with the consumer, plus a guard so an all-skip run can't go green.

+ mismatch
+ "]";
} else if (node.hasNonNull("canonical")
&& !parsed.toString().equals(node.get("canonical").asText())) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contract says the parsed type is re-serialized and must match canonical byte for byte, but this compares parsed.toString(), which isn't the wire encoding — for nested types it's struct<1: a: required int>, not Appendix-C JSON, so nested canonical cases false-fail. I'd go through SchemaParser.toJson so the write path is actually exercised, wherever the grader ends up.

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