Skip to content

[CH] Decide the fate of the ClickHouse Iceberg Java tests, which no build compiles today #12934

Description

@LuciferYang

Where things stand

backends-clickhouse has two Iceberg Java tests, TestPositionDeletesTableGluten and TestFlinkUpsert. #9996 introduced the versioned source layout, put both in src-iceberg-spark33/test/java, and added a JUnit 5 port of TestPositionDeletesTableGluten under src-iceberg-spark35/test/java without porting TestFlinkUpsert. #12902 removes Spark 3.3 and with it the two 3.3 copies, leaving the 3.5 TestPositionDeletesTableGluten as the only survivor.

Neither test has been compiled since #9996. Two independent reasons:

  1. src-iceberg-spark<N>/test/{scala,java} is added only by the iceberg-test profile, and no ClickHouse job passes it. The Jenkins command passes -Piceberg and -Dsurefire.skipTests=false but not -Piceberg-test.
  2. Even when it is passed, the build fails, because iceberg and iceberg-test collide on one build-helper execution id and the collision drops src-iceberg/test/scala, where ClickHouseIcebergHiveTableSupport lives. Filed as [BUILD] iceberg and iceberg-test profiles collide on one build-helper execution id, silently dropping src-iceberg/test sources #12933.

The ClickHouse Jenkins job also still hardcodes -Pspark-3.3, so it needs an update of its own before any of this runs.

Meanwhile the cost is not zero: the iceberg profile in backends-clickhouse/pom.xml carries nine Flink artifacts (three iceberg-flink-* plus flink-streaming-java, flink-table-api-bridge-base, flink-table-api-java, flink-table-api-java-bridge, flink-test-utils, flink-table-planner_2.12), resolved by every -Piceberg ClickHouse build for a test that never compiles. After #12902 no source file in the module references org.apache.flink at all.

Option 1: keep them

In this order, because each step depends on the previous one:

  1. Fix the execution-id collision, [BUILD] iceberg and iceberg-test profiles collide on one build-helper execution id, silently dropping src-iceberg/test sources #12933.
  2. Add -Piceberg-test to the ClickHouse job, whose -Pspark-3.3 also has to go.
  3. Port TestFlinkUpsert to src-iceberg-spark35. This is smaller than it looks. The file is already JUnit 5 (@TestTemplate, org.apache.iceberg.Parameters) and extends CatalogTestBase, which iceberg-flink-1.19-1.10.0-tests.jar still ships. The only unresolved symbol is MiniClusterResource, used once for DISABLE_CLASSLOADER_CHECK_CONFIG; MiniFlinkClusterExtension replaces it.
  4. Run both tests and fix what the run turns up. Nobody has seen either of them execute, so this step is the one with unknown cost.

Option 2: drop them

Delete src-iceberg-spark35/test/java/.../TestPositionDeletesTableGluten.java and the nine Flink dependencies. TestFlinkUpsert is already gone with #12902.

Recommendation

Option 1 if someone will own the ClickHouse CI wiring in step 2, since without it step 3 produces another file that nothing builds. Otherwise option 2: a test no build compiles is worse than no test, because it reads as coverage that does not exist. This is a call for the ClickHouse maintainers rather than something #12902 should decide.

cc @jackylee-ch

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions