Skip to content

Commit 8798b1d

Browse files
andygroveclaude
andygrove
and
claude
committed
test: add iceberg-public-api module with API stability tests
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>
1 parent 7b58965 commit 8798b1d

22 files changed

Lines changed: 3205 additions & 1 deletion

‎common/src/main/java/org/apache/comet/vector/CometVector.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public abstract class CometVector extends ColumnVector {
6262
}
6363

6464
@IcebergApi
65-
protected CometVector(DataType type, boolean useDecimal128) {
65+
public CometVector(DataType type, boolean useDecimal128) {
6666
super(type);
6767
this.useDecimal128 = useDecimal128;
6868
}

‎iceberg-public-api/pom.xml‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
3+
<!--
4+
Licensed to the Apache Software Foundation (ASF) under one
5+
or more contributor license agreements. See the NOTICE file
6+
distributed with this work for additional information
7+
regarding copyright ownership. The ASF licenses this file
8+
to you under the Apache License, Version 2.0 (the
9+
"License"); you may not use this file except in compliance
10+
with the License. You may obtain a copy of the License at
11+
12+
http://www.apache.org/licenses/LICENSE-2.0
13+
14+
Unless required by applicable law or agreed to in writing,
15+
software distributed under the License is distributed on an
16+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
17+
KIND, either express or implied. See the License for the
18+
specific language governing permissions and limitations
19+
under the License.
20+
-->
21+
22+
23+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
24+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
25+
<modelVersion>4.0.0</modelVersion>
26+
<parent>
27+
<groupId>org.apache.datafusion</groupId>
28+
<artifactId>comet-parent-spark${spark.version.short}_${scala.binary.version}</artifactId>
29+
<version>0.13.0-SNAPSHOT</version>
30+
<relativePath>../pom.xml</relativePath>
31+
</parent>
32+
33+
<artifactId>comet-iceberg-public-api-spark${spark.version.short}_${scala.binary.version}</artifactId>
34+
<name>comet-iceberg-public-api</name>
35+
<description>Tests for Comet's public API used by Apache Iceberg</description>
36+
37+
<properties>
38+
<!-- Skip install/deploy - this is a test-only module -->
39+
<maven.install.skip>true</maven.install.skip>
40+
<maven.deploy.skip>true</maven.deploy.skip>
41+
</properties>
42+
43+
<dependencies>
44+
<!-- Module under test -->
45+
<dependency>
46+
<groupId>org.apache.datafusion</groupId>
47+
<artifactId>comet-common-spark${spark.version.short}_${scala.binary.version}</artifactId>
48+
<version>${project.version}</version>
49+
<scope>test</scope>
50+
</dependency>
51+
52+
<!-- Test framework -->
53+
<dependency>
54+
<groupId>junit</groupId>
55+
<artifactId>junit</artifactId>
56+
<scope>test</scope>
57+
</dependency>
58+
<dependency>
59+
<groupId>org.assertj</groupId>
60+
<artifactId>assertj-core</artifactId>
61+
<scope>test</scope>
62+
</dependency>
63+
64+
<!-- Parquet dependencies for test file creation -->
65+
<dependency>
66+
<groupId>org.apache.parquet</groupId>
67+
<artifactId>parquet-hadoop</artifactId>
68+
<scope>test</scope>
69+
</dependency>
70+
<dependency>
71+
<groupId>org.apache.parquet</groupId>
72+
<artifactId>parquet-column</artifactId>
73+
<scope>test</scope>
74+
</dependency>
75+
76+
<!-- Hadoop for file system operations -->
77+
<dependency>
78+
<groupId>org.apache.hadoop</groupId>
79+
<artifactId>hadoop-client-minicluster</artifactId>
80+
<scope>test</scope>
81+
<exclusions>
82+
<exclusion>
83+
<groupId>commons-logging</groupId>
84+
<artifactId>commons-logging</artifactId>
85+
</exclusion>
86+
</exclusions>
87+
</dependency>
88+
89+
<!-- Spark SQL for schema conversion -->
90+
<dependency>
91+
<groupId>org.apache.spark</groupId>
92+
<artifactId>spark-sql_${scala.binary.version}</artifactId>
93+
<scope>test</scope>
94+
</dependency>
95+
96+
<!-- Arrow dependencies -->
97+
<dependency>
98+
<groupId>org.apache.arrow</groupId>
99+
<artifactId>arrow-vector</artifactId>
100+
<scope>test</scope>
101+
</dependency>
102+
<dependency>
103+
<groupId>org.apache.arrow</groupId>
104+
<artifactId>arrow-memory-unsafe</artifactId>
105+
<scope>test</scope>
106+
</dependency>
107+
<dependency>
108+
<groupId>org.apache.arrow</groupId>
109+
<artifactId>arrow-c-data</artifactId>
110+
<scope>test</scope>
111+
</dependency>
112+
</dependencies>
113+
114+
<build>
115+
<testSourceDirectory>src/test/java</testSourceDirectory>
116+
<plugins>
117+
<plugin>
118+
<groupId>org.codehaus.mojo</groupId>
119+
<artifactId>build-helper-maven-plugin</artifactId>
120+
<executions>
121+
<execution>
122+
<id>add-test-source</id>
123+
<phase>generate-test-sources</phase>
124+
<goals>
125+
<goal>add-test-source</goal>
126+
</goals>
127+
<configuration>
128+
<sources>
129+
<source>src/test/java</source>
130+
</sources>
131+
</configuration>
132+
</execution>
133+
</executions>
134+
</plugin>
135+
<plugin>
136+
<groupId>org.apache.maven.plugins</groupId>
137+
<artifactId>maven-compiler-plugin</artifactId>
138+
<configuration>
139+
<!-- Override parent pom skip settings -->
140+
<skipMain>true</skipMain>
141+
<skip>false</skip>
142+
</configuration>
143+
<executions>
144+
<execution>
145+
<id>test-compile</id>
146+
<phase>test-compile</phase>
147+
<goals>
148+
<goal>testCompile</goal>
149+
</goals>
150+
</execution>
151+
</executions>
152+
</plugin>
153+
<plugin>
154+
<groupId>org.apache.maven.plugins</groupId>
155+
<artifactId>maven-surefire-plugin</artifactId>
156+
<configuration>
157+
<systemPropertyVariables>
158+
<log4j.configurationFile>file:src/test/resources/log4j2.properties</log4j.configurationFile>
159+
</systemPropertyVariables>
160+
<failIfNoSpecifiedTests>false</failIfNoSpecifiedTests>
161+
</configuration>
162+
</plugin>
163+
<plugin>
164+
<groupId>com.diffplug.spotless</groupId>
165+
<artifactId>spotless-maven-plugin</artifactId>
166+
<executions>
167+
<execution>
168+
<goals>
169+
<goal>check</goal>
170+
</goals>
171+
<phase>compile</phase>
172+
</execution>
173+
</executions>
174+
</plugin>
175+
</plugins>
176+
</build>
177+
178+
</project>

‎iceberg-public-api/src/test/java/org/apache/comet/iceberg/api/AbstractApiTest.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.comet.iceberg.api;
21+
22+
import java.io.File;
23+
import java.io.IOException;
24+
import java.lang.reflect.Constructor;
25+
import java.lang.reflect.Field;
26+
import java.lang.reflect.Method;
27+
import java.lang.reflect.Modifier;
28+
import java.nio.file.Files;
29+
import java.nio.file.Path;
30+
import java.util.Comparator;
31+
32+
import org.junit.After;
33+
import org.junit.Before;
34+
35+
import org.apache.comet.IcebergApi;
36+
37+
/**
38+
* Base class for Iceberg API tests. Provides common utilities for testing annotated API elements.
39+
*/
40+
public abstract class AbstractApiTest {
41+
42+
protected Path tempDir;
43+
44+
@Before
45+
public void setUp() throws IOException {
46+
tempDir = Files.createTempDirectory("iceberg-api-test");
47+
}
48+
49+
@After
50+
public void tearDown() throws IOException {
51+
if (tempDir != null && Files.exists(tempDir)) {
52+
Files.walk(tempDir).sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
53+
}
54+
}
55+
56+
/** Checks if a class has the @IcebergApi annotation. */
57+
protected static boolean hasIcebergApiAnnotation(Class<?> clazz) {
58+
return clazz.isAnnotationPresent(IcebergApi.class);
59+
}
60+
61+
/** Checks if a method has the @IcebergApi annotation. */
62+
protected static boolean hasIcebergApiAnnotation(Method method) {
63+
return method.isAnnotationPresent(IcebergApi.class);
64+
}
65+
66+
/** Checks if a constructor has the @IcebergApi annotation. */
67+
protected static boolean hasIcebergApiAnnotation(Constructor<?> constructor) {
68+
return constructor.isAnnotationPresent(IcebergApi.class);
69+
}
70+
71+
/** Checks if a field has the @IcebergApi annotation. */
72+
protected static boolean hasIcebergApiAnnotation(Field field) {
73+
return field.isAnnotationPresent(IcebergApi.class);
74+
}
75+
76+
/** Checks if a class is public. */
77+
protected static boolean isPublic(Class<?> clazz) {
78+
return Modifier.isPublic(clazz.getModifiers());
79+
}
80+
81+
/** Checks if a method is public. */
82+
protected static boolean isPublic(Method method) {
83+
return Modifier.isPublic(method.getModifiers());
84+
}
85+
86+
/** Checks if a constructor is public. */
87+
protected static boolean isPublic(Constructor<?> constructor) {
88+
return Modifier.isPublic(constructor.getModifiers());
89+
}
90+
91+
/** Checks if a field is public or protected. */
92+
protected static boolean isAccessible(Field field) {
93+
int modifiers = field.getModifiers();
94+
return Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers);
95+
}
96+
97+
/** Checks if native library is available. */
98+
protected static boolean isNativeLibraryAvailable() {
99+
try {
100+
Class.forName("org.apache.comet.NativeBase");
101+
return true;
102+
} catch (Throwable t) {
103+
return false;
104+
}
105+
}
106+
107+
/** Creates a temp file path for testing. */
108+
protected String createTempFilePath(String name) {
109+
return tempDir.resolve(name).toString();
110+
}
111+
}

0 commit comments

Comments
 (0)