Skip to content

Commit

Permalink
Core: Migrate tests to JUnit5 (#10014)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomtongue authored Mar 21, 2024
1 parent 7151401 commit 59ffa33
Show file tree
Hide file tree
Showing 10 changed files with 667 additions and 910 deletions.
209 changes: 82 additions & 127 deletions core/src/test/java/org/apache/iceberg/TestReplacePartitions.java

Large diffs are not rendered by default.

47 changes: 24 additions & 23 deletions core/src/test/java/org/apache/iceberg/TestScanDataFileColumns.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,22 @@

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

import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.file.Files;
import java.nio.file.Path;
import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.hadoop.HadoopTables;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
import org.apache.iceberg.types.Types;
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 TestScanDataFileColumns {
private static final Schema SCHEMA =
Expand All @@ -44,15 +45,15 @@ public class TestScanDataFileColumns {
private static final Configuration CONF = new Configuration();
private static final Tables TABLES = new HadoopTables(CONF);

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

private String tableLocation = null;
private Table table = null;

@Before
@BeforeEach
public void createTables() throws IOException {
File location = temp.newFolder("shared");
Assert.assertTrue(location.delete());
File location = Files.createTempDirectory(temp, "junit").toFile();
assertThat(location.delete()).isTrue();
this.tableLocation = location.toString();
this.table =
TABLES.create(
Expand Down Expand Up @@ -115,22 +116,22 @@ public void createTables() throws IOException {
public void testColumnStatsIgnored() {
// stats columns should be suppressed by default
for (FileScanTask fileTask : table.newScan().planFiles()) {
Assert.assertNull(fileTask.file().valueCounts());
Assert.assertNull(fileTask.file().nullValueCounts());
Assert.assertNull(fileTask.file().lowerBounds());
Assert.assertNull(fileTask.file().upperBounds());
assertThat(fileTask.file().valueCounts()).isNull();
assertThat(fileTask.file().nullValueCounts()).isNull();
assertThat(fileTask.file().lowerBounds()).isNull();
assertThat(fileTask.file().upperBounds()).isNull();
}
}

@Test
public void testColumnStatsLoading() {
// stats columns should be suppressed by default
for (FileScanTask fileTask : table.newScan().includeColumnStats().planFiles()) {
Assert.assertEquals(2, fileTask.file().valueCounts().size());
Assert.assertEquals(2, fileTask.file().nullValueCounts().size());
Assert.assertEquals(2, fileTask.file().lowerBounds().size());
Assert.assertEquals(2, fileTask.file().upperBounds().size());
Assert.assertEquals(2, fileTask.file().columnSizes().size());
assertThat(fileTask.file().valueCounts()).hasSize(2);
assertThat(fileTask.file().nullValueCounts()).hasSize(2);
assertThat(fileTask.file().lowerBounds()).hasSize(2);
assertThat(fileTask.file().upperBounds()).hasSize(2);
assertThat(fileTask.file().columnSizes()).hasSize(2);
}
}

Expand All @@ -139,11 +140,11 @@ public void testColumnStatsPartial() {
// stats columns should be suppressed by default
for (FileScanTask fileTask :
table.newScan().includeColumnStats(ImmutableSet.of("id")).planFiles()) {
Assert.assertEquals(1, fileTask.file().valueCounts().size());
Assert.assertEquals(1, fileTask.file().nullValueCounts().size());
Assert.assertEquals(1, fileTask.file().lowerBounds().size());
Assert.assertEquals(1, fileTask.file().upperBounds().size());
Assert.assertEquals(1, fileTask.file().columnSizes().size());
assertThat(fileTask.file().valueCounts()).hasSize(1);
assertThat(fileTask.file().nullValueCounts()).hasSize(1);
assertThat(fileTask.file().lowerBounds()).hasSize(1);
assertThat(fileTask.file().upperBounds()).hasSize(1);
assertThat(fileTask.file().columnSizes()).hasSize(1);
}
}

Expand Down
133 changes: 59 additions & 74 deletions core/src/test/java/org/apache/iceberg/TestScanSummary.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,24 @@
import static org.apache.iceberg.expressions.Expressions.greaterThanOrEqual;
import static org.apache.iceberg.expressions.Expressions.lessThan;
import static org.apache.iceberg.expressions.Expressions.lessThanOrEqual;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.Arrays;
import java.util.List;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.util.Pair;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)
public class TestScanSummary extends TableTestBase {
@Parameterized.Parameters(name = "formatVersion = {0}")
public static Object[] parameters() {
return new Object[] {1, 2};
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;

@ExtendWith(ParameterizedTestExtension.class)
public class TestScanSummary extends TestBase {
@Parameters(name = "formatVersion = {0}")
protected static List<Object> parameters() {
return Arrays.asList(1, 2);
}

public TestScanSummary(int formatVersion) {
super(formatVersion);
}

@Test
@TestTemplate
public void testSnapshotTimeRangeValidation() {
long t0 = System.currentTimeMillis();

Expand Down Expand Up @@ -76,12 +72,8 @@ public void testSnapshotTimeRangeValidation() {
// expire the first snapshot
table.expireSnapshots().expireOlderThan(t1).commit();

Assert.assertEquals(
"Should have one snapshot", 1, Lists.newArrayList(table.snapshots()).size());
Assert.assertEquals(
"Snapshot should be the second snapshot created",
secondSnapshotId,
table.currentSnapshot().snapshotId());
assertThat(table.snapshots()).hasSize(1);
assertThat(table.currentSnapshot().snapshotId()).isEqualTo(secondSnapshotId);

// this should include the first snapshot, but it was removed from the dataset
TableScan scan =
Expand All @@ -90,75 +82,68 @@ public void testSnapshotTimeRangeValidation() {
.filter(greaterThanOrEqual("dateCreated", t0))
.filter(lessThan("dateCreated", t2));

Assertions.assertThatThrownBy(() -> new ScanSummary.Builder(scan).build())
assertThatThrownBy(() -> new ScanSummary.Builder(scan).build())
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot satisfy time filters: time range may include expired snapshots");
}

@Test
@TestTemplate
public void testTimestampRanges() {
long lower = 1542750188523L;
long upper = 1542750695131L;

Assert.assertEquals(
"Should use inclusive bound",
Pair.of(Long.MIN_VALUE, upper),
timestampRange(ImmutableList.of(lessThanOrEqual("ts_ms", upper))));

Assert.assertEquals(
"Should use lower value for upper bound",
Pair.of(Long.MIN_VALUE, upper),
timestampRange(
ImmutableList.of(
lessThanOrEqual("ts_ms", upper + 918234), lessThanOrEqual("ts_ms", upper))));

Assert.assertEquals(
"Should make upper bound inclusive",
Pair.of(Long.MIN_VALUE, upper - 1),
timestampRange(ImmutableList.of(lessThan("ts_ms", upper))));

Assert.assertEquals(
"Should use inclusive bound",
Pair.of(lower, Long.MAX_VALUE),
timestampRange(ImmutableList.of(greaterThanOrEqual("ts_ms", lower))));

Assert.assertEquals(
"Should use upper value for lower bound",
Pair.of(lower, Long.MAX_VALUE),
timestampRange(
ImmutableList.of(
greaterThanOrEqual("ts_ms", lower - 918234), greaterThanOrEqual("ts_ms", lower))));

Assert.assertEquals(
"Should make lower bound inclusive",
Pair.of(lower + 1, Long.MAX_VALUE),
timestampRange(ImmutableList.of(greaterThan("ts_ms", lower))));

Assert.assertEquals(
"Should set both bounds for equals",
Pair.of(lower, lower),
timestampRange(ImmutableList.of(equal("ts_ms", lower))));

Assert.assertEquals(
"Should set both bounds",
Pair.of(lower, upper - 1),
timestampRange(
ImmutableList.of(greaterThanOrEqual("ts_ms", lower), lessThan("ts_ms", upper))));
assertThat(timestampRange(ImmutableList.of(lessThanOrEqual("ts_ms", upper))))
.isEqualTo(Pair.of(Long.MIN_VALUE, upper));

assertThat(
timestampRange(
ImmutableList.of(
lessThanOrEqual("ts_ms", upper + 918234), lessThanOrEqual("ts_ms", upper))))
.as("Should use lower value for upper bound")
.isEqualTo(Pair.of(Long.MIN_VALUE, upper));

assertThat(timestampRange(ImmutableList.of(lessThan("ts_ms", upper))))
.as("Should make upper bound inclusive")
.isEqualTo(Pair.of(Long.MIN_VALUE, upper - 1));

assertThat(timestampRange(ImmutableList.of(greaterThanOrEqual("ts_ms", lower))))
.isEqualTo(Pair.of(lower, Long.MAX_VALUE));

assertThat(
timestampRange(
ImmutableList.of(
greaterThanOrEqual("ts_ms", lower - 918234),
greaterThanOrEqual("ts_ms", lower))))
.as("Should use upper value for lower bound")
.isEqualTo(Pair.of(lower, Long.MAX_VALUE));

assertThat(timestampRange(ImmutableList.of(greaterThan("ts_ms", lower))))
.as("Should make lower bound inclusive")
.isEqualTo(Pair.of(lower + 1, Long.MAX_VALUE));

assertThat(timestampRange(ImmutableList.of(equal("ts_ms", lower))))
.isEqualTo(Pair.of(lower, lower));

assertThat(
timestampRange(
ImmutableList.of(greaterThanOrEqual("ts_ms", lower), lessThan("ts_ms", upper))))
.as("Should set both bounds and make upper bound inclusive")
.isEqualTo(Pair.of(lower, upper - 1));

// >= lower and < lower is an empty range
Assertions.assertThatThrownBy(
assertThatThrownBy(
() ->
timestampRange(
ImmutableList.of(greaterThanOrEqual("ts_ms", lower), lessThan("ts_ms", lower))))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("No timestamps can match filters");
}

@Test
@TestTemplate
public void testToMillis() {
long millis = 1542750947417L;
Assert.assertEquals(1542750947000L, toMillis(millis / 1000));
Assert.assertEquals(1542750947417L, toMillis(millis));
Assert.assertEquals(1542750947417L, toMillis(millis * 1000 + 918));
assertThat(toMillis(millis / 1000)).isEqualTo(1542750947000L);
assertThat(toMillis(millis)).isEqualTo(1542750947417L);
assertThat(toMillis(millis * 1000 + 918)).isEqualTo(1542750947417L);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@
package org.apache.iceberg;

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

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import org.apache.avro.generic.GenericData;
Expand All @@ -33,15 +37,12 @@
import org.apache.iceberg.io.OutputFile;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.types.Types;
import org.junit.After;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;

@ExtendWith(ParameterizedTestExtension.class)
public class TestScansAndSchemaEvolution {
private static final Schema SCHEMA =
new Schema(
Expand All @@ -52,18 +53,14 @@ public class TestScansAndSchemaEvolution {
private static final PartitionSpec SPEC =
PartitionSpec.builderFor(SCHEMA).identity("part").build();

@Parameterized.Parameters(name = "formatVersion = {0}")
public static Object[] parameters() {
return new Object[] {1, 2};
@Parameters(name = "formatVersion = {0}")
protected static List<Object> parameters() {
return Arrays.asList(1, 2);
}

public final int formatVersion;

public TestScansAndSchemaEvolution(int formatVersion) {
this.formatVersion = formatVersion;
}
@Parameter private int formatVersion;

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

private DataFile createDataFile(String partValue) throws IOException {
List<GenericData.Record> expected = RandomAvroData.generate(SCHEMA, 100, 0L);
Expand All @@ -87,15 +84,15 @@ private DataFile createDataFile(String partValue) throws IOException {
.build();
}

@After
@AfterEach
public void cleanupTables() {
TestTables.clearTables();
}

@Test
@TestTemplate
public void testPartitionSourceRename() throws IOException {
File location = temp.newFolder();
Assert.assertTrue(location.delete()); // should be created by table create
File location = Files.createTempDirectory(temp, "junit").toFile();
assertThat(location.delete()).isTrue(); // should be created by table create

Table table = TestTables.create(location, "test", SCHEMA, SPEC, formatVersion);

Expand All @@ -107,13 +104,13 @@ public void testPartitionSourceRename() throws IOException {
List<FileScanTask> tasks =
Lists.newArrayList(table.newScan().filter(Expressions.equal("part", "one")).planFiles());

Assert.assertEquals("Should produce 1 matching file task", 1, tasks.size());
assertThat(tasks).hasSize(1);

table.updateSchema().renameColumn("part", "p").commit();

// plan the scan using the new name in a filter
tasks = Lists.newArrayList(table.newScan().filter(Expressions.equal("p", "one")).planFiles());

Assert.assertEquals("Should produce 1 matching file task", 1, tasks.size());
assertThat(tasks).hasSize(1);
}
}
Loading

0 comments on commit 59ffa33

Please sign in to comment.