test: Add Iceberg API stability tests #3240

andygrove · 2026-01-22T17:04:38Z

Rationale

The goal of this PR is to add dedicated tests for the public API that Iceberg currently depends on (based on the latest Iceberg main branch). All of the tests were generated with AI. They may not be perfect, but it is a start. We can always add more tests.

Important

Note that these tests only check that the API is stable. They do not currently test the functionality, but there are existing Comet integration tests that verify it via the native_comet scan. In the future, we will add functional tests to the new iceberg-api module because the native_comet scan is deprecated in Comet and will eventually be removed, along with all of its dedicated tests. See #2177 for more details about this.

The rest of this PR description was written by AI.

Summary

Add new dedicated unit tests for all @IcebergApi annotated classes
Ensure the public API contract with Apache Iceberg remains stable and tested

Note: This PR builds on #3237 which adds the @IcebergApi annotation. Please merge #3237 first.

Test Coverage (169 tests)

Category	Tests	Description
API Verification	`IcebergApiVerificationTest`	Reflection-based verification of all @IcebergApi elements
Parquet	`FileReaderApiTest`, `RowGroupReaderApiTest`	File and row group reading APIs
Column Readers	`ColumnReaderApiTest`, `BatchReaderApiTest`, etc.	Column reader class hierarchies
Native	`NativeApiTest`	JNI method signature verification
Type Utils	`TypeUtilApiTest`, `UtilsApiTest`	Type conversion and utility methods
Vector	`CometVectorApiTest`	Arrow vector wrapper APIs
Schema	`CometSchemaImporterApiTest`	Schema import APIs
Adapter	`WrappedInputFileApiTest`	InputFile wrapper for Iceberg

Test plan

All 169 tests pass locally with mvn test -DwildcardSuites="org.apache.comet.iceberg.api"

🤖 Generated with Claude Code

Add documentation detailing all Comet classes and methods that form the public API used by Apache Iceberg. This helps contributors understand which APIs may affect Iceberg integration and need backward compatibility considerations. The documentation covers: - org.apache.comet.parquet: FileReader, RowGroupReader, ReadOptions, ParquetColumnSpec, column readers, BatchReader, Native JNI methods - org.apache.comet: CometSchemaImporter - org.apache.comet.vector: CometVector - org.apache.comet.shaded.arrow: RootAllocator, ValueVector Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add a custom Java annotation @IcebergApi to mark all classes, methods, constructors, and fields that form the public API used by Apache Iceberg. This makes it easy to identify which APIs need backward compatibility considerations when making changes. The annotation is applied to: - org.apache.comet.parquet: FileReader, RowGroupReader, ReadOptions, WrappedInputFile, ParquetColumnSpec, AbstractColumnReader, ColumnReader, BatchReader, MetadataColumnReader, ConstantColumnReader, Native, TypeUtil, Utils - org.apache.comet: CometSchemaImporter - org.apache.comet.vector: CometVector Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add annotations to: - AbstractColumnReader.nativeHandle (protected field accessed by Iceberg subclasses) - AbstractCometSchemaImporter.close() (called by Iceberg) Also update documentation to include these APIs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add a new Maven module containing dedicated unit tests for all @IcebergApi annotated classes, ensuring the public API contract with Apache Iceberg remains stable and tested. Key changes: - Add iceberg-public-api module with 169 tests covering all @IcebergApi classes - Fix CometVector constructor visibility (protected -> public) to match API annotation - Add IcebergApiVerificationTest for reflection-based API verification - Add tests for FileReader, BatchReader, ColumnReader, Native, TypeUtil, Utils - Add tests for CometVector, CometSchemaImporter, WrappedInputFile Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add a dedicated workflow that runs the iceberg-public-api module tests when changes are made to the common module or the API test module itself. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Use local `root_op` variable instead of unwrapping `exec_context.root_op` - Replace `is_some()` + `unwrap()` pattern with `if let Some(...)` Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

The CometVector constructor should remain protected since it's an abstract class meant to be subclassed (by Iceberg). The @IcebergApi annotation is kept since the constructor is part of the API contract for subclasses. Updated IcebergApiVerificationTest to allow protected constructors for abstract classes, as this is a valid pattern for inheritance-based APIs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Shorter, cleaner module name. The module can potentially hold both API source code and tests in the future. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

parthchandra · 2026-01-22T18:02:28Z

This would require an update in iceberg which would require a comet release. In the meantime, we have apache/iceberg#13786 open.

