Copilot commented on code in PR #12388: URL: https://github.com/apache/gluten/pull/12388#discussion_r3488621143
########## .github/workflows/delta_spark_ut.yml: ########## @@ -0,0 +1,682 @@ +# 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. + +# Runs Delta Lake's `spark` sbt module unit tests against a Gluten Velox bundle +# that is built from the source in this repository. The pipeline: +# +# 1. Builds the Velox/Gluten native libraries (centos-7 + vcpkg, x86_64). +# 2. Builds the Gluten Java/Scala jars and assembles the +# `gluten-velox-bundle-spark<spark>_<scala>-linux_amd64-<version>.jar` +# fat jar for Spark 4.1 + Scala 2.13 + Java 17 with the Delta profile. +# 3. Clones delta-io/delta at the requested release tag (default `v4.2.0`), +# drops the bundle jar into `spark-unified/lib/` only (NOT `spark/lib/` +# -- see setup-delta.sh for the unmanagedJars scoping rationale), +# patches Delta's `DeltaSQLCommandTest` to register the Gluten plugin, +# and runs `sbt spark/test` sharded across the matrix. +# +# Limited to Velox + x86 to keep the matrix simple, per the pipeline's purpose +# of validating Gluten changes against the latest Delta release. + +name: Delta Spark UT (Gluten) + +on: + # Reusable workflow. velox_backend_x86.yml calls this (gated on Delta-relevant + # changes) and passes the native-lib + arrow-jars artifacts it already built, + # so the expensive native C++ build is NOT duplicated. Those artifacts live in + # the CALLER's run (a called workflow runs as part of the caller run), so the + # jobs below download them by name. See velox_backend_x86.yml `delta-spark-ut`. + # + # NOTE: the `pull_request` trigger was removed so this no longer runs as its own + # workflow on PRs (which would double-run the Delta suite). velox_backend_x86.yml + # is now the single PR entry point; `workflow_dispatch` keeps manual standalone + # runs working (those build the native lib themselves -- see build-native-lib). + workflow_call: + inputs: + native_lib_artifact: + description: 'Name of the cpp/build artifact uploaded by the caller' + type: string + required: true + arrow_jars_artifact: + description: 'Name of the org.apache.arrow jars artifact uploaded by the caller' + type: string + required: true + delta_ref: + type: string + required: false + default: 'v4.2.0' + spark_version: + type: string + required: false + default: '4.1' + test_parallelism: + type: string + required: false + default: '4' + update_baseline: + type: boolean + required: false + default: false + fail_on_fixed: + type: boolean + required: false + default: true + workflow_dispatch: + inputs: + delta_ref: + description: 'delta-io/delta git ref (tag/branch/SHA) to test against' + required: true + default: 'v4.2.0' + spark_version: + description: 'Delta `-DsparkVersion` value (must match the Gluten -P profile below)' + required: true + default: '4.1' + test_parallelism: + description: 'Forked test JVMs per shard (TEST_PARALLELISM_COUNT)' + required: true + default: '4' + update_baseline: + description: 'Seed/refresh the known-failures baseline instead of enforcing it' + type: boolean + required: false + default: false + fail_on_fixed: + description: 'Fail when a baseline test now passes (keeps the baseline honest)' + type: boolean + required: false + default: true + +env: + ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true + MVN_CMD: 'build/mvn -ntp' + CCACHE_DIR: "${{ github.workspace }}/.ccache" + # Gluten profile / bundle naming for the build-gluten-bundle and + # delta-spark-test jobs. Spark 4.1 + Scala 2.13 + JDK 17 matches Delta v4.2.0's + # default Spark version (4.1.0) from project/CrossSparkVersions.scala. + GLUTEN_SPARK_PROFILE: 'spark-4.1' + GLUTEN_SCALA_PROFILE: 'scala-2.13' + GLUTEN_JAVA_PROFILE: 'java-17' + GLUTEN_BUNDLE_SPARK_VERSION: '4.1' + GLUTEN_BUNDLE_SCALA_VERSION: '2.13' + DELTA_SCALA_VERSION: '2.13.16' + # Number of shards in the delta-spark-test matrix. Must equal the length of + # the `shard` matrix below. + # + # 4 shards x TEST_PARALLELISM_COUNT=4 gives ~16-way parallelism packed into 4 + # runner jobs (4 forks each) rather than 16 single-fork jobs -- fewer concurrent + # runners for the same throughput. Sharding is by SUITE; total work + # (~1250 shard-minutes) is fixed. Each forked test JVM uses ~4G (2G heap + 2G + # off-heap), so 4 forks plus the sbt launcher sit close to the ~16G runner limit; + # this fits because the worst memory hog (DeletionVectorsSuite 2B-row) is + # force-failed in setup-delta.sh. + DELTA_NUM_SHARDS: '4' + +# No `concurrency:` here on purpose. As a reusable workflow this runs inside the +# caller's run, where `github.workflow` resolves to the CALLER's name -- a group +# keyed on it would collide with the caller's own group and, with +# cancel-in-progress, could cancel the parent run. The caller's concurrency +# already governs cancellation. (A standalone workflow_dispatch run just won't +# auto-cancel, which is fine for infrequent manual runs.) + +jobs: + build-native-lib-centos-7: + # Standalone (workflow_dispatch) only. When called by velox_backend_x86.yml + # the caller already built the native lib + arrow jars and passes them as + # inputs, so this job is skipped and the duplicate native build is avoided. + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + - name: Get Ccache + uses: actions/cache/restore@v4 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-delta-spark-ut-centos7-release-default-${{github.sha}} + restore-keys: | + ccache-delta-spark-ut-centos7-release-default + ccache-centos7-release-default + - name: Build Gluten native libraries + run: | + docker run -v $GITHUB_WORKSPACE:/work -w /work apache/gluten:vcpkg-centos-7-gcc13 bash -c " + set -e + yum install tzdata -y + df -a + cd /work + export CCACHE_DIR=/work/.ccache + export CCACHE_MAXSIZE=1G + mkdir -p /work/.ccache + ccache -sz + bash dev/ci-velox-buildstatic-centos-7.sh + ccache -s + mkdir -p /work/.m2/repository/org/apache/arrow/ + cp -r /root/.m2/repository/org/apache/arrow/* /work/.m2/repository/org/apache/arrow/ + " + - name: Save Ccache + if: always() + uses: actions/cache/save@v4 + with: + path: '${{ env.CCACHE_DIR }}' + key: ccache-delta-spark-ut-centos7-release-default-${{github.sha}} + - uses: actions/upload-artifact@v4 + with: + name: delta-spark-ut-native-lib-centos-7-${{github.sha}} + path: ./cpp/build/ + if-no-files-found: error + - uses: actions/upload-artifact@v4 + with: + name: delta-spark-ut-arrow-jars-centos-7-${{github.sha}} + path: .m2/repository/org/apache/arrow/ + if-no-files-found: error + + build-gluten-bundle: + needs: build-native-lib-centos-7 + # Run whether the native lib was built here (dispatch -> success) or provided + # by the caller (workflow_call -> build-native-lib-centos-7 skipped). + if: ${{ always() && needs.build-native-lib-centos-7.result != 'failure' && needs.build-native-lib-centos-7.result != 'cancelled' }} + runs-on: ubuntu-22.04 + container: apache/gluten:centos-9-jdk17 + steps: + - uses: actions/checkout@v4 + - name: Download native artifacts + uses: actions/download-artifact@v4 + with: + name: ${{ inputs.native_lib_artifact || format('delta-spark-ut-native-lib-centos-7-{0}', github.sha) }} + path: ./cpp/build/ + - name: Cache Maven repository + uses: actions/cache@v4 + with: + path: /root/.m2/repository + key: m2-delta-spark-ut-bundle-${{ env.GLUTEN_SPARK_PROFILE }}-${{ env.GLUTEN_SCALA_PROFILE }}-${{ hashFiles('pom.xml', '**/pom.xml') }} + restore-keys: | + m2-delta-spark-ut-bundle-${{ env.GLUTEN_SPARK_PROFILE }}-${{ env.GLUTEN_SCALA_PROFILE }}- + m2-delta-spark-ut-bundle- + - name: Download Arrow jars + uses: actions/download-artifact@v4 + with: + name: ${{ inputs.arrow_jars_artifact || format('delta-spark-ut-arrow-jars-centos-7-{0}', github.sha) }} + path: /root/.m2/repository/org/apache/arrow/ + - name: Build Gluten Velox + Delta bundle + run: | + set -euo pipefail + yum install -y java-17-openjdk-devel + export JAVA_HOME=/usr/lib/jvm/java-17-openjdk + export PATH=$JAVA_HOME/bin:$PATH + java -version + cd "$GITHUB_WORKSPACE" + # `install` (not `package`) so the gluten-delta artifact is in the local + # m2 repo before the `package/` shaded jar is built. `Dmaven.compiler.release=17` + # overrides any user settings.xml that may pin release=1.8 for Java 17 builds. + $MVN_CMD clean install \ + -P${{ env.GLUTEN_SPARK_PROFILE }} \ + -P${{ env.GLUTEN_SCALA_PROFILE }} \ + -P${{ env.GLUTEN_JAVA_PROFILE }} \ + -Pbackends-velox -Pdelta \ + -DskipTests -Dmaven.compiler.release=17 + - name: Stage bundle jar + run: | + set -euo pipefail + mkdir -p bundle-out + # Match the renamed fat jar produced by package/pom.xml's copy-fat-jar + # exec. The version part may bump (e.g. 1.7.0-SNAPSHOT -> 1.8.0-SNAPSHOT), + # so glob the version suffix. `2>/dev/null ... || true` keeps a no-match + # `ls` from aborting the step under `set -o pipefail`, so the explicit + # check below runs instead of dying with a generic "cannot access". + jar=$(ls package/target/gluten-velox-bundle-spark${{ env.GLUTEN_BUNDLE_SPARK_VERSION }}_${{ env.GLUTEN_BUNDLE_SCALA_VERSION }}-linux_amd64-*.jar 2>/dev/null | head -n 1 || true) + if [ -z "$jar" ] || [ ! -f "$jar" ]; then + echo "ERROR: Could not find Gluten bundle jar under package/target/" >&2 + ls -la package/target/ || true + exit 1 + fi + cp "$jar" bundle-out/ + ls -lh bundle-out/ + - uses: actions/upload-artifact@v4 + with: + name: delta-spark-ut-gluten-bundle-${{github.sha}} + path: bundle-out/gluten-velox-bundle-spark*_*-linux_amd64-*.jar + if-no-files-found: error + + delta-spark-test: + needs: build-gluten-bundle + # build-gluten-bundle runs via `if: always()` (its build-native-lib-centos-7 need + # is skipped on workflow_call), so this job needs an explicit condition too -- + # otherwise GitHub's transitive skip propagation, seeing the skipped + # build-native-lib-centos-7 ancestor, would skip the whole shard matrix. + if: ${{ !cancelled() && needs.build-gluten-bundle.result == 'success' }} + runs-on: ubuntu-22.04 + container: apache/gluten:centos-9-jdk17 + # 350-min safety cap. With 4 forks per shard the per-shard suites run + # 4-at-a-time, so a shard finishes well under this. + timeout-minutes: 350 + strategy: + fail-fast: false + matrix: + # Length of this list MUST equal env.DELTA_NUM_SHARDS. + shard: [0, 1, 2, 3] + env: + # Mirror Delta's spark_test.yaml env vars used by run-tests.py / + # TestParallelization.scala. + SHARD_ID: ${{ matrix.shard }} + steps: + - uses: actions/checkout@v4 + + - name: Resolve workflow inputs + id: resolve + # Every input has a default (workflow_call + workflow_dispatch), so they + # are always set; just surface them as step outputs for the steps below. + env: + DELTA_REF: ${{ inputs.delta_ref }} + SPARK_VERSION: ${{ inputs.spark_version }} + TEST_PARALLELISM: ${{ inputs.test_parallelism }} + UPDATE_BASELINE: ${{ inputs.update_baseline }} + FAIL_ON_FIXED: ${{ inputs.fail_on_fixed }} + run: | + set -euo pipefail + { + echo "delta_ref=${DELTA_REF}" + echo "spark_version=${SPARK_VERSION}" + echo "test_parallelism=${TEST_PARALLELISM}" + echo "update_baseline=${UPDATE_BASELINE}" + echo "fail_on_fixed=${FAIL_ON_FIXED}" + } | tee -a "$GITHUB_OUTPUT" + + - name: Download Gluten bundle jar + uses: actions/download-artifact@v4 + with: + name: delta-spark-ut-gluten-bundle-${{github.sha}} + path: gluten-bundle + + - name: Install minimal tools + run: | + set -euo pipefail + # apache/gluten:centos-9-jdk17 already has java-17, git, tar, a POSIX + # shell, and curl-minimal (which provides the `curl` command sbt's + # launcher needs). Install the rest of what Delta's build/sbt and the + # tests may need. We deliberately do NOT install the full `curl` + # package -- it conflicts with the pre-installed curl-minimal. + yum install -y java-17-openjdk-devel which findutils gzip python3 + export JAVA_HOME=/usr/lib/jvm/java-17-openjdk + export PATH=$JAVA_HOME/bin:$PATH + java -version + git --version + curl --version | head -n 1 + + - name: Cache sbt / Ivy / Coursier + uses: actions/cache@v4 + with: + path: | + /root/.sbt + /root/.ivy2 + /root/.cache/coursier + # Intentionally NOT keyed by ${{ matrix.shard }} -- all shards share + # the same dependency tree, so a single shared cache (with parallel + # save races resolved by GH on a first-write-wins basis) gives the + # best storage / hit-rate tradeoff. + key: delta-spark-ut-sbt-${{ steps.resolve.outputs.delta_ref }}-${{ steps.resolve.outputs.spark_version }}-${{ env.DELTA_SCALA_VERSION }} + restore-keys: | + delta-spark-ut-sbt-${{ steps.resolve.outputs.delta_ref }}-${{ steps.resolve.outputs.spark_version }}- + delta-spark-ut-sbt-${{ steps.resolve.outputs.delta_ref }}- + + - name: Clone and patch Delta + run: | + set -euo pipefail + # `2>/dev/null ... || true` keeps a no-match `ls` from aborting the step + # under `set -o pipefail`, so the explicit check below emits a clear + # error instead of a generic "cannot access". + GLUTEN_JAR=$(ls "$GITHUB_WORKSPACE"/gluten-bundle/gluten-velox-bundle-spark*_*-linux_amd64-*.jar 2>/dev/null | head -n 1 || true) + if [ -z "$GLUTEN_JAR" ] || [ ! -f "$GLUTEN_JAR" ]; then + echo "ERROR: No Gluten bundle jar found under $GITHUB_WORKSPACE/gluten-bundle/" >&2 + ls -la "$GITHUB_WORKSPACE/gluten-bundle/" || true + exit 1 + fi + echo "Using Gluten bundle: $GLUTEN_JAR" + bash "$GITHUB_WORKSPACE/.github/workflows/util/delta-spark-ut/setup-delta.sh" \ + "${{ steps.resolve.outputs.delta_ref }}" \ + "$GITHUB_WORKSPACE/delta" \ + "$GLUTEN_JAR" \ + "$GITHUB_WORKSPACE" + + - name: Run Delta spark module tests (shard ${{ matrix.shard }} / ${{ env.DELTA_NUM_SHARDS }}) + env: + NUM_SHARDS: ${{ env.DELTA_NUM_SHARDS }} + TEST_PARALLELISM_COUNT: ${{ steps.resolve.outputs.test_parallelism }} + # Required by Delta to enable testing-only code paths + # (see delta build.sbt: "Test / envVars += DELTA_TESTING -> 1"). + DELTA_TESTING: '1' + # JDK 17 + Gluten/Arrow/Netty requires extra --add-opens and the + # `io.netty.tryReflectionSetAccessible` system property; otherwise + # the forked test JVM fails with + # java.lang.UnsupportedOperationException: sun.misc.Unsafe or + # java.nio.DirectByteBuffer.<init>(long, int) not available + # as soon as Gluten's bundled Arrow allocator initializes Netty + # direct buffers. Delta's own `Test / javaOptions` (see + # project/CrossSparkVersions.scala `java17TestSettings`) sets the + # base add-opens but NOT the Netty property -- Delta's own tests + # don't load Arrow/Netty buffers in a way that triggers it. + # + # Use JAVA_TOOL_OPTIONS so the flags propagate to BOTH the sbt + # launcher JVM and the forked test JVM (sbt forks tests and the + # child inherits the parent's env). The set below mirrors + # `extraJavaTestArgs` from Gluten's own root pom.xml (the + # canonical Gluten test JVM flag set). + # + # NOTE: we deliberately do NOT put `-Xmx` here. JAVA_TOOL_OPTIONS + # is processed BEFORE the JVM command line, so Delta's explicit + # `-Xmx1024m` (set in build.sbt `Test / javaOptions`) would still + # win (last `-Xmx` wins). The forked-test-JVM heap is bumped via + # an sbt `set spark / Test / javaOptions ++= ...` command below, + # which APPENDS to Delta's own seq -- so our `-Xmx` lands AFTER + # `-Xmx1024m` and wins. + JAVA_TOOL_OPTIONS: >- + -XX:+IgnoreUnrecognizedVMOptions + --add-opens=java.base/java.lang=ALL-UNNAMED + --add-opens=java.base/java.lang.invoke=ALL-UNNAMED + --add-opens=java.base/java.lang.reflect=ALL-UNNAMED + --add-opens=java.base/java.io=ALL-UNNAMED + --add-opens=java.base/java.net=ALL-UNNAMED + --add-opens=java.base/java.nio=ALL-UNNAMED + --add-opens=java.base/java.util=ALL-UNNAMED + --add-opens=java.base/java.util.concurrent=ALL-UNNAMED + --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED + --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED + --add-opens=java.base/sun.nio.ch=ALL-UNNAMED + --add-opens=java.base/sun.nio.cs=ALL-UNNAMED + --add-opens=java.base/sun.security.action=ALL-UNNAMED + --add-opens=java.base/sun.util.calendar=ALL-UNNAMED + -Djdk.reflect.useDirectMethodHandle=false + -Dio.netty.tryReflectionSetAccessible=true + -Dfile.encoding=UTF-8 + run: | + set -euo pipefail + export JAVA_HOME=/usr/lib/jvm/java-17-openjdk + export PATH=$JAVA_HOME/bin:$PATH + cd "$GITHUB_WORKSPACE/delta" + chmod +x build/sbt + # Only run the unified `spark` sbt project, NOT `sparkGroup/test` -- + # `sparkGroup` aggregates many other projects (sparkV2, contribs, + # sharing, connect*, ...) that are out of scope for this pipeline. + # + # JVM heap layout -- two memory consumers on the ~16G runner: + # * sbt launcher JVM: -J-Xmx4G for the test compile, then forced to + # return idle memory during the (long) test phase via G1 periodic GC + # (G1PeriodicGCInterval=10s; G1PeriodicGCSystemLoadThreshold=0 so the + # busy fork doesn't suppress it; full STW via + # -G1PeriodicGCInvokesConcurrent) that uncommits to a tight free ratio Review Comment: The comment references `-G1PeriodicGCInvokesConcurrent`, but the command line below uses `-J-XX:-G1PeriodicGCInvokesConcurrent` (note the `-XX:` and the negation). Updating the comment avoids confusion when someone tries to tune or reason about the GC behavior. ########## .github/workflows/util/delta-spark-ut/compare-test-results.py: ########## @@ -0,0 +1,467 @@ +#!/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. + +"""Gate / seed / aggregate the Delta-on-Gluten unit test results. + +Running delta-io/delta's ScalaTest suite against the Gluten Velox bundle +produces many *expected* failures (Gluten does not yet support every Delta +code path). To keep the red/green signal meaningful while we fix those +failures incrementally, we maintain a committed baseline of known failing +tests (``known-failures.txt``) and compare each CI run against it. + +This script has three modes: + +``enforce`` (default, per shard) + Parse the JUnit XML produced by ``sbt spark/test`` (ScalaTest ``-u`` + reporter) and compare against the baseline: + + * regression -- a test that FAILED but is NOT in the baseline. These + fail the build: a previously-passing test just started failing. + * expected -- a test that failed and IS in the baseline. Ignored. + * fixed -- a baseline test that now PASSES. By default these also + fail the build (``--fail-on-fixed true``) so the baseline stays honest + and contributors remove entries as they fix them. + + If the baseline is empty (not yet bootstrapped) the mode automatically + degrades to ``seed`` so the first run is never spuriously red. + +``seed`` (bootstrap / ``update_baseline``) + Never fails. Just writes the current shard's failing tests so the baseline + can be (re)generated from a real run. + +``aggregate`` (final job) + Merge every shard's ``--failures-out`` / ``--ran-out`` file into a single, + sorted, ready-to-commit ``known-failures.txt`` and report stale baseline + entries (tests no longer present in any shard). + +Baseline file format (``known-failures.txt``):: + + # comment lines start with '#' + <fully.qualified.SuiteName>#<test display name> + +The suite is always a JVM class name (dot-separated, never starts with '#'), +so a line whose first non-space character is '#' is unambiguously a comment, +and the FIRST '#' after the suite separates suite from the (possibly +'#'-containing) test name. + +Only the Python standard library is used so the script runs in the bare +centos image used by the Delta UT pipeline with no ``pip install``. +""" + +import argparse +import glob +import os +import sys +import xml.etree.ElementTree as ET + +# Synthetic "test name" recorded when a whole suite aborts (e.g. beforeAll +# throws) so that the JUnit XML reports a suite-level error with no per-test +# <testcase>. Without this, a suite that used to pass but now aborts entirely +# would record zero failing testcases and the regression would be missed. +SUITE_ABORTED = "<suite aborted>" + +SEP = "#" + + +def eprint(*args, **kwargs): + print(*args, file=sys.stderr, **kwargs) + + +# --------------------------------------------------------------------------- # +# Baseline (known-failures.txt) parsing / formatting +# --------------------------------------------------------------------------- # +def format_entry(suite, test): + return "{}{}{}".format(suite, SEP, test) + + +def parse_entry(line): + """Parse a 'suite#test' line into (suite, test) or return None for blanks/comments.""" + stripped = line.strip() + if not stripped or stripped.startswith("#"): + return None + idx = stripped.find(SEP) + if idx < 0: + # No separator: treat the whole line as a suite-level entry. + return (stripped, SUITE_ABORTED) + return (stripped[:idx], stripped[idx + len(SEP) :]) + + +def load_entries(path): + """Load a set of (suite, test) tuples from a baseline/shard-list file.""" + entries = set() + if not path or not os.path.exists(path): + return entries + with open(path, "r", encoding="utf-8") as fh: + for line in fh: + parsed = parse_entry(line) + if parsed is not None: + entries.add(parsed) + return entries + + +def write_entries(path, entries, header=None): + """Write a sorted set of (suite, test) tuples to a file.""" + os.makedirs(os.path.dirname(os.path.abspath(path)) or ".", exist_ok=True) + with open(path, "w", encoding="utf-8") as fh: + if header: + for hl in header.splitlines(): + fh.write(hl.rstrip() + "\n") + for suite, test in sorted(entries): + # Defensive: collapse any stray newlines so each entry stays on one line. + safe_test = test.replace("\r", " ").replace("\n", " ") + fh.write(format_entry(suite, safe_test) + "\n") + + +# --------------------------------------------------------------------------- # +# JUnit XML parsing +# --------------------------------------------------------------------------- # +def _iter_testsuites(root): + """Yield every <testsuite> element regardless of whether the file root is + <testsuites> (wrapper) or a single <testsuite>.""" + tag = root.tag.split("}")[-1] # strip any namespace + if tag == "testsuites": + for child in root: + if child.tag.split("}")[-1] == "testsuite": + yield child + elif tag == "testsuite": + yield root + + +def _child_local_tags(elem): + return {c.tag.split("}")[-1] for c in elem} + + +def parse_reports(reports_dir): + """Walk reports_dir for JUnit XML and classify every test. + + Returns (passed, failed, skipped) sets of (suite, test) tuples. A test is + 'failed' if its <testcase> has a <failure> or <error> child, 'skipped' if + it has a <skipped> child, otherwise 'passed'. Suite-level aborts (a + <testsuite> reporting errors/failures with no failing <testcase>) are + recorded as a synthetic (suite, SUITE_ABORTED) failure. + """ + passed, failed, skipped = set(), set(), set() + + xml_files = [] + # ScalaTest's -u reporter and Maven surefire both write `TEST-<suite>.xml` + # under a `target/.../*-reports/` dir. Restrict the secondary glob to + # `target/` so we never parse Delta's own XML *test resources* (which live + # under src/test/resources and are not reports). The <testsuite>-root guard + # below is a final safety net. + for pattern in ("**/TEST-*.xml", "**/target/**/*.xml"): + xml_files.extend(glob.glob(os.path.join(reports_dir, pattern), recursive=True)) + xml_files = sorted(set(xml_files)) + + parsed_any = False + for xml_file in xml_files: + try: + tree = ET.parse(xml_file) + except ET.ParseError as exc: + eprint("WARNING: could not parse {}: {}".format(xml_file, exc)) + continue + root = tree.getroot() + root_tag = root.tag.split("}")[-1] + if root_tag not in ("testsuites", "testsuite"): + continue # not a JUnit report + + for ts in _iter_testsuites(root): + parsed_any = True + suite_name = ts.get("name") or "" + suite_has_failing_tc = False + for tc in ts: + if tc.tag.split("}")[-1] != "testcase": + continue + suite = tc.get("classname") or suite_name + name = tc.get("name") or "" + key = (suite, name) + tags = _child_local_tags(tc) + if "failure" in tags or "error" in tags: + failed.add(key) + suite_has_failing_tc = True + elif "skipped" in tags: + skipped.add(key) + else: + passed.add(key) + + # Suite-level abort: counters say something failed but no testcase + # carried the failure (the suite blew up in beforeAll/constructor). + # Record a + # synthetic entry so the regression is visible. + try: + errors = int(ts.get("errors", "0") or "0") + failures = int(ts.get("failures", "0") or "0") + except ValueError: + errors = failures = 0 + if (errors + failures) > 0 and not suite_has_failing_tc: + failed.add((suite_name, SUITE_ABORTED)) + + if not parsed_any: + eprint( + "WARNING: no JUnit <testsuite> elements found under {}".format(reports_dir) + ) + + # A test can't be both passed and failed; failure wins. Skipped only counts + # if the test was not otherwise seen (e.g. retried). + passed -= failed + skipped -= failed + skipped -= passed + return passed, failed, skipped + + +# --------------------------------------------------------------------------- # +# Reporting helpers +# --------------------------------------------------------------------------- # +def _summary_sink(): + """Return a writer that mirrors to GITHUB_STEP_SUMMARY when available.""" + path = os.environ.get("GITHUB_STEP_SUMMARY") + handle = open(path, "a", encoding="utf-8") if path else None + + def write(line=""): + print(line) + if handle: + handle.write(line + "\n") + + return write, handle + + +def _print_block(write, title, entries, limit=50): + write("") + write("### {} ({})".format(title, len(entries))) + if not entries: + return + write("") + write("```") + for i, (suite, test) in enumerate(sorted(entries)): + if i >= limit: + write("... and {} more".format(len(entries) - limit)) + break + write(format_entry(suite, test)) + write("```") + + +# --------------------------------------------------------------------------- # +# Modes +# --------------------------------------------------------------------------- # +def run_enforce(args): + baseline = load_entries(args.known_failures) + passed, failed, skipped = parse_reports(args.reports_dir) Review Comment: In `--mode enforce`, a missing/typoed `--known-failures` path silently degrades into seed mode because `load_entries()` returns an empty set when the file doesn’t exist. That can mask regressions by making the gate always pass. Consider treating a non-existent baseline file as a configuration error in enforce mode (while still allowing an existing-but-empty baseline file to seed). ########## .github/workflows/util/delta-spark-ut/known-failures.txt: ########## @@ -0,0 +1,830 @@ +# Known Delta-on-Gluten unit test failures. +# +# Baseline of delta-io/delta `spark` ScalaTest tests EXPECTED to fail under the +# Gluten Velox bundle. The Delta Spark UT (Gluten) workflow enforces this list: +# a failing test NOT listed here is a regression (fails CI); a listed test that +# now passes should be removed. Format: <fully.qualified.SuiteName>#<test name>. +# Lines starting with '#' are comments. See README.md in this directory. +# +# --------------------------------------------------------------------------- +# Baseline for the committed 4-shard x 4-fork config. Originally seeded from run +# 27490052632; refreshed from run 28318129710 (4 x 4) after the FileSourceScanLike +# test fixes (delta-io/delta #7104 + #7105). 811 known failures total. Review Comment: The header hard-codes a total failure count (currently 811). The PR description cites a different count (776), and this number will change as the baseline is refreshed, so leaving it here risks drift and confusion. Consider removing the hard-coded total (or regenerating it automatically) and relying on the aggregate job summary/artifact for the authoritative count. -- 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]
