From 12359eb979450a89c1e9a4bf6809067710eac2b1 Mon Sep 17 00:00:00 2001 From: Hacking-Dev-Ideas Date: Sat, 17 Jun 2023 17:36:49 +0100 Subject: [PATCH 1/2] Aliyun: Migrate tests to junit5 in client factory --- .../aliyun/TestAliyunClientFactories.java | 29 +++++-------------- build.gradle | 3 ++ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java b/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java index fa071e86051f..7a884dafcfb3 100644 --- a/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java +++ b/aliyun/src/test/java/org/apache/iceberg/aliyun/TestAliyunClientFactories.java @@ -22,43 +22,30 @@ import java.util.Map; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.junit.Assert; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; public class TestAliyunClientFactories { @Test public void testLoadDefault() { - Assert.assertEquals( - "Default client should be singleton", - AliyunClientFactories.defaultFactory(), - AliyunClientFactories.defaultFactory()); + assertThat(AliyunClientFactories.defaultFactory()).as("Default client should be singleton").isEqualTo(AliyunClientFactories.defaultFactory()); AliyunClientFactory defaultFactory = AliyunClientFactories.from(Maps.newHashMap()); - Assert.assertTrue( - "Should load default when factory impl not configured", - defaultFactory instanceof AliyunClientFactories.DefaultAliyunClientFactory); - Assert.assertNull( - "Should have no Aliyun properties set", defaultFactory.aliyunProperties().accessKeyId()); + assertThat(defaultFactory instanceof AliyunClientFactories.DefaultAliyunClientFactory).as("Should load default when factory impl not configured").isTrue(); + assertThat(defaultFactory.aliyunProperties().accessKeyId()).as("Should have no Aliyun properties set").isNull(); AliyunClientFactory defaultFactoryWithConfig = AliyunClientFactories.from(ImmutableMap.of(AliyunProperties.CLIENT_ACCESS_KEY_ID, "key")); - Assert.assertTrue( - "Should load default when factory impl not configured", - defaultFactoryWithConfig instanceof AliyunClientFactories.DefaultAliyunClientFactory); - Assert.assertEquals( - "Should have access key set", - "key", - defaultFactoryWithConfig.aliyunProperties().accessKeyId()); + assertThat(defaultFactoryWithConfig instanceof AliyunClientFactories.DefaultAliyunClientFactory).as("Should load default when factory impl not configured").isTrue(); + assertThat(defaultFactoryWithConfig.aliyunProperties().accessKeyId()).as("Should have access key set").isEqualTo("key"); } @Test public void testLoadCustom() { Map properties = Maps.newHashMap(); properties.put(AliyunProperties.CLIENT_FACTORY, CustomFactory.class.getName()); - Assert.assertTrue( - "Should load custom class", - AliyunClientFactories.from(properties) instanceof CustomFactory); + assertThat(AliyunClientFactories.from(properties) instanceof CustomFactory).as("Should load custom class").isTrue(); } public static class CustomFactory implements AliyunClientFactory { diff --git a/build.gradle b/build.gradle index 805f708359e1..245d51826853 100644 --- a/build.gradle +++ b/build.gradle @@ -424,6 +424,9 @@ project(':iceberg-data') { } project(':iceberg-aliyun') { + test { + useJUnitPlatform() + } dependencies { implementation project(path: ':iceberg-bundled-guava', configuration: 'shadow') api project(':iceberg-api') From fb6ae48f03c32e0b8956506f78661cfcda3954a8 Mon Sep 17 00:00:00 2001 From: Hacking-Dev-Ideas Date: Tue, 20 Jun 2023 22:11:51 +0100 Subject: [PATCH 2/2] arrow: Migrate tests to junit5 --- .../iceberg/arrow/ArrowSchemaUtilTest.java | 76 +++++++++---------- .../arrow/vectorized/ArrowReaderTest.java | 25 +++--- .../parquet/DecimalVectorUtilTest.java | 18 ++--- 3 files changed, 60 insertions(+), 59 deletions(-) diff --git a/arrow/src/test/java/org/apache/iceberg/arrow/ArrowSchemaUtilTest.java b/arrow/src/test/java/org/apache/iceberg/arrow/ArrowSchemaUtilTest.java index 7568abd46009..286c68c92e0c 100644 --- a/arrow/src/test/java/org/apache/iceberg/arrow/ArrowSchemaUtilTest.java +++ b/arrow/src/test/java/org/apache/iceberg/arrow/ArrowSchemaUtilTest.java @@ -34,8 +34,8 @@ import org.apache.iceberg.types.Types.StringType; import org.apache.iceberg.types.Types.TimeType; import org.apache.iceberg.types.Types.TimestampType; -import org.junit.Assert; -import org.junit.Test; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; public class ArrowSchemaUtilTest { @@ -97,86 +97,86 @@ public void convertComplex() { MapType.ofOptional( 4, 5, StringType.get(), ListType.ofOptional(6, TimestampType.withoutZone())))); org.apache.arrow.vector.types.pojo.Schema arrow = ArrowSchemaUtil.convert(iceberg); - Assert.assertEquals(iceberg.columns().size(), arrow.getFields().size()); + Assertions.assertEquals(iceberg.columns().size(), arrow.getFields().size()); } private void validate(Schema iceberg, org.apache.arrow.vector.types.pojo.Schema arrow) { - Assert.assertEquals(iceberg.columns().size(), arrow.getFields().size()); + Assertions.assertEquals(iceberg.columns().size(), arrow.getFields().size()); for (Types.NestedField nf : iceberg.columns()) { Field field = arrow.findField(nf.name()); - Assert.assertNotNull("Missing filed: " + nf, field); + Assertions.assertNotNull(field, "Missing filed: " + nf); validate(nf.type(), field, nf.isOptional()); } } private void validate(Type iceberg, Field field, boolean optional) { ArrowType arrowType = field.getType(); - Assert.assertEquals(optional, field.isNullable()); + Assertions.assertEquals(optional, field.isNullable()); switch (iceberg.typeId()) { case BOOLEAN: - Assert.assertEquals(BOOLEAN_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Bool, arrowType.getTypeID()); + Assertions.assertEquals(BOOLEAN_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Bool, arrowType.getTypeID()); break; case INTEGER: - Assert.assertEquals(INTEGER_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Int, arrowType.getTypeID()); + Assertions.assertEquals(INTEGER_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Int, arrowType.getTypeID()); break; case LONG: - Assert.assertEquals(LONG_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Int, arrowType.getTypeID()); + Assertions.assertEquals(LONG_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Int, arrowType.getTypeID()); break; case FLOAT: - Assert.assertEquals(FLOAT_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.FloatingPoint, arrowType.getTypeID()); + Assertions.assertEquals(FLOAT_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.FloatingPoint, arrowType.getTypeID()); break; case DOUBLE: - Assert.assertEquals(DOUBLE_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.FloatingPoint, arrowType.getTypeID()); + Assertions.assertEquals(DOUBLE_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.FloatingPoint, arrowType.getTypeID()); break; case DATE: - Assert.assertEquals(DATE_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Date, arrowType.getTypeID()); + Assertions.assertEquals(DATE_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Date, arrowType.getTypeID()); break; case TIME: - Assert.assertEquals(TIME_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Time, arrowType.getTypeID()); + Assertions.assertEquals(TIME_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Time, arrowType.getTypeID()); break; case TIMESTAMP: - Assert.assertEquals(TIMESTAMP_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Timestamp, arrowType.getTypeID()); + Assertions.assertEquals(TIMESTAMP_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Timestamp, arrowType.getTypeID()); break; case STRING: - Assert.assertEquals(STRING_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Utf8, arrowType.getTypeID()); + Assertions.assertEquals(STRING_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Utf8, arrowType.getTypeID()); break; case FIXED: - Assert.assertEquals(FIXED_WIDTH_BINARY_FIELD, field.getName()); - Assert.assertEquals(ArrowType.FixedSizeBinary.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(FIXED_WIDTH_BINARY_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.FixedSizeBinary.TYPE_TYPE, arrowType.getTypeID()); break; case BINARY: - Assert.assertEquals(BINARY_FIELD, field.getName()); - Assert.assertEquals(ArrowType.Binary.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(BINARY_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.Binary.TYPE_TYPE, arrowType.getTypeID()); break; case DECIMAL: - Assert.assertEquals(DECIMAL_FIELD, field.getName()); - Assert.assertEquals(ArrowType.Decimal.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(DECIMAL_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.Decimal.TYPE_TYPE, arrowType.getTypeID()); break; case STRUCT: - Assert.assertEquals(STRUCT_FIELD, field.getName()); - Assert.assertEquals(ArrowType.Struct.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(STRUCT_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.Struct.TYPE_TYPE, arrowType.getTypeID()); break; case LIST: - Assert.assertEquals(LIST_FIELD, field.getName()); - Assert.assertEquals(ArrowType.List.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(LIST_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.List.TYPE_TYPE, arrowType.getTypeID()); break; case MAP: - Assert.assertEquals(MAP_FIELD, field.getName()); - Assert.assertEquals(ArrowType.ArrowTypeID.Map, arrowType.getTypeID()); + Assertions.assertEquals(MAP_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.ArrowTypeID.Map, arrowType.getTypeID()); break; case UUID: - Assert.assertEquals(UUID_FIELD, field.getName()); - Assert.assertEquals(ArrowType.FixedSizeBinary.TYPE_TYPE, arrowType.getTypeID()); + Assertions.assertEquals(UUID_FIELD, field.getName()); + Assertions.assertEquals(ArrowType.FixedSizeBinary.TYPE_TYPE, arrowType.getTypeID()); break; default: throw new UnsupportedOperationException("Check not implemented for type: " + iceberg); diff --git a/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/ArrowReaderTest.java b/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/ArrowReaderTest.java index 983c5ba1b56d..652a8292977e 100644 --- a/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/ArrowReaderTest.java +++ b/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/ArrowReaderTest.java @@ -19,6 +19,7 @@ package org.apache.iceberg.arrow.vectorized; import static org.apache.iceberg.Files.localInput; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -225,7 +226,7 @@ public void testReadRangeFilterEmptyResult() throws Exception { numRoots++; } } - assertEquals(0, numRoots); + assertThat(numRoots).isEqualTo(0); } /** @@ -322,14 +323,14 @@ private void readAndCheckArrowResult( for (ColumnarBatch batch : itr) { List expectedRows = rowsWritten.subList(rowIndex, rowIndex + numRowsPerRoot); VectorSchemaRoot root = batch.createVectorSchemaRootFromVectors(); - assertEquals(createExpectedArrowSchema(columnSet), root.getSchema()); + assertThat(root.getSchema()).isEqualTo(createExpectedArrowSchema(columnSet)); checkAllVectorTypes(root, columnSet); checkAllVectorValues(numRowsPerRoot, expectedRows, root, columnSet); rowIndex += numRowsPerRoot; totalRows += root.getRowCount(); } } - assertEquals(expectedTotalRows, totalRows); + assertThat(totalRows).isEqualTo(expectedTotalRows); } private void readAndCheckHasNextIsIdempotent( @@ -349,12 +350,12 @@ private void readAndCheckHasNextIsIdempotent( // Call hasNext() a few extra times. // This should not affect the total number of rows read. for (int i = 0; i < numExtraCallsToHasNext; i++) { - assertTrue(iterator.hasNext()); + assertThat(iterator.hasNext()).isTrue(); } ColumnarBatch batch = iterator.next(); VectorSchemaRoot root = batch.createVectorSchemaRootFromVectors(); - assertEquals(createExpectedArrowSchema(columnSet), root.getSchema()); + assertThat(root.getSchema()).isEqualTo(createExpectedArrowSchema(columnSet)); checkAllVectorTypes(root, columnSet); List expectedRows = rowsWritten.subList(rowIndex, rowIndex + numRowsPerRoot); checkAllVectorValues(numRowsPerRoot, expectedRows, root, columnSet); @@ -362,7 +363,7 @@ private void readAndCheckHasNextIsIdempotent( totalRows += root.getRowCount(); } } - assertEquals(expectedTotalRows, totalRows); + assertThat(totalRows).isEqualTo(expectedTotalRows); } @SuppressWarnings("MethodLength") @@ -378,8 +379,8 @@ private void checkColumnarBatch( } Set columnSet = columnNameToIndex.keySet(); - assertEquals(expectedNumRows, batch.numRows()); - assertEquals(columns.size(), batch.numCols()); + assertThat(batch.numRows()).isEqualTo(expectedNumRows); + assertThat(batch.numCols()).isEqualTo(columns.size()); checkColumnarArrayValues( expectedNumRows, @@ -872,7 +873,7 @@ private List createConstantRecordsForDate(Schema schema, LocalDat private DataFile writeParquetFile(Table table, List records) throws IOException { rowsWritten.addAll(records); File parquetFile = temp.newFile(); - assertTrue(parquetFile.delete()); + assertThat(parquetFile.delete()).isTrue(); FileAppender appender = Parquet.write(Files.localOutput(parquetFile)) .schema(table.schema()) @@ -949,7 +950,7 @@ private void checkAllVectorTypes(VectorSchemaRoot root, Set columnSet) { private void assertEqualsForField( VectorSchemaRoot root, Set columnSet, String columnName, Class expected) { if (columnSet.contains(columnName)) { - assertEquals(expected, root.getVector(columnName).getClass()); + assertThat(root.getVector(columnName).getClass()).isEqualTo(expected); } } @@ -959,7 +960,7 @@ private void checkAllVectorValues( List expectedRows, VectorSchemaRoot root, Set columnSet) { - assertEquals(expectedNumRows, root.getRowCount()); + assertThat(root.getRowCount()).isEqualTo(expectedNumRows); checkVectorValues( expectedNumRows, @@ -1196,7 +1197,7 @@ private static void checkVectorValues( BiFunction vectorValueExtractor) { if (columnSet.contains(columnName)) { FieldVector vector = root.getVector(columnName); - assertEquals(expectedNumRows, vector.getValueCount()); + assertThat(vector.getValueCount()).isEqualTo(expectedNumRows); for (int i = 0; i < expectedNumRows; i++) { Object expectedValue = expectedValueExtractor.apply(expectedRows, i); Object actualValue = vectorValueExtractor.apply(vector, i); diff --git a/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/parquet/DecimalVectorUtilTest.java b/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/parquet/DecimalVectorUtilTest.java index 4e78bafd0a1a..49f1f52b4ddf 100644 --- a/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/parquet/DecimalVectorUtilTest.java +++ b/arrow/src/test/java/org/apache/iceberg/arrow/vectorized/parquet/DecimalVectorUtilTest.java @@ -22,7 +22,7 @@ import java.math.BigInteger; import org.assertj.core.api.Assertions; -import org.junit.Test; +import org.junit.jupiter.api.Test; public class DecimalVectorUtilTest { @@ -32,9 +32,9 @@ public void testPadBigEndianBytes() { byte[] bytes = bigInt.toByteArray(); byte[] paddedBytes = DecimalVectorUtil.padBigEndianBytes(bytes, 16); - assertEquals(16, paddedBytes.length); + org.junit.jupiter.api.Assertions.assertEquals(16, paddedBytes.length); BigInteger result = new BigInteger(paddedBytes); - assertEquals(bigInt, result); + org.junit.jupiter.api.Assertions.assertEquals(bigInt, result); } @Test @@ -43,9 +43,9 @@ public void testPadBigEndianBytesNegative() { byte[] bytes = bigInt.toByteArray(); byte[] paddedBytes = DecimalVectorUtil.padBigEndianBytes(bytes, 16); - assertEquals(16, paddedBytes.length); + org.junit.jupiter.api.Assertions.assertEquals(16, paddedBytes.length); BigInteger result = new BigInteger(paddedBytes); - assertEquals(bigInt, result); + org.junit.jupiter.api.Assertions.assertEquals(bigInt, result); } @Test @@ -53,16 +53,16 @@ public void testPadBigEndianBytesZero() { byte[] bytes = BigInteger.ZERO.toByteArray(); byte[] paddedBytes = DecimalVectorUtil.padBigEndianBytes(bytes, 16); - assertEquals(16, paddedBytes.length); + org.junit.jupiter.api.Assertions.assertEquals(16, paddedBytes.length); BigInteger result = new BigInteger(paddedBytes); - assertEquals(BigInteger.ZERO, result); + org.junit.jupiter.api.Assertions.assertEquals(BigInteger.ZERO, result); bytes = new byte[0]; paddedBytes = DecimalVectorUtil.padBigEndianBytes(bytes, 16); - assertEquals(16, paddedBytes.length); + org.junit.jupiter.api.Assertions.assertEquals(16, paddedBytes.length); result = new BigInteger(paddedBytes); - assertEquals(BigInteger.ZERO, result); + org.junit.jupiter.api.Assertions.assertEquals(BigInteger.ZERO, result); } @Test