sungwy commented on code in PR #4: URL: https://github.com/apache/iceberg-verification/pull/4#discussion_r3937695648
########## dev/validate-fixtures.py: ########## @@ -0,0 +1,112 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Validate the conformance fixtures. + +Reads every table-spec/**/cases.json and checks each case: a unique id, a boolean +`valid`, an `input`, and a `decoded` when `valid` is true. Prints every problem +and exits non-zero if any are found. +""" + +import glob +import json +import sys + +CASE_GLOBS = ["table-spec/**/cases.json"] + + +def validate_case(case, where, errors, seen_ids, require_spec): + if not isinstance(case, dict): + errors.append(f"{where}: case is not a JSON object") + return + + cid = case.get("id") + if not isinstance(cid, str) or not cid: + errors.append(f"{where}: missing or empty string 'id'") + elif cid in seen_ids: + errors.append(f"{where}: duplicate id '{cid}' (first seen at {seen_ids[cid]})") + else: + seen_ids[cid] = where + + valid = case.get("valid") + if not isinstance(valid, bool): + errors.append(f"{where}: 'valid' must be a boolean") + + if "input" not in case: + errors.append(f"{where}: missing 'input'") + + if valid is True and "decoded" not in case: + errors.append(f"{where}: valid case must have 'decoded'") + if valid is False and "decoded" in case: + errors.append(f"{where}: invalid case must not have 'decoded'") Review Comment: It looks like we are being prescriptive of the input and expected values format of the fixtures. Do we anticipate that it'll be the same shape in all of our cases, or is this validation script narrowly targeting the type checks? ########## runners/go/main.go: ########## @@ -0,0 +1,307 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Command conformance-go is the reference runner that checks iceberg-go against +// the type-surface conformance fixtures. It reads every table-spec/**/cases.json, +// parses each `input` with iceberg-go's own type parser, and applies the case +// assertion contract: +// +// valid=false -> the parser must return an error +// valid=true -> parse ok and the decoded shape == decoded; a type iceberg-go +// does not model is reported UNSUPPORTED (not a failure) +// +// The process exits non-zero if any case FAILs. +package main + +import ( + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "reflect" + "sort" + + "github.com/apache/iceberg-go" +) + +// surfaceRoots are the directories under the repo root that hold cases. +var surfaceRoots = []string{"table-spec"} + +type testCase struct { + ID string `json:"id"` + Input json.RawMessage `json:"input"` + Valid bool `json:"valid"` + Decoded json.RawMessage `json:"decoded"` + Canonical *string `json:"canonical"` + source string +} + +// caseFile is the on-disk shape of a cases.json file. +type caseFile struct { + Cases []testCase `json:"cases"` +} + +// findRepoRoot walks up from start until it finds a directory containing +// table-spec/, so the runner works from any working directory. +func findRepoRoot(start string) (string, error) { + dir := start + for { + if fi, err := os.Stat(filepath.Join(dir, "table-spec")); err == nil && fi.IsDir() { + return dir, nil + } + parent := filepath.Dir(dir) + if parent == dir { + return "", fmt.Errorf("could not locate repo root (no table-spec/) above %s", start) + } + dir = parent + } +} + +// loadCases reads every cases.json under the surface roots. +func loadCases(root string) ([]testCase, error) { + var cases []testCase + for _, sr := range surfaceRoots { + base := filepath.Join(root, sr) + if _, err := os.Stat(base); os.IsNotExist(err) { + continue + } + err := filepath.WalkDir(base, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() || d.Name() != "cases.json" { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + var cf caseFile + if err := json.Unmarshal(data, &cf); err != nil { + return fmt.Errorf("%s: %w", path, err) + } + rel, _ := filepath.Rel(root, path) + for _, c := range cf.Cases { + c.source = rel + cases = append(cases, c) + } + return nil + }) + if err != nil { + return nil, err + } + } + sort.SliceStable(cases, func(i, j int) bool { return cases[i].ID < cases[j].ID }) + return cases, nil +} + +// parseType routes an Appendix-C type value (a JSON string for a primitive/geo +// type, or a JSON object for a nested type) through iceberg-go's public JSON +// parse path (NestedField.UnmarshalJSON -> typeIFace.UnmarshalJSON) by embedding +// it as the `type` of a field. +func parseType(input json.RawMessage) (iceberg.Type, error) { + fieldJSON := fmt.Sprintf(`{"id":1,"name":"f","required":true,"type":%s}`, string(input)) + var nf iceberg.NestedField + if err := json.Unmarshal([]byte(fieldJSON), &nf); err != nil { + return nil, err + } + return nf.Type, nil +} + +// decodedShape maps an iceberg-go type to the fixture's language-neutral +// `decoded` shape. Numbers are float64 to match json-decoded expectations. +// supported=false means iceberg-go has no such type. +func decodedShape(t iceberg.Type) (map[string]any, bool) { Review Comment: The need for applying language implementation specific wrapper methods to map the input and decoded values makes me want to take a step back from this approach. This type of handling will be necessary whether that be through a `iceberg-verification` based runner executed model, or through a submodule based model where each implementation chooses the surfaces they want to test. I understand why we'd want to try to keep the verification code away from the implementations, and in this repository, but I think maintaining different language implementations in a non-language repository feels like an considerable amount of overhead to me. ########## .github/workflows/conformance-nightly.yml: ########## @@ -0,0 +1,280 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Nightly conformance: run all four runners once a day, render the aggregate +# matrix into the run summary, and refresh the README status badges. Timed after +# apache/iceberg publishes its Java SNAPSHOT at 00:00 UTC, so the Java job tracks +# the moving 1.12.0-SNAPSHOT; Go/Rust/Python re-confirm their pinned releases. +# Report-only: every job is non-blocking and nothing here gates or commits. +# The per-push release lanes (conformance-*.yml) are unchanged. +name: "Conformance (Nightly)" + +on: + schedule: + - cron: '0 6 * * *' # 06:00 UTC daily, after the 00:00 UTC Java SNAPSHOT publish + workflow_dispatch: + +concurrency: + group: conformance-nightly + cancel-in-progress: false + +permissions: + contents: read + +jobs: + nightly-go: + name: iceberg-go (nightly) + runs-on: ubuntu-24.04 + continue-on-error: true + steps: + - name: Checkout fixtures + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Install Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.25.9' + cache-dependency-path: runners/go/go.sum + - name: Run type conformance + id: run + working-directory: runners/go + run: | + set +e + mkdir -p /tmp/status + out=/tmp/status/iceberg-go.txt + go mod tidy + if ! go build -o /tmp/conformance-go . ; then + echo "ERROR: go build failed (see step log)" > "$out" + echo "code=2" >> "$GITHUB_OUTPUT"; exit 0 + fi + /tmp/conformance-go 2>&1 | tee "$out" + echo "code=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" + - name: Report results (step summary + annotations) + if: always() + run: > + bash "$GITHUB_WORKSPACE/dev/ci-report.sh" + iceberg-go /tmp/status/iceberg-go.txt + nightly "${{ steps.run.outputs.code }}" + [email protected] + - name: Upload per-impl result + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: status-iceberg-go + path: /tmp/status/iceberg-go.txt + if-no-files-found: error + + nightly-python: + name: pyiceberg (nightly) + runs-on: ubuntu-24.04 + continue-on-error: true + steps: + - name: Checkout fixtures + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Install Python + uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 + with: + python-version: '3.12' + - name: Install uv + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + enable-cache: true + - name: Install pyiceberg (pinned release) + run: uv pip install --system "pyiceberg==0.12.0" + - name: Run type conformance + id: run + working-directory: runners/python + run: | + set +e + mkdir -p /tmp/status + out=/tmp/status/pyiceberg.txt + python runner.py 2>&1 | tee "$out" + echo "code=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" + - name: Report results (step summary + annotations) + if: always() + run: > + bash "$GITHUB_WORKSPACE/dev/ci-report.sh" + pyiceberg /tmp/status/pyiceberg.txt + nightly "${{ steps.run.outputs.code }}" + pyiceberg==0.12.0 + - name: Upload per-impl result + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: status-pyiceberg + path: /tmp/status/pyiceberg.txt + if-no-files-found: error + + nightly-rust: + name: iceberg-rust (nightly) + runs-on: ubuntu-24.04 + continue-on-error: true + steps: + - name: Checkout fixtures + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Setup Rust toolchain + working-directory: runners/rust + run: rustup show && rustup update stable + - name: Cache Rust artifacts + uses: Swatinem/rust-cache@63fed3e2fecf6f7b51dc6f043341b79ef82a9ae7 # v2.9.2 + with: + workspaces: runners/rust + - name: Run type conformance + id: run + working-directory: runners/rust + run: | + set +e + mkdir -p /tmp/status + out=/tmp/status/iceberg-rust.txt + if ! cargo build --quiet ; then + echo "ERROR: cargo build failed (see step log)" > "$out" + echo "code=2" >> "$GITHUB_OUTPUT"; exit 0 + fi + cargo run --quiet 2>&1 | tee "$out" + echo "code=${PIPESTATUS[0]}" >> "$GITHUB_OUTPUT" + - name: Report results (step summary + annotations) + if: always() + run: > + bash "$GITHUB_WORKSPACE/dev/ci-report.sh" + iceberg-rust /tmp/status/iceberg-rust.txt + nightly "${{ steps.run.outputs.code }}" + [email protected] + - name: Upload per-impl result + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: status-iceberg-rust + path: /tmp/status/iceberg-rust.txt + if-no-files-found: error + + nightly-java: + name: iceberg-java (nightly SNAPSHOT) + runs-on: ubuntu-24.04 + continue-on-error: true + steps: + - name: Checkout fixtures + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Install Java + uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 + with: + distribution: zulu + java-version: '17' + - name: Set up Gradle + uses: gradle/actions/setup-gradle@9c971963bec38e04b3d30dcc455b5382be2fdbfb # v6.3.0 + - name: Run type conformance + id: run + working-directory: runners/java + # installDist then run the launcher directly so the runner's own exit code + # (0 PASS / 1 FAIL / 2 ERROR) survives; `gradlew run` collapses 1 and 2. + run: | + set +e + mkdir -p /tmp/status + out=/tmp/status/iceberg-java.txt + if ! ./gradlew installDist --quiet -PicebergVersion=1.12.0-SNAPSHOT ; then Review Comment: good use of SNAPSHOT artifact here. Is there a way to deduce the current version, instead of pinning it so that it reduces the maintenance overhead? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
