Skip to content

CI: restore test execution for core and Spark modules - #266

Open
cbb330 wants to merge 1 commit into
linkedin:openhouse-1.2.0from
cbb330:chbush/ci-restore-tests
Open

CI: restore test execution for core and Spark modules#266
cbb330 wants to merge 1 commit into
linkedin:openhouse-1.2.0from
cbb330:chbush/ci-restore-tests

Conversation

@cbb330

@cbb330 cbb330 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Problem

Spark and Java CI have not executed any tests since #173, which added -x test -x integrationTest to both workflows. A green check on this fork currently means "it compiles."

No workflow runs :iceberg-api:test, :iceberg-core:test, :iceberg-orc:test, or :iceberg-data:test:

Workflow Coverage today
java-ci build -x test — compile only
spark-ci -x test -x integrationTest
flink-ci :iceberg-flink:*:check only
hive-ci hive projects only
delta-conversion-ci :iceberg-delta-lake only, Spark 3.3
api-binary-compatibility revapi only

Gradle's :project:check runs only that project's test task — dependencies are compiled, never tested.

Why it happened

#173 described itself as "minor changes to match 1.0.x branch," but the match was partial. On li-1.0.x, java-ci.yml had two jobs:

  • core-tests./gradlew check ... -Pquick=true -x javadoc, ran tests, uploaded test logs on failure
  • build-checksbuild -x test -x javadoc -x integrationTest, deliberately compile-only

-x test is correct by design in build-checks, because core-tests supplied the coverage. #173 kept build-checks and deleted core-tests, so the exclusion outlived its justification. It then propagated the same flag into spark-ci.yml, which li-1.0.x never had — 1.0.x ran the Spark suites. The same commit also removed the failure-log upload steps, consistent with knowing tests no longer ran.

Flink kept its tests only because flink-ci.yml wasn't in that diff. The asymmetry is file scope, not policy.

Changes

  • java-ci: new core-tests job over api, core, common, orc, parquet, arrow, data on JVM 8 and 11.
  • spark-ci: drop -x test; restore the failure-log upload Update spark-ci and java-ci. yml #173 removed.
  • both: fetch-depth: 0 so generateGitProperties can populate iceberg-build.properties — without it TestIcebergBuild fails on unknown version/commit.
  • release now depends on core-tests as well as build-checks.

integrationTest stays excluded — it depends on shadowJar and is a separate concern. Happy to add it as a follow-up.

One thing worth reviewing

The li-1.0.x command can't be reused verbatim. settings.gradle:128 unconditionally includes the Spark 3.1 projects for this backport branch, but their dependencies are only wired when sparkVersions contains 3.1. So ./gradlew check -DsparkVersions= fails with package org.apache.spark.sql does not exist — likely a contributing reason the job was dropped rather than fixed. The new job passes -DsparkVersions=3.1 and names the core projects explicitly, keeping Spark test compilation out of it.

Pre-existing failure surfaced — and fixed

TestRequiredDistributionAndOrdering#testSaveAsTableAppendWithRangeDistribution was added by #243 and has never run in CI. Under spark_catalog it failed:

AnalysisException: The format of the existing table default.table is `HiveFileFormat`.
It doesn't match the specified format `ParquetDataSourceV2`.

This is a test bug, not a product bug, and it is fixable rather than skippable. From Spark 3.1's DataFrameWriter:

case nameParts @ SessionCatalogAndIdentifier(catalog, ident)
  if ident.namespace().length <= 1 && canUseV2(ident) =>
  saveAsTable(catalog.asTableCatalog, ident, nameParts)   // v2

case AsTableIdentifier(tableIdentifier) =>
  saveAsTable(tableIdentifier)                            // v1

private def lookupV2Provider(): Option[TableProvider] = {
  DataSource.lookupDataSourceV2(source, ...) match {
    // TODO(SPARK-28396): File source v2 write path is currently broken.
    case Some(_: FileDataSourceV2) => None
    case other => other
  }
}

The write omitted .format("iceberg"), so source defaulted to parquet; lookupV2Provider() maps ParquetDataSourceV2 to None under the SPARK-28396 guard, canUseV2 is false, and the write falls to the v1 branch where PreprocessTableCreation rejects it against the Hive-registered table. AppendData — the plan node the rule under test matches — was never produced, so the assertion never exercised the rule. The non-session catalogs resolve as NonSessionCatalogAndIdentifier and reach v2 regardless, which is why only spark_catalog failed.

