Skip to content

Commit

Permalink
Core: Migrate tests to JUnit5 (#9994)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomtongue
tomtongue authored Mar 20, 2024
1 parent f8d60ea commit aa17c0a
Show file tree
Hide file tree
Showing 6 changed files with 675 additions and 942 deletions.
115 changes: 51 additions & 64 deletions core/src/test/java/org/apache/iceberg/TestDeleteFiles.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,29 @@
package org.apache.iceberg;

import static org.apache.iceberg.util.SnapshotUtil.latestSnapshot;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assumptions.assumeThat;

import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.iceberg.ManifestEntry.Status;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.expressions.Expression;
import org.apache.iceberg.expressions.Expressions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.StructLikeWrapper;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.jupiter.api.TestTemplate;
import org.junit.jupiter.api.extension.ExtendWith;

@RunWith(Parameterized.class)
public class TestDeleteFiles extends TableTestBase {
@ExtendWith(ParameterizedTestExtension.class)
public class TestDeleteFiles extends TestBase {

private static final DataFile DATA_FILE_BUCKET_0_IDS_0_2 =
DataFiles.builder(SPEC)
Expand Down Expand Up @@ -78,53 +77,48 @@ public class TestDeleteFiles extends TableTestBase {
))
.build();

private final String branch;
@Parameter(index = 1)
private String branch;

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

public TestDeleteFiles(int formatVersion, String branch) {
super(formatVersion);
this.branch = branch;
}

@Test
@TestTemplate
public void testMultipleDeletes() {
commit(
table, table.newAppend().appendFile(FILE_A).appendFile(FILE_B).appendFile(FILE_C), branch);
Snapshot append = latestSnapshot(readMetadata(), branch);
Assert.assertEquals("Metadata should be at version 1", 1L, (long) version());
assertThat(version()).isEqualTo(1);
validateSnapshot(null, append, FILE_A, FILE_B, FILE_C);

commit(table, table.newDelete().deleteFile(FILE_A), branch);
Snapshot delete1 = latestSnapshot(readMetadata(), branch);

Assert.assertEquals("Metadata should be at version 2", 2L, (long) version());
Assert.assertEquals("Should have 1 manifest", 1, delete1.allManifests(FILE_IO).size());
assertThat(version()).isEqualTo(2);
assertThat(delete1.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
delete1.allManifests(table.io()).get(0),
ids(delete1.snapshotId(), append.snapshotId(), append.snapshotId()),
files(FILE_A, FILE_B, FILE_C),
statuses(Status.DELETED, Status.EXISTING, Status.EXISTING));

Snapshot delete2 = commit(table, table.newDelete().deleteFile(FILE_B), branch);
Assert.assertEquals("Metadata should be at version 3", 3L, (long) version());
Assert.assertEquals("Should have 1 manifest", 1, delete2.allManifests(FILE_IO).size());
assertThat(version()).isEqualTo(3);
assertThat(delete2.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
delete2.allManifests(FILE_IO).get(0),
ids(delete2.snapshotId(), append.snapshotId()),
files(FILE_B, FILE_C),
statuses(Status.DELETED, Status.EXISTING));
}

@Test
@TestTemplate
public void testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {
PartitionSpec spec = table.spec();

Expand Down Expand Up @@ -169,7 +163,7 @@ public void testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {
table.newFastAppend().appendFile(firstDataFile).appendFile(secondDataFile),
branch);

Assert.assertEquals("Should have 1 manifest", 1, initialSnapshot.allManifests(FILE_IO).size());
assertThat(initialSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
initialSnapshot.allManifests(FILE_IO).get(0),
ids(initialSnapshot.snapshotId(), initialSnapshot.snapshotId()),
Expand All @@ -178,7 +172,7 @@ public void testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {

// delete the first data file
Snapshot deleteSnapshot = commit(table, table.newDelete().deleteFile(firstDataFile), branch);
Assert.assertEquals("Should have 1 manifest", 1, deleteSnapshot.allManifests(FILE_IO).size());
assertThat(deleteSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
deleteSnapshot.allManifests(FILE_IO).get(0),
ids(deleteSnapshot.snapshotId(), initialSnapshot.snapshotId()),
Expand All @@ -190,15 +184,15 @@ public void testAlreadyDeletedFilesAreIgnoredDuringDeletesByRowFilter() {
Snapshot finalSnapshot =
commit(table, table.newDelete().deleteFromRowFilter(Expressions.lessThan("id", 7)), branch);

Assert.assertEquals("Should have 1 manifest", 1, finalSnapshot.allManifests(FILE_IO).size());
assertThat(finalSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
finalSnapshot.allManifests(FILE_IO).get(0),
ids(finalSnapshot.snapshotId()),
files(secondDataFile),
statuses(Status.DELETED));
}

@Test
@TestTemplate
public void testDeleteSomeFilesByRowFilterWithoutPartitionPredicates() {
// add both data files
Snapshot initialSnapshot =
Expand All @@ -210,7 +204,7 @@ public void testDeleteSomeFilesByRowFilterWithoutPartitionPredicates() {
.appendFile(DATA_FILE_BUCKET_0_IDS_8_10),
branch);

Assert.assertEquals("Should have 1 manifest", 1, initialSnapshot.allManifests(FILE_IO).size());
assertThat(initialSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
initialSnapshot.allManifests(FILE_IO).get(0),
ids(initialSnapshot.snapshotId(), initialSnapshot.snapshotId()),
Expand All @@ -222,15 +216,15 @@ public void testDeleteSomeFilesByRowFilterWithoutPartitionPredicates() {
commit(
table, table.newDelete().deleteFromRowFilter(Expressions.greaterThan("id", 5)), branch);

Assert.assertEquals("Should have 1 manifest", 1, deleteSnapshot.allManifests(FILE_IO).size());
assertThat(deleteSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
deleteSnapshot.allManifests(FILE_IO).get(0),
ids(initialSnapshot.snapshotId(), deleteSnapshot.snapshotId()),
files(DATA_FILE_BUCKET_0_IDS_0_2, DATA_FILE_BUCKET_0_IDS_8_10),
statuses(Status.EXISTING, Status.DELETED));
}

@Test
@TestTemplate
public void testDeleteSomeFilesByRowFilterWithCombinedPredicates() {
// add both data files
Snapshot initialSnapshot =
Expand All @@ -242,7 +236,7 @@ public void testDeleteSomeFilesByRowFilterWithCombinedPredicates() {
.appendFile(DATA_FILE_BUCKET_0_IDS_8_10),
branch);

Assert.assertEquals("Should have 1 manifest", 1, initialSnapshot.allManifests(FILE_IO).size());
assertThat(initialSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
initialSnapshot.allManifests(FILE_IO).get(0),
ids(initialSnapshot.snapshotId(), initialSnapshot.snapshotId()),
Expand All @@ -255,17 +249,17 @@ public void testDeleteSomeFilesByRowFilterWithCombinedPredicates() {
Expression predicate = Expressions.and(partPredicate, rowPredicate);
Snapshot deleteSnapshot =
commit(table, table.newDelete().deleteFromRowFilter(predicate), branch);
Assert.assertEquals("Should have 1 manifest", 1, deleteSnapshot.allManifests(FILE_IO).size());
assertThat(deleteSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
deleteSnapshot.allManifests(FILE_IO).get(0),
ids(initialSnapshot.snapshotId(), deleteSnapshot.snapshotId()),
files(DATA_FILE_BUCKET_0_IDS_0_2, DATA_FILE_BUCKET_0_IDS_8_10),
statuses(Status.EXISTING, Status.DELETED));
}

@Test
@TestTemplate
public void testCannotDeleteFileWhereNotAllRowsMatchPartitionFilter() {
Assume.assumeTrue(formatVersion == 2);
assumeThat(formatVersion).isEqualTo(2);

table
.updateSpec()
Expand All @@ -285,7 +279,7 @@ public void testCannotDeleteFileWhereNotAllRowsMatchPartitionFilter() {

commit(table, table.newFastAppend().appendFile(dataFile), branch);

Assertions.assertThatThrownBy(
assertThatThrownBy(
() ->
commit(
table,
Expand All @@ -295,18 +289,18 @@ public void testCannotDeleteFileWhereNotAllRowsMatchPartitionFilter() {
.hasMessageStartingWith("Cannot delete file where some, but not all, rows match filter");
}

@Test
@TestTemplate
public void testDeleteCaseSensitivity() {
commit(table, table.newFastAppend().appendFile(DATA_FILE_BUCKET_0_IDS_0_2), branch);

Expression rowFilter = Expressions.lessThan("iD", 5);

Assertions.assertThatThrownBy(
assertThatThrownBy(
() -> commit(table, table.newDelete().deleteFromRowFilter(rowFilter), branch))
.isInstanceOf(ValidationException.class)
.hasMessageStartingWith("Cannot find field 'iD'");

Assertions.assertThatThrownBy(
assertThatThrownBy(
() ->
commit(
table,
Expand All @@ -319,15 +313,15 @@ public void testDeleteCaseSensitivity() {
commit(
table, table.newDelete().deleteFromRowFilter(rowFilter).caseSensitive(false), branch);

Assert.assertEquals("Should have 1 manifest", 1, deleteSnapshot.allManifests(FILE_IO).size());
assertThat(deleteSnapshot.allManifests(FILE_IO)).hasSize(1);
validateManifestEntries(
deleteSnapshot.allManifests(FILE_IO).get(0),
ids(deleteSnapshot.snapshotId()),
files(DATA_FILE_BUCKET_0_IDS_0_2),
statuses(Status.DELETED));
}

@Test
@TestTemplate
public void testDeleteFilesOnIndependentBranches() {
String testBranch = "testBranch";
table.newAppend().appendFile(FILE_A).appendFile(FILE_B).appendFile(FILE_C).commit();
Expand Down Expand Up @@ -355,7 +349,7 @@ public void testDeleteFilesOnIndependentBranches() {
statuses(Status.EXISTING, Status.DELETED, Status.DELETED));
}

@Test
@TestTemplate
public void testDeleteWithCollision() {
Schema schema = new Schema(Types.NestedField.of(0, false, "x", Types.StringType.get()));
PartitionSpec spec = PartitionSpec.builderFor(schema).identity("x").build();
Expand All @@ -367,9 +361,8 @@ public void testDeleteWithCollision() {
PartitionData partitionTwo = new PartitionData(spec.partitionType());
partitionTwo.set(0, "BB");

Assert.assertEquals(
StructLikeWrapper.forType(spec.partitionType()).set(partitionOne).hashCode(),
StructLikeWrapper.forType(spec.partitionType()).set(partitionTwo).hashCode());
assertThat(StructLikeWrapper.forType(spec.partitionType()).set(partitionTwo).hashCode())
.isEqualTo(StructLikeWrapper.forType(spec.partitionType()).set(partitionOne).hashCode());

DataFile testFileOne =
DataFiles.builder(spec)
Expand All @@ -394,10 +387,7 @@ public void testDeleteWithCollision() {
.map(s -> ((PartitionData) s.partition()).copy())
.collect(Collectors.toList());

Assert.assertEquals(
"We should have both partitions",
ImmutableList.of(partitionOne, partitionTwo),
beforeDeletePartitions);
assertThat(beforeDeletePartitions).containsExactly(partitionOne, partitionTwo);

collisionTable.newDelete().deleteFromRowFilter(Expressions.equal("x", "BB")).commit();

Expand All @@ -406,13 +396,10 @@ public void testDeleteWithCollision() {
.map(s -> ((PartitionData) s.partition()).copy())
.collect(Collectors.toList());

Assert.assertEquals(
"We should have deleted partitionTwo",
ImmutableList.of(partitionOne),
afterDeletePartitions);
assertThat(afterDeletePartitions).containsExactly(partitionOne);
}

@Test
@TestTemplate
public void testDeleteValidateFileExistence() {
commit(table, table.newFastAppend().appendFile(FILE_B), branch);
Snapshot delete =
Expand All @@ -423,12 +410,12 @@ public void testDeleteValidateFileExistence() {
files(FILE_B),
statuses(Status.DELETED));

Assertions.assertThatThrownBy(
assertThatThrownBy(
() -> commit(table, table.newDelete().deleteFile(FILE_B).validateFilesExist(), branch))
.isInstanceOf(ValidationException.class);
}

@Test
@TestTemplate
public void testDeleteFilesNoValidation() {
commit(table, table.newFastAppend().appendFile(FILE_B), branch);
Snapshot delete1 = commit(table, table.newDelete().deleteFile(FILE_B), branch);
Expand All @@ -439,8 +426,8 @@ public void testDeleteFilesNoValidation() {
statuses(Status.DELETED));

Snapshot delete2 = commit(table, table.newDelete().deleteFile(FILE_B), branch);
Assertions.assertThat(delete2.allManifests(FILE_IO).isEmpty()).isTrue();
Assertions.assertThat(delete2.removedDataFiles(FILE_IO).iterator().hasNext()).isFalse();
assertThat(delete2.allManifests(FILE_IO)).isEmpty();
assertThat(delete2.removedDataFiles(FILE_IO)).isEmpty();
}

private static ByteBuffer longToBuffer(long value) {
Expand Down
Loading

0 comments on commit aa17c0a

Please sign in to comment.