diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java index 61f100ca5c09..6203bf89bfa1 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/CatalogTestBase.java @@ -19,7 +19,6 @@ package org.apache.iceberg.spark; import java.nio.file.Path; -import java.util.Map; import org.apache.iceberg.ParameterizedTestExtension; import org.apache.iceberg.Parameters; import org.junit.jupiter.api.extension.ExtendWith; @@ -30,7 +29,7 @@ public abstract class CatalogTestBase extends TestBaseWithCatalog { // these parameters are broken out to avoid changes that need to modify lots of test suites @Parameters(name = "catalogName = {0}, implementation = {1}, config = {2}") - public static Object[][] parameters() { + protected static Object[][] parameters() { return new Object[][] { { SparkCatalogConfig.HIVE.catalogName(), @@ -51,12 +50,4 @@ public static Object[][] parameters() { } @TempDir protected Path temp; - - public CatalogTestBase(SparkCatalogConfig config) { - super(config); - } - - public CatalogTestBase(String catalogName, String implementation, Map config) { - super(catalogName, implementation, config); - } } diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java index 83767039bb96..dd00405dc87c 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestBaseWithCatalog.java @@ -24,6 +24,9 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Parameter; +import org.apache.iceberg.ParameterizedTestExtension; +import org.apache.iceberg.Parameters; import org.apache.iceberg.PlanningMode; import org.apache.iceberg.TableProperties; import org.apache.iceberg.catalog.Catalog; @@ -35,11 +38,25 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +@ExtendWith(ParameterizedTestExtension.class) public abstract class TestBaseWithCatalog extends TestBase { protected static File warehouse = null; + @Parameters(name = "catalogName = {0}, implementation = {1}, config = {2}") + protected static Object[][] parameters() { + return new Object[][] { + { + SparkCatalogConfig.HADOOP.catalogName(), + SparkCatalogConfig.HADOOP.implementation(), + SparkCatalogConfig.HADOOP.properties() + }, + }; + } + @BeforeAll public static void createWarehouse() throws IOException { TestBaseWithCatalog.warehouse = File.createTempFile("warehouse", null); @@ -59,25 +76,22 @@ public static void dropWarehouse() throws IOException { @TempDir protected File temp; - protected final String catalogName; - protected final Map catalogConfig; - protected final Catalog validationCatalog; - protected final SupportsNamespaces validationNamespaceCatalog; - protected final TableIdentifier tableIdent = TableIdentifier.of(Namespace.of("default"), "table"); - protected final String tableName; + @Parameter(index = 0) + protected String catalogName; - public TestBaseWithCatalog() { - this(SparkCatalogConfig.HADOOP); - } + @Parameter(index = 1) + protected String implementation; - public TestBaseWithCatalog(SparkCatalogConfig config) { - this(config.catalogName(), config.implementation(), config.properties()); - } + @Parameter(index = 2) + protected Map catalogConfig; + + protected Catalog validationCatalog; + protected SupportsNamespaces validationNamespaceCatalog; + protected TableIdentifier tableIdent = TableIdentifier.of(Namespace.of("default"), "table"); + protected String tableName; - public TestBaseWithCatalog( - String catalogName, String implementation, Map config) { - this.catalogName = catalogName; - this.catalogConfig = config; + @BeforeEach + public void before() { this.validationCatalog = catalogName.equals("testhadoop") ? new HadoopCatalog(spark.sessionState().newHadoopConf(), "file:" + warehouse) @@ -85,10 +99,10 @@ public TestBaseWithCatalog( this.validationNamespaceCatalog = (SupportsNamespaces) validationCatalog; spark.conf().set("spark.sql.catalog." + catalogName, implementation); - config.forEach( + catalogConfig.forEach( (key, value) -> spark.conf().set("spark.sql.catalog." + catalogName + "." + key, value)); - if (config.get("type").equalsIgnoreCase("hadoop")) { + if (catalogConfig.get("type").equalsIgnoreCase("hadoop")) { spark.conf().set("spark.sql.catalog." + catalogName + ".warehouse", "file:" + warehouse); } diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java index 4959cd2a9e06..ffdaa8b477cb 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java @@ -37,7 +37,7 @@ import org.apache.spark.sql.internal.SQLConf; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; public class TestDataFrameWriterV2 extends TestBaseWithCatalog { @BeforeEach @@ -50,7 +50,7 @@ public void removeTables() { sql("DROP TABLE IF EXISTS %s", tableName); } - @Test + @TestTemplate public void testMergeSchemaFailsWithoutWriterOption() throws Exception { sql( "ALTER TABLE %s SET TBLPROPERTIES ('%s'='true')", @@ -82,7 +82,7 @@ public void testMergeSchemaFailsWithoutWriterOption() throws Exception { .hasMessage("Field new_col not found in source schema"); } - @Test + @TestTemplate public void testMergeSchemaWithoutAcceptAnySchema() throws Exception { Dataset twoColDF = jsonToDF( @@ -109,7 +109,7 @@ public void testMergeSchemaWithoutAcceptAnySchema() throws Exception { "Cannot write to `testhadoop`.`default`.`table`, the reason is too many data columns"); } - @Test + @TestTemplate public void testMergeSchemaSparkProperty() throws Exception { sql( "ALTER TABLE %s SET TBLPROPERTIES ('%s'='true')", @@ -143,7 +143,7 @@ public void testMergeSchemaSparkProperty() throws Exception { sql("select * from %s order by id", tableName)); } - @Test + @TestTemplate public void testMergeSchemaIcebergProperty() throws Exception { sql( "ALTER TABLE %s SET TBLPROPERTIES ('%s'='true')", @@ -177,7 +177,7 @@ public void testMergeSchemaIcebergProperty() throws Exception { sql("select * from %s order by id", tableName)); } - @Test + @TestTemplate public void testWriteWithCaseSensitiveOption() throws NoSuchTableException, ParseException { SparkSession sparkSession = spark.cloneSession(); sparkSession diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java index 00c9083e25fc..ff6ddea32360 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java @@ -59,7 +59,7 @@ import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.io.TempDir; public class TestDataSourceOptions extends TestBaseWithCatalog { @@ -84,7 +84,7 @@ public static void stopSpark() { currentSpark.stop(); } - @Test + @TestTemplate public void testWriteFormatOptionOverridesTableProperties() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -114,7 +114,7 @@ public void testWriteFormatOptionOverridesTableProperties() throws IOException { } } - @Test + @TestTemplate public void testNoWriteFormatOption() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -139,7 +139,7 @@ public void testNoWriteFormatOption() throws IOException { } } - @Test + @TestTemplate public void testHadoopOptions() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); Configuration sparkHadoopConf = spark.sessionState().newHadoopConf(); @@ -181,7 +181,7 @@ public void testHadoopOptions() throws IOException { } } - @Test + @TestTemplate public void testSplitOptionsOverridesTableProperties() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -224,7 +224,7 @@ public void testSplitOptionsOverridesTableProperties() throws IOException { .isEqualTo(2); } - @Test + @TestTemplate public void testIncrementalScanOptions() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -315,7 +315,7 @@ public void testIncrementalScanOptions() throws IOException { assertThat(resultDf.count()).as("Unprocessed count should match record count").isEqualTo(1); } - @Test + @TestTemplate public void testMetadataSplitSizeOptionOverrideTableProperties() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -355,7 +355,7 @@ public void testMetadataSplitSizeOptionOverrideTableProperties() throws IOExcept assertThat(entriesDf.javaRDD().getNumPartitions()).as("Num partitions must match").isEqualTo(1); } - @Test + @TestTemplate public void testDefaultMetadataSplitSize() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); @@ -389,7 +389,7 @@ public void testDefaultMetadataSplitSize() throws IOException { assertThat(partitionNum).as("Spark partitions should match").isEqualTo(expectedSplits); } - @Test + @TestTemplate public void testExtraSnapshotMetadata() throws IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); HadoopTables tables = new HadoopTables(CONF); @@ -414,7 +414,7 @@ public void testExtraSnapshotMetadata() throws IOException { .containsEntry("another-key", "anotherValue"); } - @Test + @TestTemplate public void testExtraSnapshotMetadataWithSQL() throws InterruptedException, IOException { String tableLocation = temp.resolve("iceberg-table").toFile().toString(); HadoopTables tables = new HadoopTables(CONF); @@ -459,7 +459,7 @@ public void testExtraSnapshotMetadataWithSQL() throws InterruptedException, IOEx .containsEntry("another-key", "anotherValue"); } - @Test + @TestTemplate public void testExtraSnapshotMetadataWithDelete() throws InterruptedException, NoSuchTableException { spark.sessionState().conf().setConfString("spark.sql.shuffle.partitions", "1"); diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java index 0696c3291e36..547ab32eac24 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java @@ -30,6 +30,7 @@ import java.util.Map; import org.apache.iceberg.DataFile; import org.apache.iceberg.Files; +import org.apache.iceberg.Parameters; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; @@ -48,7 +49,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.io.TempDir; public class TestMetadataTableReadableMetrics extends TestBaseWithCatalog { @@ -78,9 +79,16 @@ public class TestMetadataTableReadableMetrics extends TestBaseWithCatalog { optional(8, "fixedCol", Types.FixedType.ofLength(3)), optional(9, "binaryCol", Types.BinaryType.get())); - public TestMetadataTableReadableMetrics() { - // only SparkCatalog supports metadata table sql queries - super(SparkCatalogConfig.HIVE); + @Parameters(name = "catalogName = {0}, implementation = {1}, config = {2}") + protected static Object[][] parameters() { + return new Object[][] { + { + // only SparkCatalog supports metadata table sql queries + SparkCatalogConfig.HIVE.catalogName(), + SparkCatalogConfig.HIVE.implementation(), + SparkCatalogConfig.HIVE.properties() + }, + }; } protected String tableName() { @@ -190,7 +198,7 @@ private GenericRecord createNestedRecord(Long longCol, Double doubleCol) { return record; } - @Test + @TestTemplate public void testPrimitiveColumns() throws Exception { Table table = createPrimitiveTable(); DataFile dataFile = table.currentSnapshot().addedDataFiles(table.io()).iterator().next(); @@ -289,7 +297,7 @@ public void testPrimitiveColumns() throws Exception { assertEquals("Row should match for entries table", expected, entriesReadableMetrics); } - @Test + @TestTemplate public void testSelectPrimitiveValues() throws Exception { createPrimitiveTable(); @@ -328,7 +336,7 @@ public void testSelectPrimitiveValues() throws Exception { sql("SELECT readable_metrics.longCol.value_count, status FROM %s.entries", tableName)); } - @Test + @TestTemplate public void testSelectNestedValues() throws Exception { createNestedTable(); @@ -349,7 +357,7 @@ public void testSelectNestedValues() throws Exception { entriesReadableMetrics); } - @Test + @TestTemplate public void testNestedValues() throws Exception { Pair table = createNestedTable(); int longColId = diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalogCacheExpiration.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalogCacheExpiration.java index 9ff3de74f6fb..99eeb930af28 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalogCacheExpiration.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkCatalogCacheExpiration.java @@ -23,6 +23,7 @@ import java.util.Map; import org.apache.iceberg.CachingCatalog; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.Parameters; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.spark.SparkCatalog; @@ -30,12 +31,10 @@ import org.apache.iceberg.spark.TestBaseWithCatalog; import org.apache.spark.sql.connector.catalog.TableCatalog; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; public class TestSparkCatalogCacheExpiration extends TestBaseWithCatalog { - private static final String sessionCatalogName = "spark_catalog"; - private static final String sessionCatalogImpl = SparkSessionCatalog.class.getName(); private static final Map sessionCatalogConfig = ImmutableMap.of( "type", @@ -47,6 +46,13 @@ public class TestSparkCatalogCacheExpiration extends TestBaseWithCatalog { CatalogProperties.CACHE_EXPIRATION_INTERVAL_MS, "3000"); + @Parameters(name = "catalogName = {0}, implementation = {1}, config = {2}") + public static Object[][] parameters() { + return new Object[][] { + {"spark_catalog", SparkSessionCatalog.class.getName(), sessionCatalogConfig}, + }; + } + private static String asSqlConfCatalogKeyFor(String catalog, String configKey) { // configKey is empty when the catalog's class is being defined if (configKey.isEmpty()) { @@ -87,11 +93,7 @@ public static void beforeClass() { (k, v) -> spark.conf().set(asSqlConfCatalogKeyFor("cache_disabled_implicitly", k), v)); } - public TestSparkCatalogCacheExpiration() { - super(sessionCatalogName, sessionCatalogImpl, sessionCatalogConfig); - } - - @Test + @TestTemplate public void testSparkSessionCatalogWithExpirationEnabled() { SparkSessionCatalog sparkCatalog = sparkSessionCatalog(); assertThat(sparkCatalog) @@ -112,7 +114,7 @@ public void testSparkSessionCatalogWithExpirationEnabled() { }); } - @Test + @TestTemplate public void testCacheEnabledAndExpirationDisabled() { SparkCatalog sparkCatalog = getSparkCatalog("expiration_disabled"); assertThat(sparkCatalog).extracting("cacheEnabled").isEqualTo(true); @@ -126,7 +128,7 @@ public void testCacheEnabledAndExpirationDisabled() { }); } - @Test + @TestTemplate public void testCacheDisabledImplicitly() { SparkCatalog sparkCatalog = getSparkCatalog("cache_disabled_implicitly"); assertThat(sparkCatalog).extracting("cacheEnabled").isEqualTo(false); diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReadMetrics.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReadMetrics.java index 4ee2d9e4e558..895861e95948 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReadMetrics.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReadMetrics.java @@ -30,7 +30,7 @@ import org.apache.spark.sql.execution.SparkPlan; import org.apache.spark.sql.execution.metric.SQLMetric; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestTemplate; import scala.collection.JavaConverters; public class TestSparkReadMetrics extends TestBaseWithCatalog { @@ -40,7 +40,7 @@ public void removeTables() { sql("DROP TABLE IF EXISTS %s", tableName); } - @Test + @TestTemplate public void testReadMetricsForV1Table() throws NoSuchTableException { sql( "CREATE TABLE %s (id BIGINT) USING iceberg TBLPROPERTIES ('format-version'='1')", @@ -83,7 +83,7 @@ public void testReadMetricsForV1Table() throws NoSuchTableException { assertThat(metricsMap.get("skippedDeleteFiles").value()).isEqualTo(0); } - @Test + @TestTemplate public void testReadMetricsForV2Table() throws NoSuchTableException { sql( "CREATE TABLE %s (id BIGINT) USING iceberg TBLPROPERTIES ('format-version'='2')", @@ -127,7 +127,7 @@ public void testReadMetricsForV2Table() throws NoSuchTableException { assertThat(metricsMap.get("skippedDeleteFiles").value()).isEqualTo(0); } - @Test + @TestTemplate public void testDeleteMetrics() throws NoSuchTableException { sql( "CREATE TABLE %s (id BIGINT)" diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkStagedScan.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkStagedScan.java index 3305b9e91384..6ce2ce623835 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkStagedScan.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkStagedScan.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.List; -import java.util.Map; import java.util.UUID; import org.apache.iceberg.FileScanTask; import org.apache.iceberg.Table; @@ -39,11 +38,6 @@ public class TestSparkStagedScan extends CatalogTestBase { - public TestSparkStagedScan( - String catalogName, String implementation, Map config) { - super(catalogName, implementation, config); - } - @AfterEach public void removeTables() { sql("DROP TABLE IF EXISTS %s", tableName); diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkTable.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkTable.java index ecb720d06e55..46ee484b39ea 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkTable.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkTable.java @@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.util.Map; import org.apache.iceberg.spark.CatalogTestBase; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.connector.catalog.CatalogManager; @@ -32,10 +31,6 @@ public class TestSparkTable extends CatalogTestBase { - public TestSparkTable(String catalogName, String implementation, Map config) { - super(catalogName, implementation, config); - } - @BeforeEach public void createTable() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName);