LuciferYang opened a new issue, #12934:
URL: https://github.com/apache/gluten/issues/12934
### 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 #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, #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
--
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]
