Skip to content

[FEA] Add Iceberg fast test matrix [fast-ut] - #15876

Open
liurenjie1024 wants to merge 8 commits into
NVIDIA:mainfrom
liurenjie1024:ray/15875
Open

[FEA] Add Iceberg fast test matrix [fast-ut]#15876
liurenjie1024 wants to merge 8 commits into
NVIDIA:mainfrom
liurenjie1024:ray/15875

Conversation

@liurenjie1024

@liurenjie1024 liurenjie1024 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15875.

Description

Expand Iceberg integration-test coverage across the supported Spark and Iceberg version matrix without running the complete test suite for every combination.

This change:

  • Adds an authoritative, readable JSON compatibility matrix at iceberg/iceberg-versions.json, based on Apache Iceberg's Spark patch baselines.
  • Records upstream-compatible but currently unsupported combinations with explicit reasons.
  • Adds an object-oriented, validated Python matrix reader that uses the standard-library JSON parser and is shared by nightly and premerge test scripts.
  • Adds focused standard-library unit tests for release-profile mappings, selectors, and malformed metadata.
  • Uses the matrix to select every supported Iceberg version for local Hadoop, REST, and S3 Tables catalog runs.
  • Allows callers to enable ICEBERG_TEST_FAST_RUN=1 for local Hadoop-catalog runs; the flag defaults to 0 when it is not set.
  • Preserves validation and caller control for explicitly requested Iceberg versions.
  • Applies fast-mode skipping only to tests already excluded from remote catalogs to reduce runtime, retaining local-only coverage.

AI assistance disclosure: This change and pull request description were prepared with Codex and reviewed by the author before submission and update.

Testing performed:

  • Validated the checked-in JSON compatibility matrix against every Spark shim declared in scala2.13/pom.xml.
  • Verified that the JSON is canonically formatted with two-space indentation.
  • Exercised supported, unsupported, and explicitly requested version queries through the object model and CLI.
  • Ran all 31 script unit tests; the 11 new Iceberg matrix tests cover every Scala 2.13 release profile, stub and empty selections, requested versions, malformed metadata, and missing mappings.
  • Compiled all changed Python sources.
  • Validated 79 fast-mode test skips and 10 catalog-only skips through AST inspection.
  • Checked shell syntax and ShellCheck for the modified Jenkins scripts.
  • Verified the default and caller-provided ICEBERG_TEST_FAST_RUN values.
  • Ran Apache RAT checks through both the Scala 2.12 and Scala 2.13 Maven entry points.
  • Verified the final diff with git diff --check.

Full GPU integration tests were not run locally because the required built artifacts and GPU test environment were unavailable.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
@liurenjie1024
liurenjie1024 requested a review from a team as a code owner September 2, 2026 09:05
@liurenjie1024 liurenjie1024 added the test Only impacts tests label Sep 2, 2026
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable regression or repository-rule violation remains.

Summary

  • Introduces a JSON matrix reader shared by premerge and nightly Jenkins scripts.
  • Adds opt-in fast-mode skips for expensive local-catalog Iceberg tests.
  • Adds unit coverage for matrix validation, release-profile selection, and CLI error handling.
  • Expands Maven’s Python test discovery so the new matrix tests run in both build trees.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  POM[Scala 2.13 Spark profiles] --> Reader[Iceberg matrix reader]
  Matrix[iceberg-versions.json] --> Reader
  Reader --> Selection[Supported Iceberg versions]
  Selection --> Premerge[Premerge version-detection tests]
  Selection --> Nightly[Nightly catalog test runs]
  Fast[ICEBERG_TEST_FAST_RUN] --> Nightly
  Nightly --> Suite[Iceberg integration suites]
Loading

Reviews (6) · Last reviewed commit: "Test the Iceberg version matrix selector"

@liurenjie1024
liurenjie1024 marked this pull request as draft September 2, 2026 09:17
Comment thread iceberg/iceberg-versions.yml Outdated
Comment thread scripts/tests/test_get_iceberg_versions.py
Comment thread scripts/tests/test_iceberg_fast_test_selection.py Outdated
Comment thread jenkins/get_iceberg_versions.py Outdated
Comment thread jenkins/spark-tests.sh Outdated
Comment thread jenkins/spark-tests.sh Outdated
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
@liurenjie1024
liurenjie1024 marked this pull request as ready for review September 4, 2026 06:36
Comment thread iceberg/iceberg-versions.yml Outdated
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
Comment thread jenkins/spark-tests.sh Outdated
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
Comment thread integration_tests/requirements.txt Outdated
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
Comment thread jenkins/spark-tests.sh
Comment thread jenkins/get_iceberg_versions.py Outdated
Comment thread jenkins/spark-tests.sh
Signed-off-by: Ray Liu <liurenjie2008@gmail.com>