Adding .format("iceberg") makes all three catalogs take the intended v2 path:

testhive         PASSED
testhadoop       PASSED
spark_catalog    PASSED

The test now passes everywhere rather than being skipped, so the spark_catalog coverage #243 intended is real. Module total: 860 tests, 0 failures, skips down 32 → 31.

Verification

Full suites run locally on JDK 11 at b8eeef208, zero failures:

Module Tests
iceberg-api 594
iceberg-core 2850
iceberg-orc 51
iceberg-data 394
iceberg-parquet 153
iceberg-arrow 14
iceberg-spark-3.1_2.12 1327
iceberg-spark-extensions-3.1_2.12 860

CI runs JVM 8 for the Spark jobs. No JDK 8 was available locally, so that combination is exercised for the first time by this PR's own checks — worth watching before merge.

Spark and Java CI have not executed any tests since linkedin#173, which passed
-x test -x integrationTest to both workflows. A green check on this fork
currently means "it compiles" -- no iceberg-api, iceberg-core, iceberg-orc,
iceberg-data, or Spark 3.1 test has run in CI for over a year.

linkedin#173 described itself as matching the li-1.0.x branch, but the match was
partial. On li-1.0.x, java-ci had two jobs: core-tests, which ran
`./gradlew check -Pquick=true` and uploaded test logs on failure, and
build-checks, a deliberately compile-only cross-version job where -x test is
correct by design. linkedin#173 kept build-checks and dropped core-tests, so the
exclusion outlived the job that justified it. It then added the same
exclusion to spark-ci, which li-1.0.x never had -- 1.0.x ran the Spark
suites.

Restore coverage:

- java-ci: add a core-tests job over api, core, common, orc, parquet, arrow,
  and data on JVM 8 and 11. The li-1.0.x command (`-DsparkVersions=`) cannot
  be reused verbatim: settings.gradle unconditionally includes the Spark 3.1
  projects for this backport branch, but their dependencies are only wired
  when sparkVersions contains 3.1, so an empty value fails compileTestJava.
  This job passes -DsparkVersions=3.1 and names the projects explicitly.
- spark-ci: drop -x test and restore the failure-log upload that linkedin#173
  removed. integrationTest stays excluded; it depends on shadowJar and is a
  separate concern.
- Both: fetch-depth 0, so generateGitProperties can populate
  iceberg-build.properties. Without it TestIcebergBuild fails on "unknown"
  version and commit id.

Restoring Spark coverage surfaced one pre-existing failure.
TestRequiredDistributionAndOrdering#testSaveAsTableAppendWithRangeDistribution
was added by linkedin#243 and has never run in CI. It failed under SparkSessionCatalog
with "The format of the existing table default.table is HiveFileFormat"
because the write omitted format("iceberg").

DataFrameWriter#saveAsTable only takes the v2 path for a
SessionCatalogAndIdentifier when lookupV2Provider() is defined, and that
returns None for the default "parquet" source: ParquetDataSourceV2 is a
FileDataSourceV2, which lookupV2Provider explicitly maps to None under
SPARK-28396. So the write fell through to the v1 path, where
PreprocessTableCreation rejected it against the Hive-registered table, and
AppendData -- the plan node the rule under test matches -- was never
produced. The non-session catalogs resolve as NonSessionCatalogAndIdentifier
and reach v2 regardless, which is why only spark_catalog failed.

Adding format("iceberg") makes all three catalogs exercise the intended v2
path. The test now passes everywhere rather than being skipped, so the
spark_catalog coverage linkedin#243 intended is real.

Verified locally on JDK 11:

  api 594, core 2850, orc 51, data 394, parquet 153, arrow 14
  spark-3.1 1327, spark-extensions-3.1 860

all passing, zero failures. CI runs JVM 8 for the Spark jobs; no JDK 8 was
available locally, so that combination is exercised for the first time by
this PR.
@cbb330
cbb330 force-pushed the chbush/ci-restore-tests branch from a8deb85 to 3fdc30a Compare August 3, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant