Skip to content

Commit

Permalink
MR: Migrate tests to JUnit5 (apache#9241)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lschetanrao @lisirrx
lschetanrao authored and lisirrx committed Jan 4, 2024
1 parent 213b4b8 commit c394a07
Show file tree
Hide file tree
Showing 17 changed files with 400 additions and 391 deletions.
3 changes: 3 additions & 0 deletions mr/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ project(':iceberg-mr') {
exclude group: 'org.apache.parquet', module: 'parquet-hadoop-bundle'
}
}
test {
useJUnitPlatform()
}

dependencies {
implementation project(path: ':iceberg-bundled-guava', configuration: 'shadow')
Expand Down
95 changes: 48 additions & 47 deletions mr/src/test/java/org/apache/iceberg/mr/TestCatalogs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@

import static org.apache.iceberg.types.Types.NestedField.required;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
import java.util.Properties;
import org.apache.hadoop.conf.Configuration;
Expand All @@ -39,12 +41,9 @@
import org.apache.iceberg.hadoop.HadoopTables;
import org.apache.iceberg.hive.HiveCatalog;
import org.apache.iceberg.types.Types;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

public class TestCatalogs {

Expand All @@ -54,9 +53,9 @@ public class TestCatalogs {

private Configuration conf;

@Rule public TemporaryFolder temp = new TemporaryFolder();
@TempDir private Path temp;

@Before
@BeforeEach
public void before() {
conf = new Configuration();
}
Expand All @@ -65,25 +64,25 @@ public void before() {
public void testLoadTableFromLocation() throws IOException {
conf.set(CatalogUtil.ICEBERG_CATALOG_TYPE, Catalogs.LOCATION);

Assertions.assertThatThrownBy(() -> Catalogs.loadTable(conf))
assertThatThrownBy(() -> Catalogs.loadTable(conf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Table location not set");

HadoopTables tables = new HadoopTables();
Table hadoopTable = tables.create(SCHEMA, temp.newFolder("hadoop_tables").toString());
Table hadoopTable = tables.create(SCHEMA, temp.resolve("hadoop_tables").toString());

conf.set(InputFormatConfig.TABLE_LOCATION, hadoopTable.location());

Assert.assertEquals(hadoopTable.location(), Catalogs.loadTable(conf).location());
assertThat(Catalogs.loadTable(conf).location()).isEqualTo(hadoopTable.location());
}

@Test
public void testLoadTableFromCatalog() throws IOException {
String defaultCatalogName = "default";
String warehouseLocation = temp.newFolder("hadoop", "warehouse").toString();
String warehouseLocation = temp.resolve("hadoop").resolve("warehouse").toString();
setCustomCatalogProperties(defaultCatalogName, warehouseLocation);

Assertions.assertThatThrownBy(() -> Catalogs.loadTable(conf))
assertThatThrownBy(() -> Catalogs.loadTable(conf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Table identifier not set");

Expand All @@ -92,28 +91,28 @@ public void testLoadTableFromCatalog() throws IOException {

conf.set(InputFormatConfig.TABLE_IDENTIFIER, "table");

Assert.assertEquals(hadoopCatalogTable.location(), Catalogs.loadTable(conf).location());
assertThat(Catalogs.loadTable(conf).location()).isEqualTo(hadoopCatalogTable.location());
}

@Test
public void testCreateDropTableToLocation() throws IOException {
Properties missingSchema = new Properties();
missingSchema.put("location", temp.newFolder("hadoop_tables").toString());
missingSchema.put("location", temp.resolve("hadoop_tables").toString());

Assertions.assertThatThrownBy(() -> Catalogs.createTable(conf, missingSchema))
assertThatThrownBy(() -> Catalogs.createTable(conf, missingSchema))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table schema not set");

conf.set(CatalogUtil.ICEBERG_CATALOG_TYPE, Catalogs.LOCATION);
Properties missingLocation = new Properties();
missingLocation.put(InputFormatConfig.TABLE_SCHEMA, SchemaParser.toJson(SCHEMA));

Assertions.assertThatThrownBy(() -> Catalogs.createTable(conf, missingLocation))
assertThatThrownBy(() -> Catalogs.createTable(conf, missingLocation))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table location not set");

Properties properties = new Properties();
properties.put("location", temp.getRoot() + "/hadoop_tables");
properties.put("location", temp.toFile() + "/hadoop_tables");
properties.put(InputFormatConfig.TABLE_SCHEMA, SchemaParser.toJson(SCHEMA));
properties.put(InputFormatConfig.PARTITION_SPEC, PartitionSpecParser.toJson(SPEC));
properties.put("dummy", "test");
Expand All @@ -123,20 +122,21 @@ public void testCreateDropTableToLocation() throws IOException {
HadoopTables tables = new HadoopTables();
Table table = tables.load(properties.getProperty("location"));

Assert.assertEquals(properties.getProperty("location"), table.location());
Assert.assertEquals(SchemaParser.toJson(SCHEMA), SchemaParser.toJson(table.schema()));
Assert.assertEquals(PartitionSpecParser.toJson(SPEC), PartitionSpecParser.toJson(table.spec()));
assertThat(table.location()).isEqualTo(properties.getProperty("location"));
assertThat(SchemaParser.toJson(table.schema())).isEqualTo(SchemaParser.toJson(SCHEMA));
assertThat(PartitionSpecParser.toJson(table.spec()))
.isEqualTo(PartitionSpecParser.toJson(SPEC));
assertThat(table.properties()).containsEntry("dummy", "test");

Assertions.assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties()))
assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties()))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table location not set");

Properties dropProperties = new Properties();
dropProperties.put("location", temp.getRoot() + "/hadoop_tables");
dropProperties.put("location", temp.toFile() + "/hadoop_tables");
Catalogs.dropTable(conf, dropProperties);

Assertions.assertThatThrownBy(() -> Catalogs.loadTable(conf, dropProperties))
assertThatThrownBy(() -> Catalogs.loadTable(conf, dropProperties))
.isInstanceOf(NoSuchTableException.class)
.hasMessage("Table does not exist at location: " + properties.getProperty("location"));
}
Expand All @@ -145,22 +145,22 @@ public void testCreateDropTableToLocation() throws IOException {
public void testCreateDropTableToCatalog() throws IOException {
TableIdentifier identifier = TableIdentifier.of("test", "table");
String defaultCatalogName = "default";
String warehouseLocation = temp.newFolder("hadoop", "warehouse").toString();
String warehouseLocation = temp.resolve("hadoop").resolve("warehouse").toString();

setCustomCatalogProperties(defaultCatalogName, warehouseLocation);

Properties missingSchema = new Properties();
missingSchema.put("name", identifier.toString());
missingSchema.put(InputFormatConfig.CATALOG_NAME, defaultCatalogName);

Assertions.assertThatThrownBy(() -> Catalogs.createTable(conf, missingSchema))
assertThatThrownBy(() -> Catalogs.createTable(conf, missingSchema))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table schema not set");

Properties missingIdentifier = new Properties();
missingIdentifier.put(InputFormatConfig.TABLE_SCHEMA, SchemaParser.toJson(SCHEMA));
missingIdentifier.put(InputFormatConfig.CATALOG_NAME, defaultCatalogName);
Assertions.assertThatThrownBy(() -> Catalogs.createTable(conf, missingIdentifier))
assertThatThrownBy(() -> Catalogs.createTable(conf, missingIdentifier))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table identifier not set");

Expand All @@ -176,11 +176,12 @@ public void testCreateDropTableToCatalog() throws IOException {
HadoopCatalog catalog = new CustomHadoopCatalog(conf, warehouseLocation);
Table table = catalog.loadTable(identifier);

Assert.assertEquals(SchemaParser.toJson(SCHEMA), SchemaParser.toJson(table.schema()));
Assert.assertEquals(PartitionSpecParser.toJson(SPEC), PartitionSpecParser.toJson(table.spec()));
assertThat(SchemaParser.toJson(table.schema())).isEqualTo(SchemaParser.toJson(SCHEMA));
assertThat(PartitionSpecParser.toJson(table.spec()))
.isEqualTo(PartitionSpecParser.toJson(SPEC));
assertThat(table.properties()).containsEntry("dummy", "test");

Assertions.assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties()))
assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties()))
.isInstanceOf(NullPointerException.class)
.hasMessage("Table identifier not set");

Expand All @@ -189,7 +190,7 @@ public void testCreateDropTableToCatalog() throws IOException {
dropProperties.put(InputFormatConfig.CATALOG_NAME, defaultCatalogName);
Catalogs.dropTable(conf, dropProperties);

Assertions.assertThatThrownBy(() -> Catalogs.loadTable(conf, dropProperties))
assertThatThrownBy(() -> Catalogs.loadTable(conf, dropProperties))
.isInstanceOf(NoSuchTableException.class)
.hasMessage("Table does not exist: test.table");
}
Expand All @@ -198,11 +199,11 @@ public void testCreateDropTableToCatalog() throws IOException {
public void testLoadCatalogDefault() {
String catalogName = "barCatalog";
Optional<Catalog> defaultCatalog = Catalogs.loadCatalog(conf, catalogName);
Assert.assertTrue(defaultCatalog.isPresent());
Assertions.assertThat(defaultCatalog.get()).isInstanceOf(HiveCatalog.class);
assertThat(defaultCatalog).isPresent();
assertThat(defaultCatalog.get()).isInstanceOf(HiveCatalog.class);
Properties properties = new Properties();
properties.put(InputFormatConfig.CATALOG_NAME, catalogName);
Assert.assertTrue(Catalogs.hiveCatalog(conf, properties));
assertThat(Catalogs.hiveCatalog(conf, properties)).isTrue();
}

@Test
Expand All @@ -212,11 +213,11 @@ public void testLoadCatalogHive() {
InputFormatConfig.catalogPropertyConfigKey(catalogName, CatalogUtil.ICEBERG_CATALOG_TYPE),
CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE);
Optional<Catalog> hiveCatalog = Catalogs.loadCatalog(conf, catalogName);
Assert.assertTrue(hiveCatalog.isPresent());
Assertions.assertThat(hiveCatalog.get()).isInstanceOf(HiveCatalog.class);
assertThat(hiveCatalog).isPresent();
assertThat(hiveCatalog.get()).isInstanceOf(HiveCatalog.class);
Properties properties = new Properties();
properties.put(InputFormatConfig.CATALOG_NAME, catalogName);
Assert.assertTrue(Catalogs.hiveCatalog(conf, properties));
assertThat(Catalogs.hiveCatalog(conf, properties)).isTrue();
}

@Test
Expand All @@ -230,13 +231,13 @@ public void testLoadCatalogHadoop() {
catalogName, CatalogProperties.WAREHOUSE_LOCATION),
"/tmp/mylocation");
Optional<Catalog> hadoopCatalog = Catalogs.loadCatalog(conf, catalogName);
Assert.assertTrue(hadoopCatalog.isPresent());
Assertions.assertThat(hadoopCatalog.get()).isInstanceOf(HadoopCatalog.class);
Assert.assertEquals(
"HadoopCatalog{name=barCatalog, location=/tmp/mylocation}", hadoopCatalog.get().toString());
assertThat(hadoopCatalog).isPresent();
assertThat(hadoopCatalog.get()).isInstanceOf(HadoopCatalog.class);
assertThat(hadoopCatalog.get().toString())
.isEqualTo("HadoopCatalog{name=barCatalog, location=/tmp/mylocation}");
Properties properties = new Properties();
properties.put(InputFormatConfig.CATALOG_NAME, catalogName);
Assert.assertFalse(Catalogs.hiveCatalog(conf, properties));
assertThat(Catalogs.hiveCatalog(conf, properties)).isFalse();
}

@Test
Expand All @@ -250,16 +251,16 @@ public void testLoadCatalogCustom() {
catalogName, CatalogProperties.WAREHOUSE_LOCATION),
"/tmp/mylocation");
Optional<Catalog> customHadoopCatalog = Catalogs.loadCatalog(conf, catalogName);
Assert.assertTrue(customHadoopCatalog.isPresent());
Assertions.assertThat(customHadoopCatalog.get()).isInstanceOf(CustomHadoopCatalog.class);
assertThat(customHadoopCatalog).isPresent();
assertThat(customHadoopCatalog.get()).isInstanceOf(CustomHadoopCatalog.class);
Properties properties = new Properties();
properties.put(InputFormatConfig.CATALOG_NAME, catalogName);
Assert.assertFalse(Catalogs.hiveCatalog(conf, properties));
assertThat(Catalogs.hiveCatalog(conf, properties)).isFalse();
}

@Test
public void testLoadCatalogLocation() {
Assert.assertFalse(Catalogs.loadCatalog(conf, Catalogs.ICEBERG_HADOOP_TABLE_NAME).isPresent());
assertThat(Catalogs.loadCatalog(conf, Catalogs.ICEBERG_HADOOP_TABLE_NAME)).isNotPresent();
}

@Test
Expand All @@ -269,7 +270,7 @@ public void testLoadCatalogUnknown() {
InputFormatConfig.catalogPropertyConfigKey(catalogName, CatalogUtil.ICEBERG_CATALOG_TYPE),
"fooType");

Assertions.assertThatThrownBy(() -> Catalogs.loadCatalog(conf, catalogName))
assertThatThrownBy(() -> Catalogs.loadCatalog(conf, catalogName))
.isInstanceOf(UnsupportedOperationException.class)
.hasMessage("Unknown catalog type: fooType");
}
Expand Down
21 changes: 9 additions & 12 deletions mr/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergTestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.iceberg.mr.hive;