@res-life res-life left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a positive marker for the Iceberg matrix smoke suite

Adding is_iceberg_test_fast_run() to 79 existing tests makes CI-selection policy part of each test and creates a large mechanical diff. It also causes future Iceberg tests to enter the reduced matrix suite by default unless they explicitly add the skip condition. Please use ICEBERG_TEST_MATRIX_SMOKE_ONLY=1 to enable this mode, register @pytest.mark.iceberg_matrix_smoke, and apply the marker only to the intentionally selected cases. The runner can then use -m "iceberg and iceberg_matrix_smoke" in matrix-smoke mode, while normal and remote-catalog runs continue using -m iceberg. This keeps the smoke suite as an explicit allowlist and removes the repeated conditional changes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supported Spark × Iceberg combinations can and should be derived from Maven release profiles and module POMs. This JSON is not the source of truth has already diverged:

  • Spark 3.5.0 => no Iceberg version.
  • Spark 3.5.4 => despite packaging 1.9.x and 1.10.x selects none for testging
  • Spark 3.5.5 => omits packaged 1.10.x support.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a combination of upstream oss with our pom, and they are not diverged.

For example: Spark 3.5.0 => no Iceberg version, there is no 3.5.0 listed the json.

expected_versions.update(
spark_version for spark_version in spark_shims
if _spark_family(spark_version) == family and
_version_tuple(spark_version) >= minimum_tuple)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This >= minimum inference is not supported by the upstream data. Iceberg libs.versions.toml records exact Spark dependency/test pins—for example, Iceberg 1.11 pins Spark 3.5.8 and consumes it directly as both a compileOnly and integration-test dependency (catalog, build). It does not declare that pin as the minimum compatible Spark patch. Routine upstream pin bumps would therefore silently remove older patches from our matrix. Please retain this only as tested-baseline metadata, if we need it for sampling, and derive packaged support from our release profiles.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treating upstream recorded version as minimum patch version is following the discussion we had last time, please check the google doc for reference.

Comment thread jenkins/get_iceberg_versions.py
iceberg_versions="1.9.2 1.10.1"
iceberg_versions=$(python jenkins/get_iceberg_versions.py \
--spark-version "$spark_ver") || return 1
if [[ -z "$iceberg_versions" ]]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could an empty selection fail when the chosen release profile packages an Iceberg module? Matrix drift currently sends Spark 3.5.0 and 3.5.4 through this branch, reporting success after running zero version-detection tests. Skipping is appropriate when the POM profile genuinely has no Iceberg integration; otherwise this should fail loudly.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we trust the result of get_iceberg_versions.py, then I think it's fine.

@liurenjie1024

Copy link
Copy Markdown
Collaborator Author

Use a positive marker for the Iceberg matrix smoke suite

Adding is_iceberg_test_fast_run() to 79 existing tests makes CI-selection policy part of each test and creates a large mechanical diff. It also causes future Iceberg tests to enter the reduced matrix suite by default unless they explicitly add the skip condition. Please use ICEBERG_TEST_MATRIX_SMOKE_ONLY=1 to enable this mode, register @pytest.mark.iceberg_matrix_smoke, and apply the marker only to the intentionally selected cases. The runner can then use -m "iceberg and iceberg_matrix_smoke" in matrix-smoke mode, while normal and remote-catalog runs continue using -m iceberg. This keeps the smoke suite as an explicit allowlist and removes the repeated conditional changes.

I think this is similar to what we do, but using a positive marker is easier to maintain. cc @yinqingh WDYT? I'm fine with both.

Signed-off-by: Ray Liu <liurenjie2008@gmail.com>
@yinqingh

Copy link
Copy Markdown
Collaborator

Use a positive marker for the Iceberg matrix smoke suite
Adding is_iceberg_test_fast_run() to 79 existing tests makes CI-selection policy part of each test and creates a large mechanical diff. It also causes future Iceberg tests to enter the reduced matrix suite by default unless they explicitly add the skip condition. Please use ICEBERG_TEST_MATRIX_SMOKE_ONLY=1 to enable this mode, register @pytest.mark.iceberg_matrix_smoke, and apply the marker only to the intentionally selected cases. The runner can then use -m "iceberg and iceberg_matrix_smoke" in matrix-smoke mode, while normal and remote-catalog runs continue using -m iceberg. This keeps the smoke suite as an explicit allowlist and removes the repeated conditional changes.

I think this is similar to what we do, but using a positive marker is easier to maintain. cc @yinqingh WDYT? I'm fine with both.

From the caller’s perspective, I would still only need to set ICEBERG_TEST_MATRIX_SMOKE_ONLY=1 to run the smoke suite, so there’s no behavioral change for me if I understand correctly. I’m fine with this approach if the positive marker makes the suite easier to maintain.

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

Labels

test Only impacts tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Better iceberg test coverage

5 participants