…dule The iceberg-api module tests could not access AbstractCometSchemaImporter because the shade plugin relocates org.apache.arrow.* classes. Moving the tests to the common module allows them to run before shading occurs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Add a new "iceberg" test suite containing the Iceberg API verification tests that were moved to the common module. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

This reverts commit 6a467f1.

codecov-commenter · 2026-01-22T20:48:27Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.85%. Comparing base (f09f8af) to head (637f9ed).
⚠️ Report is 865 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3240      +/-   ##
============================================
+ Coverage     56.12%   60.85%   +4.72%     
- Complexity      976     1464     +488     
============================================
  Files           119      170      +51     
  Lines         11743    15774    +4031     
  Branches       2251     2606     +355     
============================================
+ Hits           6591     9599    +3008     
- Misses         4012     4849     +837     
- Partials       1140     1326     +186

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

andygrove · 2026-02-01T19:35:48Z

I am closing this PR for now and will revisit this if/when #3237 gets merged

andygrove and others added 8 commits January 21, 2026 17:24

format

7b58965

ci: add GitHub workflow for Iceberg public API tests

2724275

Add a dedicated workflow that runs the iceberg-public-api module tests when changes are made to the common module or the API test module itself. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Fix clippy warnings for Rust 1.93

f1f4689

- Use local `root_op` variable instead of unwrapping `exec_context.root_op` - Replace `is_some()` + `unwrap()` pattern with `if let Some(...)` Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

andygrove requested review from huaxingao, mbutrovich and parthchandra January 22, 2026 17:20

andygrove changed the title ~~test: add iceberg-public-api module with API stability tests~~ test: add iceberg-public-api module with API stability tests [iceberg] Jan 22, 2026

andygrove marked this pull request as ready for review January 22, 2026 17:24

trigger CI

fda6b98

andygrove mentioned this pull request Jan 22, 2026

Move Iceberg-only API classes to iceberg-api module #3241

Closed

4 tasks

refactor: rename iceberg-public-api module to iceberg-api

396eacc

Shorter, cleaner module name. The module can potentially hold both API source code and tests in the future. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

andygrove changed the title ~~test: add iceberg-public-api module with API stability tests [iceberg]~~ test: add iceberg-api module with API stability tests [iceberg] Jan 22, 2026

andygrove mentioned this pull request Jan 22, 2026

Future of Iceberg Support in Comet #2921

Open

andygrove added this to the 0.13.0 milestone Jan 22, 2026

Remove workflow

b207008

andygrove marked this pull request as draft January 22, 2026 20:15

andygrove changed the title ~~test: add iceberg-api module with API stability tests [iceberg]~~ test: Add Iceberg API stability tests [iceberg] Jan 22, 2026

andygrove changed the title ~~test: Add Iceberg API stability tests [iceberg]~~ test: Add Iceberg API stability tests Jan 22, 2026

andygrove and others added 4 commits January 22, 2026 13:19

ci: add iceberg test suite to Linux and macOS workflows

6a467f1

Add a new "iceberg" test suite containing the Iceberg API verification tests that were moved to the common module. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Revert "ci: add iceberg test suite to Linux and macOS workflows"

e1c1e88

This reverts commit 6a467f1.

Merge remote-tracking branch 'apache/main' into iceberg-public-api-tests

637f9ed

andygrove mentioned this pull request Jan 22, 2026

chore: Annotate classes/methods/fields that are used by Apache Iceberg #3237

Open

andygrove marked this pull request as ready for review January 22, 2026 23:09

andygrove removed this from the 0.13.0 milestone Jan 23, 2026

andygrove marked this pull request as draft January 23, 2026 17:53

andygrove closed this Feb 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Add Iceberg API stability tests #3240

test: Add Iceberg API stability tests #3240

Uh oh!

andygrove commented Jan 22, 2026 •

edited

Loading

Uh oh!

parthchandra commented Jan 22, 2026

Uh oh!

codecov-commenter commented Jan 22, 2026 •

edited

Loading

Uh oh!

andygrove commented Feb 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

test: Add Iceberg API stability tests #3240

test: Add Iceberg API stability tests #3240

Uh oh!

Conversation

andygrove commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Summary

Test Coverage (169 tests)

Test plan

Uh oh!

parthchandra commented Jan 22, 2026

Uh oh!

codecov-commenter commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andygrove commented Feb 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andygrove commented Jan 22, 2026 •

edited

Loading

codecov-commenter commented Jan 22, 2026 •

edited

Loading