nssalian commented on code in PR #4:
URL: 
https://github.com/apache/iceberg-verification/pull/4#discussion_r3960701232


##########
.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:
   I used this directly since it was already built by the nightly and I've 
scheduled it such that it runs after the iceberg nightly. For the others it's 
the latest commit hash at the time. Let me figure out how to make the version 
more explicit.



##########
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:
   Yeah I agree this is a bit of an overhead. It doesn't disappear in the 
submodule model though. The same object-to-shape mapping still gets written, 
just in the impl's repo. The reason to keep it here for a surface like types is 
that the mapping is small and needs no impl internals, so writing it centrally 
means an impl gets checked without wiring anything up. For deep surfaces where 
the mapping needs impl internals, I agree it belongs with the impl.



##########
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:
   Good point - it shouldn't be one fixed shape. In the next revision I want to 
move the validation to JSON Schema with one schema per surface: the types 
schema owns the type-string and `decoded` shape and requires 
`clause`/`spec_ref`, and the schema surface will declare its own.



##########
table-spec/types/geospatial/cases.json:
##########
@@ -0,0 +1,52 @@
+{

Review Comment:
   Agreed type-string serialization is the finest grain and shouldn't gate 
anyone on its own. We're keeping `types` as its own surface, separate from the 
schema surface in #3 , so an impl can opt into it or skip it - the manifest 
plus the `--surface` filter in the next revision make that opt-out real, and 
the quoted-CRS and whitespace cases stay marked recommended, not required. 



-- 
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]

Reply via email to