import static org.apache.iceberg.types.Types.NestedField.optional;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -63,7 +64,6 @@
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.ByteBuffers;
import org.junit.Assert;

public class HiveIcebergTestUtils {
// TODO: Can this be a constant all around the Iceberg tests?
Expand Down Expand Up @@ -218,13 +218,10 @@ public static void assertEquals(Record expected, Record actual) {
for (int i = 0; i < expected.size(); ++i) {
if (expected.get(i) instanceof OffsetDateTime) {
// For OffsetDateTime we just compare the actual instant
Assert.assertEquals(
((OffsetDateTime) expected.get(i)).toInstant(),
((OffsetDateTime) actual.get(i)).toInstant());
} else if (expected.get(i) instanceof byte[]) {
Assert.assertArrayEquals((byte[]) expected.get(i), (byte[]) actual.get(i));
assertThat(((OffsetDateTime) actual.get(i)).toInstant())
.isEqualTo(((OffsetDateTime) expected.get(i)).toInstant());
} else {
Assert.assertEquals(expected.get(i), actual.get(i));
assertThat(actual.get(i)).isEqualTo(expected.get(i));
}
}
}
Expand Down Expand Up @@ -265,7 +262,7 @@ public static void validateData(List<Record> expected, List<Record> actual, int
sortedExpected.sort(Comparator.comparingLong(record -> (Long) record.get(sortBy)));
sortedActual.sort(Comparator.comparingLong(record -> (Long) record.get(sortBy)));

Assert.assertEquals(sortedExpected.size(), sortedActual.size());
assertThat(sortedActual).hasSameSizeAs(sortedExpected);
for (int i = 0; i < sortedExpected.size(); ++i) {
assertEquals(sortedExpected.get(i), sortedActual.get(i));
}
Expand All @@ -288,9 +285,9 @@ public static void validateFiles(Table table, Configuration conf, JobID jobId, i
.filter(path -> !path.getFileName().toString().startsWith("."))
.collect(Collectors.toList());

Assert.assertEquals(dataFileNum, dataFiles.size());
Assert.assertFalse(
new File(HiveIcebergOutputCommitter.generateJobLocation(table.location(), conf, jobId))
.exists());
assertThat(dataFiles).hasSize(dataFileNum);
assertThat(
new File(HiveIcebergOutputCommitter.generateJobLocation(table.location(), conf, jobId)))
.doesNotExist();
}
}
Loading

0 comments on commit c394a07

Please sign in to comment.