diff --git a/core/src/test/java/org/apache/iceberg/LocalTableOperations.java b/core/src/test/java/org/apache/iceberg/LocalTableOperations.java index 27767801cd4e..699ef05c1e62 100644 --- a/core/src/test/java/org/apache/iceberg/LocalTableOperations.java +++ b/core/src/test/java/org/apache/iceberg/LocalTableOperations.java @@ -18,21 +18,23 @@ */ package org.apache.iceberg; +import java.io.File; import java.io.IOException; +import java.nio.file.Path; import java.util.Map; import org.apache.iceberg.exceptions.RuntimeIOException; import org.apache.iceberg.io.FileIO; import org.apache.iceberg.io.LocationProvider; import org.apache.iceberg.relocated.com.google.common.collect.Maps; -import org.junit.rules.TemporaryFolder; class LocalTableOperations implements TableOperations { - private final TemporaryFolder temp; + private final Path temp; + private final FileIO io; private final Map createdMetadataFilePaths = Maps.newHashMap(); - LocalTableOperations(TemporaryFolder temp) { + LocalTableOperations(Path temp) { this.temp = temp; this.io = new TestTables.LocalFileIO(); } @@ -63,7 +65,7 @@ public String metadataFileLocation(String fileName) { fileName, name -> { try { - return temp.newFile(name).getAbsolutePath(); + return File.createTempFile("junit", null, temp.toFile()).getAbsolutePath(); } catch (IOException e) { throw new RuntimeIOException(e); } diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshot.java b/core/src/test/java/org/apache/iceberg/TestSnapshot.java index 11b7c4a7cce4..2ec6abd4e428 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshot.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshot.java @@ -18,27 +18,25 @@ */ package org.apache.iceberg; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; + +import java.util.Arrays; +import java.util.List; import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; -import org.junit.Assert; -import org.junit.Assume; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -@RunWith(Parameterized.class) -public class TestSnapshot extends TableTestBase { - @Parameterized.Parameters(name = "formatVersion = {0}") - public static Object[] parameters() { - return new Object[] {1, 2}; - } - - public TestSnapshot(int formatVersion) { - super(formatVersion); +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(ParameterizedTestExtension.class) +public class TestSnapshot extends TestBase { + @Parameters(name = "formatVersion = {0}") + protected static List parameters() { + return Arrays.asList(1, 2); } - @Test + @TestTemplate public void testAppendFilesFromTable() { table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -58,7 +56,7 @@ public void testAppendFilesFromTable() { validateSnapshot(oldSnapshot, newSnapshot, FILE_A, FILE_B); } - @Test + @TestTemplate public void testAppendFoundFiles() { table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -81,7 +79,7 @@ public void testAppendFoundFiles() { validateSnapshot(oldSnapshot, newSnapshot, FILE_A, FILE_B); } - @Test + @TestTemplate public void testCachedDataFiles() { table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -98,27 +96,25 @@ public void testCachedDataFiles() { Snapshot thirdSnapshot = table.currentSnapshot(); Iterable removedDataFiles = thirdSnapshot.removedDataFiles(FILE_IO); - Assert.assertEquals("Must have 1 removed data file", 1, Iterables.size(removedDataFiles)); + assertThat(removedDataFiles).as("Must have 1 removed data file").hasSize(1); DataFile removedDataFile = Iterables.getOnlyElement(removedDataFiles); - Assert.assertEquals("Path must match", FILE_A.path(), removedDataFile.path()); - Assert.assertEquals("Spec ID must match", FILE_A.specId(), removedDataFile.specId()); - Assert.assertEquals("Partition must match", FILE_A.partition(), removedDataFile.partition()); + assertThat(removedDataFile.path()).isEqualTo(FILE_A.path()); + assertThat(removedDataFile.specId()).isEqualTo(FILE_A.specId()); + assertThat(removedDataFile.partition()).isEqualTo(FILE_A.partition()); Iterable addedDataFiles = thirdSnapshot.addedDataFiles(FILE_IO); - Assert.assertEquals("Must have 1 added data file", 1, Iterables.size(addedDataFiles)); + assertThat(addedDataFiles).as("Must have 1 added data file").hasSize(1); DataFile addedDataFile = Iterables.getOnlyElement(addedDataFiles); - Assert.assertEquals("Path must match", thirdSnapshotDataFile.path(), addedDataFile.path()); - Assert.assertEquals( - "Spec ID must match", thirdSnapshotDataFile.specId(), addedDataFile.specId()); - Assert.assertEquals( - "Partition must match", thirdSnapshotDataFile.partition(), addedDataFile.partition()); + assertThat(addedDataFile.path()).isEqualTo(thirdSnapshotDataFile.path()); + assertThat(addedDataFile.specId()).isEqualTo(thirdSnapshotDataFile.specId()); + assertThat(addedDataFile.partition()).isEqualTo(thirdSnapshotDataFile.partition()); } - @Test + @TestTemplate public void testCachedDeleteFiles() { - Assume.assumeTrue("Delete files only supported in V2", formatVersion >= 2); + assumeThat(formatVersion).as("Delete files only supported in V2").isGreaterThanOrEqualTo(2); table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -148,30 +144,23 @@ public void testCachedDeleteFiles() { Snapshot thirdSnapshot = table.currentSnapshot(); Iterable removedDeleteFiles = thirdSnapshot.removedDeleteFiles(FILE_IO); - Assert.assertEquals("Must have 1 removed delete file", 1, Iterables.size(removedDeleteFiles)); + assertThat(removedDeleteFiles).as("Must have 1 removed delete file").hasSize(1); DeleteFile removedDeleteFile = Iterables.getOnlyElement(removedDeleteFiles); - Assert.assertEquals( - "Path must match", secondSnapshotDeleteFile.path(), removedDeleteFile.path()); - Assert.assertEquals( - "Spec ID must match", secondSnapshotDeleteFile.specId(), removedDeleteFile.specId()); - Assert.assertEquals( - "Partition must match", - secondSnapshotDeleteFile.partition(), - removedDeleteFile.partition()); + assertThat(removedDeleteFile.path()).isEqualTo(secondSnapshotDeleteFile.path()); + assertThat(removedDeleteFile.specId()).isEqualTo(secondSnapshotDeleteFile.specId()); + assertThat(removedDeleteFile.partition()).isEqualTo(secondSnapshotDeleteFile.partition()); Iterable addedDeleteFiles = thirdSnapshot.addedDeleteFiles(FILE_IO); - Assert.assertEquals("Must have 1 added delete file", 1, Iterables.size(addedDeleteFiles)); + assertThat(addedDeleteFiles).as("Must have 1 added delete file").hasSize(1); DeleteFile addedDeleteFile = Iterables.getOnlyElement(addedDeleteFiles); - Assert.assertEquals("Path must match", thirdSnapshotDeleteFile.path(), addedDeleteFile.path()); - Assert.assertEquals( - "Spec ID must match", thirdSnapshotDeleteFile.specId(), addedDeleteFile.specId()); - Assert.assertEquals( - "Partition must match", thirdSnapshotDeleteFile.partition(), addedDeleteFile.partition()); + assertThat(addedDeleteFile.path()).isEqualTo(thirdSnapshotDeleteFile.path()); + assertThat(addedDeleteFile.specId()).isEqualTo(thirdSnapshotDeleteFile.specId()); + assertThat(addedDeleteFile.partition()).isEqualTo(thirdSnapshotDeleteFile.partition()); } - @Test + @TestTemplate public void testSequenceNumbersInAddedDataFiles() { long expectedSequenceNumber = 0L; if (formatVersion >= 2) { @@ -193,22 +182,21 @@ private void runAddedDataFileSequenceNumberTest(long expectedSequenceNumber) { Snapshot snapshot = table.currentSnapshot(); Iterable addedDataFiles = snapshot.addedDataFiles(table.io()); - Assert.assertEquals( - "Sequence number mismatch in Snapshot", expectedSequenceNumber, snapshot.sequenceNumber()); + assertThat(snapshot.sequenceNumber()) + .as("Sequence number mismatch in Snapshot") + .isEqualTo(expectedSequenceNumber); for (DataFile df : addedDataFiles) { - Assert.assertEquals( - "Data sequence number mismatch", - expectedSequenceNumber, - df.dataSequenceNumber().longValue()); - Assert.assertEquals( - "File sequence number mismatch", - expectedSequenceNumber, - df.fileSequenceNumber().longValue()); + assertThat(df.dataSequenceNumber().longValue()) + .as("Data sequence number mismatch") + .isEqualTo(expectedSequenceNumber); + assertThat(df.fileSequenceNumber().longValue()) + .as("File sequence number mismatch") + .isEqualTo(expectedSequenceNumber); } } - @Test + @TestTemplate public void testSequenceNumbersInRemovedDataFiles() { table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -239,28 +227,24 @@ private void runRemovedDataFileSequenceNumberTest( Snapshot snapshot = table.currentSnapshot(); Iterable removedDataFiles = snapshot.removedDataFiles(table.io()); - Assert.assertEquals("Must have 1 removed data file", 1, Iterables.size(removedDataFiles)); + assertThat(removedDataFiles).as("Must have 1 removed data file").hasSize(1); DataFile removedDataFile = Iterables.getOnlyElement(removedDataFiles); - Assert.assertEquals( - "Sequence number mismatch in Snapshot", - expectedSnapshotSequenceNumber, - snapshot.sequenceNumber()); - - Assert.assertEquals( - "Data sequence number mismatch", - expectedFileSequenceNumber, - removedDataFile.dataSequenceNumber().longValue()); - Assert.assertEquals( - "File sequence number mismatch", - expectedFileSequenceNumber, - removedDataFile.fileSequenceNumber().longValue()); + assertThat(snapshot.sequenceNumber()) + .as("Sequence number mismatch in Snapshot") + .isEqualTo(expectedSnapshotSequenceNumber); + assertThat(removedDataFile.dataSequenceNumber().longValue()) + .as("Data sequence number mismatch") + .isEqualTo(expectedFileSequenceNumber); + assertThat(removedDataFile.fileSequenceNumber().longValue()) + .as("File sequence number mismatch") + .isEqualTo(expectedFileSequenceNumber); } - @Test + @TestTemplate public void testSequenceNumbersInAddedDeleteFiles() { - Assume.assumeTrue("Delete files only supported in V2", formatVersion >= 2); + assumeThat(formatVersion).as("Delete files only supported in V2").isGreaterThanOrEqualTo(2); table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit(); @@ -277,20 +261,18 @@ private void runAddedDeleteFileSequenceNumberTest( Snapshot snapshot = table.currentSnapshot(); Iterable addedDeleteFiles = snapshot.addedDeleteFiles(table.io()); - Assert.assertEquals("Must have 1 added delete file", 1, Iterables.size(addedDeleteFiles)); + assertThat(addedDeleteFiles).as("Must have 1 added delete file").hasSize(1); DeleteFile addedDeleteFile = Iterables.getOnlyElement(addedDeleteFiles); - Assert.assertEquals( - "Sequence number mismatch in Snapshot", expectedSequenceNumber, snapshot.sequenceNumber()); - - Assert.assertEquals( - "Data sequence number mismatch", - expectedSequenceNumber, - addedDeleteFile.dataSequenceNumber().longValue()); - Assert.assertEquals( - "File sequence number mismatch", - expectedSequenceNumber, - addedDeleteFile.fileSequenceNumber().longValue()); + assertThat(snapshot.sequenceNumber()) + .as("Sequence number mismatch in Snapshot") + .isEqualTo(expectedSequenceNumber); + assertThat(addedDeleteFile.dataSequenceNumber().longValue()) + .as("Data sequence number mismatch") + .isEqualTo(expectedSequenceNumber); + assertThat(addedDeleteFile.fileSequenceNumber().longValue()) + .as("File sequence number mismatch") + .isEqualTo(expectedSequenceNumber); } } diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java index e384e571e162..ee1239074997 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java @@ -19,21 +19,19 @@ package org.apache.iceberg; import static org.apache.iceberg.Files.localInput; +import static org.assertj.core.api.Assertions.assertThat; import java.io.File; import java.io.IOException; +import java.nio.file.Path; import java.util.List; -import java.util.UUID; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.assertj.core.api.Assertions; -import org.junit.Assert; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; public class TestSnapshotJson { - @Rule public TemporaryFolder temp = new TemporaryFolder(); + @TempDir private Path temp; public TableOperations ops = new LocalTableOperations(temp); @@ -49,12 +47,11 @@ public void testJsonConversion() throws IOException { String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); - Assert.assertEquals("Snapshot ID should match", expected.snapshotId(), snapshot.snapshotId()); - Assert.assertEquals( - "Files should match", expected.allManifests(ops.io()), snapshot.allManifests(ops.io())); - Assert.assertNull("Operation should be null", snapshot.operation()); - Assert.assertNull("Summary should be null", snapshot.summary()); - Assert.assertEquals("Schema ID should match", Integer.valueOf(1), snapshot.schemaId()); + assertThat(snapshot.snapshotId()).isEqualTo(expected.snapshotId()); + assertThat(snapshot.allManifests(ops.io())).isEqualTo(expected.allManifests(ops.io())); + assertThat(snapshot.operation()).isNull(); + assertThat(snapshot.summary()).isNull(); + assertThat(snapshot.schemaId()).isEqualTo(1); } @Test @@ -69,12 +66,11 @@ public void testJsonConversionWithoutSchemaId() throws IOException { String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); - Assert.assertEquals("Snapshot ID should match", expected.snapshotId(), snapshot.snapshotId()); - Assert.assertEquals( - "Files should match", expected.allManifests(ops.io()), snapshot.allManifests(ops.io())); - Assert.assertNull("Operation should be null", snapshot.operation()); - Assert.assertNull("Summary should be null", snapshot.summary()); - Assert.assertNull("Schema ID should be null", snapshot.schemaId()); + assertThat(snapshot.snapshotId()).isEqualTo(expected.snapshotId()); + assertThat(snapshot.allManifests(ops.io())).isEqualTo(expected.allManifests(ops.io())); + assertThat(snapshot.operation()).isNull(); + assertThat(snapshot.summary()).isNull(); + assertThat(snapshot.schemaId()).isNull(); } @Test @@ -98,20 +94,17 @@ public void testJsonConversionWithOperation() throws IOException { String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); - Assert.assertEquals("Sequence number should default to 0 for v1", 0, snapshot.sequenceNumber()); - Assert.assertEquals("Snapshot ID should match", expected.snapshotId(), snapshot.snapshotId()); - Assert.assertEquals( - "Timestamp should match", expected.timestampMillis(), snapshot.timestampMillis()); - Assert.assertEquals("Parent ID should match", expected.parentId(), snapshot.parentId()); - Assert.assertEquals( - "Manifest list should match", - expected.manifestListLocation(), - snapshot.manifestListLocation()); - Assert.assertEquals( - "Files should match", expected.allManifests(ops.io()), snapshot.allManifests(ops.io())); - Assert.assertEquals("Operation should match", expected.operation(), snapshot.operation()); - Assert.assertEquals("Summary should match", expected.summary(), snapshot.summary()); - Assert.assertEquals("Schema ID should match", expected.schemaId(), snapshot.schemaId()); + assertThat(snapshot.sequenceNumber()) + .as("Sequence number should default to 0 for v1") + .isEqualTo(0); + assertThat(snapshot.snapshotId()).isEqualTo(expected.snapshotId()); + assertThat(snapshot.timestampMillis()).isEqualTo(expected.timestampMillis()); + assertThat(snapshot.parentId()).isEqualTo(expected.parentId()); + assertThat(snapshot.manifestListLocation()).isEqualTo(expected.manifestListLocation()); + assertThat(snapshot.allManifests(ops.io())).isEqualTo(expected.allManifests(ops.io())); + assertThat(snapshot.operation()).isEqualTo(expected.operation()); + assertThat(snapshot.summary()).isEqualTo(expected.summary()); + assertThat(snapshot.schemaId()).isEqualTo(expected.schemaId()); } @Test @@ -149,29 +142,26 @@ public void testJsonConversionWithV1Manifests() { timestampMillis); String json = SnapshotParser.toJson(expected, true); - Assertions.assertThat(json).isEqualTo(expectedJson); + assertThat(json).isEqualTo(expectedJson); Snapshot snapshot = SnapshotParser.fromJson(json); - Assertions.assertThat(snapshot).isEqualTo(expected); - - Assert.assertEquals("Sequence number should default to 0 for v1", 0, snapshot.sequenceNumber()); - Assert.assertEquals("Snapshot ID should match", expected.snapshotId(), snapshot.snapshotId()); - Assert.assertEquals( - "Timestamp should match", expected.timestampMillis(), snapshot.timestampMillis()); - Assert.assertEquals("Parent ID should match", expected.parentId(), snapshot.parentId()); - Assert.assertEquals( - "Manifest list should match", - expected.manifestListLocation(), - snapshot.manifestListLocation()); - Assert.assertEquals( - "Files should match", expected.allManifests(ops.io()), snapshot.allManifests(ops.io())); - Assert.assertEquals("Operation should match", expected.operation(), snapshot.operation()); - Assert.assertEquals("Summary should match", expected.summary(), snapshot.summary()); - Assert.assertEquals("Schema ID should match", expected.schemaId(), snapshot.schemaId()); + assertThat(snapshot).isEqualTo(expected); + + assertThat(snapshot.sequenceNumber()) + .as("Sequence number should default to 0 for v1") + .isEqualTo(0); + assertThat(snapshot.snapshotId()).isEqualTo(expected.snapshotId()); + assertThat(snapshot.timestampMillis()).isEqualTo(expected.timestampMillis()); + assertThat(snapshot.parentId()).isEqualTo(expected.parentId()); + assertThat(snapshot.manifestListLocation()).isEqualTo(expected.manifestListLocation()); + assertThat(snapshot.allManifests(ops.io())).isEqualTo(expected.allManifests(ops.io())); + assertThat(snapshot.operation()).isEqualTo(expected.operation()); + assertThat(snapshot.summary()).isEqualTo(expected.summary()); + assertThat(snapshot.schemaId()).isEqualTo(expected.schemaId()); } private String createManifestListWithManifestFiles(long snapshotId, Long parentSnapshotId) throws IOException { - File manifestList = temp.newFile("manifests" + UUID.randomUUID()); + File manifestList = File.createTempFile("manifests", null, temp.toFile()); manifestList.deleteOnExit(); List manifests = diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotLoading.java b/core/src/test/java/org/apache/iceberg/TestSnapshotLoading.java index 25a48ce0622b..89312201265d 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotLoading.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotLoading.java @@ -20,9 +20,11 @@ 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 static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.util.Arrays; import java.util.List; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -31,22 +33,17 @@ 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.SerializableSupplier; -import org.assertj.core.api.Assumptions; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestTemplate; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mockito; -@RunWith(Parameterized.class) -public class TestSnapshotLoading extends TableTestBase { - @Parameterized.Parameters(name = "formatVersion = {0}") - public static Object[] parameters() { - return new Object[] {1, 2}; - } +@ExtendWith(ParameterizedTestExtension.class) +public class TestSnapshotLoading extends TestBase { - public TestSnapshotLoading(int formatVersion) { - super(formatVersion); + @Parameters(name = "formatVersion = {0}") + protected static List parameters() { + return Arrays.asList(1, 2); } private Snapshot currentSnapshot; @@ -56,7 +53,7 @@ public TestSnapshotLoading(int formatVersion) { private SerializableSupplier> snapshotsSupplierMock; - @Before + @BeforeEach public void before() { table.newFastAppend().appendFile(FILE_A).commit(); table.newFastAppend().appendFile(FILE_B).commit(); @@ -87,7 +84,7 @@ public List get() { .build(); } - @Test + @TestTemplate public void testSnapshotsAreLoadedOnce() { latestTableMetadata.snapshots(); latestTableMetadata.snapshots(); @@ -99,7 +96,7 @@ public void testSnapshotsAreLoadedOnce() { .containsExactlyElementsOf(originalTableMetadata.snapshots()); } - @Test + @TestTemplate public void testCurrentAndMainSnapshotDoesNotLoad() { latestTableMetadata.currentSnapshot(); latestTableMetadata.snapshot(latestTableMetadata.ref(SnapshotRef.MAIN_BRANCH).snapshotId()); @@ -107,7 +104,7 @@ public void testCurrentAndMainSnapshotDoesNotLoad() { verify(snapshotsSupplierMock, times(0)).get(); } - @Test + @TestTemplate public void testUnloadedSnapshotLoadsOnce() { Snapshot unloadedSnapshot = allSnapshots.stream().filter(s -> !s.equals(currentSnapshot)).findFirst().get(); @@ -118,7 +115,7 @@ public void testUnloadedSnapshotLoadsOnce() { verify(snapshotsSupplierMock, times(1)).get(); } - @Test + @TestTemplate public void testCurrentTableScanDoesNotLoad() { latestTableMetadata.currentSnapshot(); @@ -130,9 +127,9 @@ public void testCurrentTableScanDoesNotLoad() { verify(snapshotsSupplierMock, times(0)).get(); } - @Test + @TestTemplate public void testFutureSnapshotsAreRemoved() { - Assumptions.assumeThat(formatVersion) + assumeThat(formatVersion) .as("Future snapshots are only removed for V2 tables") .isGreaterThan(1); @@ -152,7 +149,7 @@ public void testFutureSnapshotsAreRemoved() { .containsExactlyInAnyOrderElementsOf(originalTableMetadata.snapshots()); } - @Test + @TestTemplate public void testRemovedCurrentSnapshotFails() { List snapshotsMissingCurrent = allSnapshots.stream() @@ -174,7 +171,7 @@ public void testRemovedCurrentSnapshotFails() { .hasMessage("Invalid table metadata: Cannot find current version"); } - @Test + @TestTemplate public void testRemovedRefSnapshotFails() { Snapshot referencedSnapshot = allSnapshots.stream().filter(Predicate.isEqual(currentSnapshot).negate()).findFirst().get(); @@ -194,7 +191,7 @@ public void testRemovedRefSnapshotFails() { .hasMessageEndingWith("does not exist in the existing snapshots list"); } - @Test + @TestTemplate public void testBuildingNewMetadataTriggersSnapshotLoad() { TableMetadata newTableMetadata = TableMetadata.buildFrom(latestTableMetadata).removeRef(SnapshotRef.MAIN_BRANCH).build(); diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java index fd22ae24d0e4..88233dd99097 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java @@ -18,16 +18,18 @@ */ package org.apache.iceberg; +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.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; -import org.assertj.core.api.Assertions; -import org.junit.Assert; -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 TestSnapshotManager extends TableTestBase { +@ExtendWith(ParameterizedTestExtension.class) +public class TestSnapshotManager extends TestBase { // replacement for FILE_A static final DataFile REPLACEMENT_FILE_A = @@ -47,16 +49,12 @@ public class TestSnapshotManager extends TableTestBase { .withRecordCount(1) .build(); - @Parameterized.Parameters(name = "formatVersion = {0}") - public static Object[] parameters() { - return new Object[] {1, 2}; - } - - public TestSnapshotManager(int formatVersion) { - super(formatVersion); + @Parameters(name = "formatVersion = {0}") + protected static List parameters() { + return Arrays.asList(1, 2); } - @Test + @TestTemplate public void testCherryPickDynamicOverwrite() { table.newAppend().appendFile(FILE_A).commit(); @@ -64,8 +62,9 @@ public void testCherryPickDynamicOverwrite() { table.newReplacePartitions().addFile(REPLACEMENT_FILE_A).stageOnly().commit(); Snapshot staged = Iterables.getLast(table.snapshots()); - Assert.assertEquals( - "Should find the staged overwrite snapshot", DataOperations.OVERWRITE, staged.operation()); + assertThat(staged.operation()) + .as("Should find the staged overwrite snapshot") + .isEqualTo(DataOperations.OVERWRITE); // add another append so that the original commit can't be fast-forwarded table.newAppend().appendFile(FILE_B).commit(); @@ -73,21 +72,23 @@ public void testCherryPickDynamicOverwrite() { // pick the snapshot into the current state table.manageSnapshots().cherrypick(staged.snapshotId()).commit(); - Assert.assertNotEquals( - "Should not fast-forward", staged.snapshotId(), table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Should not fast-forward") + .isNotEqualTo(staged.snapshotId()); validateTableFiles(table, FILE_B, REPLACEMENT_FILE_A); } - @Test + @TestTemplate public void testCherryPickDynamicOverwriteWithoutParent() { - Assert.assertNull("Table should not have a current snapshot", table.currentSnapshot()); + assertThat(table.currentSnapshot()).isNull(); // stage an overwrite that replaces FILE_A table.newReplacePartitions().addFile(REPLACEMENT_FILE_A).stageOnly().commit(); Snapshot staged = Iterables.getLast(table.snapshots()); - Assert.assertEquals( - "Should find the staged overwrite snapshot", DataOperations.OVERWRITE, staged.operation()); + assertThat(staged.operation()) + .as("Should find the staged overwrite snapshot") + .isEqualTo(DataOperations.OVERWRITE); // add another append so that the original commit can't be fast-forwarded table.newAppend().appendFile(FILE_B).commit(); @@ -95,12 +96,13 @@ public void testCherryPickDynamicOverwriteWithoutParent() { // pick the snapshot into the current state table.manageSnapshots().cherrypick(staged.snapshotId()).commit(); - Assert.assertNotEquals( - "Should not fast-forward", staged.snapshotId(), table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Should not fast-forward") + .isNotEqualTo(staged.snapshotId()); validateTableFiles(table, FILE_B, REPLACEMENT_FILE_A); } - @Test + @TestTemplate public void testCherryPickDynamicOverwriteConflict() { table.newAppend().appendFile(FILE_A).commit(); @@ -108,27 +110,26 @@ public void testCherryPickDynamicOverwriteConflict() { table.newReplacePartitions().addFile(REPLACEMENT_FILE_A).stageOnly().commit(); Snapshot staged = Iterables.getLast(table.snapshots()); - Assert.assertEquals( - "Should find the staged overwrite snapshot", DataOperations.OVERWRITE, staged.operation()); + assertThat(staged.operation()) + .as("Should find the staged overwrite snapshot") + .isEqualTo(DataOperations.OVERWRITE); // add another append so that the original commit can't be fast-forwarded table.newAppend().appendFile(CONFLICT_FILE_A).commit(); long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) + assertThatThrownBy(() -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) .isInstanceOf(ValidationException.class) .hasMessageStartingWith("Cannot cherry-pick replace partitions with changed partition"); - Assert.assertEquals( - "Failed cherry-pick should not change the table state", - lastSnapshotId, - table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Failed cherry-pick should not change the table state") + .isEqualTo(lastSnapshotId); validateTableFiles(table, FILE_A, CONFLICT_FILE_A); } - @Test + @TestTemplate public void testCherryPickDynamicOverwriteDeleteConflict() { table.newAppend().appendFile(FILE_A).commit(); @@ -136,8 +137,9 @@ public void testCherryPickDynamicOverwriteDeleteConflict() { table.newReplacePartitions().addFile(REPLACEMENT_FILE_A).stageOnly().commit(); Snapshot staged = Iterables.getLast(table.snapshots()); - Assert.assertEquals( - "Should find the staged overwrite snapshot", DataOperations.OVERWRITE, staged.operation()); + assertThat(staged.operation()) + .as("Should find the staged overwrite snapshot") + .isEqualTo(DataOperations.OVERWRITE); // add FILE_B s table.newAppend().appendFile(FILE_B).commit(); @@ -147,19 +149,17 @@ public void testCherryPickDynamicOverwriteDeleteConflict() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) + assertThatThrownBy(() -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) .isInstanceOf(ValidationException.class) .hasMessageStartingWith("Missing required files to delete"); - Assert.assertEquals( - "Failed cherry-pick should not change the table state", - lastSnapshotId, - table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Failed cherry-pick should not change the table state") + .isEqualTo(lastSnapshotId); validateTableFiles(table, FILE_B); } - @Test + @TestTemplate public void testCherryPickFromBranch() { table.newAppend().appendFile(FILE_A).commit(); long branchSnapshotId = table.currentSnapshot().snapshotId(); @@ -177,20 +177,18 @@ public void testCherryPickFromBranch() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().cherrypick(replaceSnapshotId).commit()) + assertThatThrownBy(() -> table.manageSnapshots().cherrypick(replaceSnapshotId).commit()) .isInstanceOf(ValidationException.class) .hasMessageStartingWith( "Cannot cherry-pick overwrite not based on an ancestor of the current state"); - Assert.assertEquals( - "Failed cherry-pick should not change the table state", - lastSnapshotId, - table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Failed cherry-pick should not change the table state") + .isEqualTo(lastSnapshotId); validateTableFiles(table, FILE_A); } - @Test + @TestTemplate public void testCherryPickOverwrite() { table.newAppend().appendFile(FILE_A).commit(); @@ -198,99 +196,94 @@ public void testCherryPickOverwrite() { table.newOverwrite().deleteFile(FILE_A).addFile(REPLACEMENT_FILE_A).stageOnly().commit(); Snapshot staged = Iterables.getLast(table.snapshots()); - Assert.assertEquals( - "Should find the staged overwrite snapshot", DataOperations.OVERWRITE, staged.operation()); + assertThat(staged.operation()) + .as("Should find the staged overwrite snapshot") + .isEqualTo(DataOperations.OVERWRITE); // add another append so that the original commit can't be fast-forwarded table.newAppend().appendFile(FILE_B).commit(); long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) + assertThatThrownBy(() -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()) .isInstanceOf(ValidationException.class) .hasMessageEndingWith("not append, dynamic overwrite, or fast-forward"); - Assert.assertEquals( - "Failed cherry-pick should not change the table state", - lastSnapshotId, - table.currentSnapshot().snapshotId()); + assertThat(table.currentSnapshot().snapshotId()) + .as("Failed cherry-pick should not change the table state") + .isEqualTo(lastSnapshotId); validateTableFiles(table, FILE_A, FILE_B); } - @Test + @TestTemplate public void testCreateBranch() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); // Test a basic case of creating a branch table.manageSnapshots().createBranch("branch1", snapshotId).commit(); SnapshotRef expectedBranch = table.ops().refresh().ref("branch1"); - Assert.assertTrue( - expectedBranch != null - && expectedBranch.equals(SnapshotRef.branchBuilder(snapshotId).build())); + assertThat(expectedBranch).isNotNull().isEqualTo(SnapshotRef.branchBuilder(snapshotId).build()); } - @Test + @TestTemplate public void testCreateBranchWithoutSnapshotId() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); // Test a basic case of creating a branch table.manageSnapshots().createBranch("branch1").commit(); SnapshotRef actualBranch = table.ops().refresh().ref("branch1"); - Assertions.assertThat(actualBranch).isNotNull(); - Assertions.assertThat(actualBranch).isEqualTo(SnapshotRef.branchBuilder(snapshotId).build()); + assertThat(actualBranch).isNotNull().isEqualTo(SnapshotRef.branchBuilder(snapshotId).build()); } - @Test + @TestTemplate public void testCreateBranchOnEmptyTable() { table.manageSnapshots().createBranch("branch1").commit(); SnapshotRef mainSnapshotRef = table.ops().refresh().ref(SnapshotRef.MAIN_BRANCH); - Assertions.assertThat(mainSnapshotRef).isNull(); + assertThat(mainSnapshotRef).isNull(); SnapshotRef branch1SnapshotRef = table.ops().refresh().ref("branch1"); - Assertions.assertThat(branch1SnapshotRef).isNotNull(); - Assertions.assertThat(branch1SnapshotRef.minSnapshotsToKeep()).isNull(); - Assertions.assertThat(branch1SnapshotRef.maxSnapshotAgeMs()).isNull(); - Assertions.assertThat(branch1SnapshotRef.maxRefAgeMs()).isNull(); + assertThat(branch1SnapshotRef).isNotNull(); + assertThat(branch1SnapshotRef.minSnapshotsToKeep()).isNull(); + assertThat(branch1SnapshotRef.maxSnapshotAgeMs()).isNull(); + assertThat(branch1SnapshotRef.maxRefAgeMs()).isNull(); Snapshot snapshot = table.snapshot(branch1SnapshotRef.snapshotId()); - Assertions.assertThat(snapshot.parentId()).isNull(); - Assertions.assertThat(snapshot.addedDataFiles(table.io())).isEmpty(); - Assertions.assertThat(snapshot.removedDataFiles(table.io())).isEmpty(); - Assertions.assertThat(snapshot.addedDeleteFiles(table.io())).isEmpty(); - Assertions.assertThat(snapshot.removedDeleteFiles(table.io())).isEmpty(); + assertThat(snapshot.parentId()).isNull(); + assertThat(snapshot.addedDataFiles(table.io())).isEmpty(); + assertThat(snapshot.removedDataFiles(table.io())).isEmpty(); + assertThat(snapshot.addedDeleteFiles(table.io())).isEmpty(); + assertThat(snapshot.removedDeleteFiles(table.io())).isEmpty(); } - @Test + @TestTemplate public void testCreateBranchOnEmptyTableFailsWhenRefAlreadyExists() { table.manageSnapshots().createBranch("branch1").commit(); // Trying to create a branch with an existing name should fail - Assertions.assertThatThrownBy(() -> table.manageSnapshots().createBranch("branch1").commit()) + assertThatThrownBy(() -> table.manageSnapshots().createBranch("branch1").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref branch1 already exists"); // Trying to create another branch within the same chain - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table.manageSnapshots().createBranch("branch2").createBranch("branch2").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref branch2 already exists"); } - @Test + @TestTemplate public void testCreateBranchFailsWhenRefAlreadyExists() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); // Trying to create a branch with an existing name should fail - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().createBranch("branch1", snapshotId).commit()) + assertThatThrownBy(() -> table.manageSnapshots().createBranch("branch1", snapshotId).commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref branch1 already exists"); // Trying to create another branch within the same chain - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -301,7 +294,7 @@ public void testCreateBranchFailsWhenRefAlreadyExists() { .hasMessage("Ref branch2 already exists"); } - @Test + @TestTemplate public void testCreateTag() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -309,24 +302,22 @@ public void testCreateTag() { table.manageSnapshots().createTag("tag1", snapshotId).commit(); SnapshotRef expectedTag = table.ops().refresh().ref("tag1"); - Assert.assertTrue( - expectedTag != null && expectedTag.equals(SnapshotRef.tagBuilder(snapshotId).build())); + assertThat(expectedTag).isNotNull().isEqualTo(SnapshotRef.tagBuilder(snapshotId).build()); } - @Test + @TestTemplate public void testCreateTagFailsWhenRefAlreadyExists() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createTag("tag1", snapshotId).commit(); // Trying to create a tag with an existing name should fail - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().createTag("tag1", snapshotId).commit()) + assertThatThrownBy(() -> table.manageSnapshots().createTag("tag1", snapshotId).commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref tag1 already exists"); // Trying to create another tag within the same chain - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -337,7 +328,7 @@ public void testCreateTagFailsWhenRefAlreadyExists() { .hasMessage("Ref tag2 already exists"); } - @Test + @TestTemplate public void testRemoveBranch() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -347,31 +338,29 @@ public void testRemoveBranch() { TableMetadata updated = table.ops().refresh(); SnapshotRef expectedBranch = updated.ref("branch1"); - Assert.assertNull(expectedBranch); + assertThat(expectedBranch).isNull(); // Test chained creating and removal of branch and tag table.manageSnapshots().createBranch("branch2", snapshotId).removeBranch("branch2").commit(); updated = table.ops().refresh(); - Assert.assertNull(updated.ref("branch2")); + assertThat(updated.ref("branch2")).isNull(); } - @Test + @TestTemplate public void testRemovingNonExistingBranchFails() { - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().removeBranch("non-existing").commit()) + assertThatThrownBy(() -> table.manageSnapshots().removeBranch("non-existing").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Branch does not exist: non-existing"); } - @Test + @TestTemplate public void testRemovingMainBranchFails() { - Assertions.assertThatThrownBy( - () -> table.manageSnapshots().removeBranch(SnapshotRef.MAIN_BRANCH).commit()) + assertThatThrownBy(() -> table.manageSnapshots().removeBranch(SnapshotRef.MAIN_BRANCH).commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot remove main branch"); } - @Test + @TestTemplate public void testRemoveTag() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -380,22 +369,22 @@ public void testRemoveTag() { table.manageSnapshots().removeTag("tag1").commit(); TableMetadata updated = table.ops().refresh(); SnapshotRef expectedTag = updated.ref("tag1"); - Assert.assertNull(expectedTag); + assertThat(expectedTag).isNull(); // Test chained creating and removal of a tag table.manageSnapshots().createTag("tag2", snapshotId).removeTag("tag2").commit(); - Assert.assertEquals(updated, table.ops().refresh()); - Assert.assertNull(updated.ref("tag2")); + assertThat(table.ops().refresh()).isEqualTo(updated); + assertThat(updated.ref("tag2")).isNull(); } - @Test + @TestTemplate public void testRemovingNonExistingTagFails() { - Assertions.assertThatThrownBy(() -> table.manageSnapshots().removeTag("non-existing").commit()) + assertThatThrownBy(() -> table.manageSnapshots().removeTag("non-existing").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tag does not exist: non-existing"); } - @Test + @TestTemplate public void testReplaceBranch() { table.newAppend().appendFile(FILE_A).set("wap.id", "123").stageOnly().commit(); Snapshot firstSnapshot = Iterables.getOnlyElement(table.snapshots()); @@ -404,63 +393,61 @@ public void testReplaceBranch() { Snapshot secondSnapshot = Iterables.get(table.snapshots(), 1); table.manageSnapshots().createBranch("branch2", secondSnapshot.snapshotId()).commit(); table.manageSnapshots().replaceBranch("branch1", "branch2").commit(); - Assert.assertEquals( - table.ops().refresh().ref("branch1").snapshotId(), secondSnapshot.snapshotId()); + assertThat(secondSnapshot.snapshotId()) + .isEqualTo(table.ops().refresh().ref("branch1").snapshotId()); } - @Test + @TestTemplate public void testReplaceBranchNonExistingToBranchFails() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table.manageSnapshots().replaceBranch("branch1", "non-existing").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref does not exist: non-existing"); } - @Test + @TestTemplate public void testFastForwardBranchNonExistingFromBranchCreatesTheBranch() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); table.manageSnapshots().fastForwardBranch("new-branch", "branch1").commit(); - Assertions.assertThat(table.ops().current().ref("new-branch").isBranch()).isTrue(); - Assertions.assertThat(table.ops().current().ref("new-branch").snapshotId()) - .isEqualTo(snapshotId); + assertThat(table.ops().current().ref("new-branch").isBranch()).isTrue(); + assertThat(table.ops().current().ref("new-branch").snapshotId()).isEqualTo(snapshotId); } - @Test + @TestTemplate public void testReplaceBranchNonExistingFromBranchCreatesTheBranch() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); table.manageSnapshots().replaceBranch("new-branch", "branch1").commit(); - Assertions.assertThat(table.ops().current().ref("new-branch").isBranch()).isTrue(); - Assertions.assertThat(table.ops().current().ref("new-branch").snapshotId()) - .isEqualTo(snapshotId); + assertThat(table.ops().current().ref("new-branch").isBranch()).isTrue(); + assertThat(table.ops().current().ref("new-branch").snapshotId()).isEqualTo(snapshotId); } - @Test + @TestTemplate public void testFastForwardBranchNonExistingToFails() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table.manageSnapshots().fastForwardBranch("branch1", "non-existing").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Ref does not exist: non-existing"); } - @Test + @TestTemplate public void testFastForward() { table.newAppend().appendFile(FILE_A).commit(); table.newAppend().appendFile(FILE_B).set("wap.id", "123456789").stageOnly().commit(); - Assert.assertEquals(table.currentSnapshot().snapshotId(), 1); + assertThat(table.currentSnapshot().snapshotId()).isEqualTo(1); table.manageSnapshots().createBranch("new-branch-at-staged-snapshot", 2).commit(); table @@ -468,10 +455,10 @@ public void testFastForward() { .fastForwardBranch(SnapshotRef.MAIN_BRANCH, "new-branch-at-staged-snapshot") .commit(); - Assert.assertEquals(table.currentSnapshot().snapshotId(), 2); + assertThat(table.currentSnapshot().snapshotId()).isEqualTo(2); } - @Test + @TestTemplate public void testFastForwardWhenFromIsNotAncestorFails() { table.newAppend().appendFile(FILE_A).commit(); @@ -485,7 +472,7 @@ public void testFastForwardWhenFromIsNotAncestorFails() { final String newBranch = "new-branch-at-staged-snapshot"; table.manageSnapshots().createBranch(newBranch, snapshot).commit(); - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -496,7 +483,7 @@ public void testFastForwardWhenFromIsNotAncestorFails() { "Cannot fast-forward: main is not an ancestor of new-branch-at-staged-snapshot"); } - @Test + @TestTemplate public void testReplaceTag() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -505,10 +492,10 @@ public void testReplaceTag() { table.newAppend().appendFile(FILE_B).commit(); long currentSnapshot = table.ops().refresh().currentSnapshot().snapshotId(); table.manageSnapshots().replaceTag("tag1", currentSnapshot).commit(); - Assert.assertEquals(table.ops().refresh().ref("tag1").snapshotId(), currentSnapshot); + assertThat(currentSnapshot).isEqualTo(table.ops().refresh().ref("tag1").snapshotId()); } - @Test + @TestTemplate public void testUpdatingBranchRetention() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -520,8 +507,8 @@ public void testUpdatingBranchRetention() { .setMaxSnapshotAgeMs("branch1", 20000) .commit(); TableMetadata updated = table.ops().refresh(); - Assert.assertEquals(20000, (long) updated.ref("branch1").maxSnapshotAgeMs()); - Assert.assertEquals(10, (long) updated.ref("branch1").minSnapshotsToKeep()); + assertThat(updated.ref("branch1").maxSnapshotAgeMs()).isEqualTo(20000); + assertThat(updated.ref("branch1").minSnapshotsToKeep()).isEqualTo(10); // Test creating and updating in a chain table .manageSnapshots() @@ -530,16 +517,16 @@ public void testUpdatingBranchRetention() { .setMaxSnapshotAgeMs("branch2", 20000) .commit(); updated = table.ops().refresh(); - Assert.assertEquals(20000, (long) updated.ref("branch2").maxSnapshotAgeMs()); - Assert.assertEquals(10, (long) updated.ref("branch2").minSnapshotsToKeep()); + assertThat(updated.ref("branch2").maxSnapshotAgeMs()).isEqualTo(20000); + assertThat(updated.ref("branch2").minSnapshotsToKeep()).isEqualTo(10); } - @Test + @TestTemplate public void testSettingBranchRetentionOnTagFails() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -549,7 +536,7 @@ public void testSettingBranchRetentionOnTagFails() { .isInstanceOf(IllegalArgumentException.class) .hasMessage("Tags do not support setting minSnapshotsToKeep"); - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -560,7 +547,7 @@ public void testSettingBranchRetentionOnTagFails() { .hasMessage("Tags do not support setting maxSnapshotAgeMs"); } - @Test + @TestTemplate public void testUpdatingBranchMaxRefAge() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -570,11 +557,10 @@ public void testUpdatingBranchMaxRefAge() { table.manageSnapshots().createBranch("branch1", snapshotId).commit(); table.manageSnapshots().setMaxRefAgeMs("branch1", 10000).commit(); TableMetadata updated = table.ops().refresh(); - Assert.assertEquals(maxRefAgeMs, (long) updated.ref("branch1").maxRefAgeMs()); - Assert.assertEquals(maxRefAgeMs, (long) updated.ref("branch1").maxRefAgeMs()); + assertThat(updated.ref("branch1").maxRefAgeMs()).isEqualTo(maxRefAgeMs); } - @Test + @TestTemplate public void testUpdatingTagMaxRefAge() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); @@ -585,7 +571,7 @@ public void testUpdatingTagMaxRefAge() { table.manageSnapshots().setMaxRefAgeMs("tag1", maxRefAgeMs).commit(); TableMetadata updated = table.ops().refresh(); - Assert.assertEquals(maxRefAgeMs, (long) updated.ref("tag1").maxRefAgeMs()); + assertThat(updated.ref("tag1").maxRefAgeMs()).isEqualTo(maxRefAgeMs); // Test creating and updating in a chain table @@ -594,10 +580,10 @@ public void testUpdatingTagMaxRefAge() { .setMaxRefAgeMs("tag2", maxRefAgeMs) .commit(); updated = table.ops().refresh(); - Assert.assertEquals(maxRefAgeMs, (long) updated.ref("tag2").maxRefAgeMs()); + assertThat(updated.ref("tag2").maxRefAgeMs()).isEqualTo(maxRefAgeMs); } - @Test + @TestTemplate public void testRenameBranch() { table.newAppend().appendFile(FILE_A).commit(); table.newAppend().appendFile(FILE_A).commit(); @@ -607,8 +593,8 @@ public void testRenameBranch() { table.manageSnapshots().createBranch("branch1", snapshotId).commit(); table.manageSnapshots().renameBranch("branch1", "branch2").commit(); TableMetadata updated = table.ops().refresh(); - Assert.assertNull(updated.ref("branch1")); - Assert.assertEquals(updated.ref("branch2"), SnapshotRef.branchBuilder(snapshotId).build()); + assertThat(updated.ref("branch1")).isNull(); + assertThat(SnapshotRef.branchBuilder(snapshotId).build()).isEqualTo(updated.ref("branch2")); table .manageSnapshots() @@ -617,13 +603,13 @@ public void testRenameBranch() { .commit(); updated = table.ops().refresh(); - Assert.assertNull(updated.ref("branch3")); - Assert.assertEquals(updated.ref("branch4"), SnapshotRef.branchBuilder(snapshotId).build()); + assertThat(updated.ref("branch3")).isNull(); + assertThat(SnapshotRef.branchBuilder(snapshotId).build()).isEqualTo(updated.ref("branch4")); } - @Test + @TestTemplate public void testFailRenamingMainBranch() { - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table .manageSnapshots() @@ -633,16 +619,16 @@ public void testFailRenamingMainBranch() { .hasMessage("Cannot rename main branch"); } - @Test + @TestTemplate public void testRenamingNonExistingBranchFails() { - Assertions.assertThatThrownBy( + assertThatThrownBy( () -> table.manageSnapshots().renameBranch("some-missing-branch", "some-branch").commit()) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Branch does not exist: some-missing-branch"); } - @Test + @TestTemplate public void testCreateReferencesAndRollback() { table.newAppend().appendFile(FILE_A).commit(); table.newAppend().appendFile(FILE_A).commit(); @@ -656,15 +642,15 @@ public void testCreateReferencesAndRollback() { .commit(); TableMetadata current = table.ops().current(); - Assert.assertEquals(current.currentSnapshot().snapshotId(), 1); + assertThat(current.currentSnapshot().snapshotId()).isEqualTo(1); SnapshotRef actualTag = current.ref("tag1"); SnapshotRef actualBranch = current.ref("branch1"); - Assert.assertEquals(1, current.currentSnapshot().snapshotId()); - Assert.assertEquals(SnapshotRef.branchBuilder(snapshotPriorToRollback).build(), actualBranch); - Assert.assertEquals(SnapshotRef.tagBuilder(snapshotPriorToRollback).build(), actualTag); + assertThat(current.currentSnapshot().snapshotId()).isEqualTo(1); + assertThat(actualBranch).isEqualTo(SnapshotRef.branchBuilder(snapshotPriorToRollback).build()); + assertThat(actualTag).isEqualTo(SnapshotRef.tagBuilder(snapshotPriorToRollback).build()); } - @Test + @TestTemplate public void testCreateReferencesAndCherrypick() { table.newAppend().appendFile(FILE_A).commit(); @@ -681,15 +667,15 @@ public void testCreateReferencesAndCherrypick() { .commit(); TableMetadata current = table.ops().current(); - Assert.assertEquals(current.currentSnapshot().snapshotId(), 2); + assertThat(current.currentSnapshot().snapshotId()).isEqualTo(2); SnapshotRef actualTag = current.ref("tag1"); SnapshotRef actualBranch = current.ref("branch1"); - Assert.assertEquals(2, current.currentSnapshot().snapshotId()); - Assert.assertEquals(SnapshotRef.branchBuilder(1).build(), actualBranch); - Assert.assertEquals(SnapshotRef.tagBuilder(1).build(), actualTag); + assertThat(current.currentSnapshot().snapshotId()).isEqualTo(2); + assertThat(actualBranch).isEqualTo(SnapshotRef.branchBuilder(1).build()); + assertThat(actualTag).isEqualTo(SnapshotRef.tagBuilder(1).build()); } - @Test + @TestTemplate public void testAttemptToRollbackToCurrentSnapshot() { table.newAppend().appendFile(FILE_A).commit(); @@ -700,7 +686,7 @@ public void testAttemptToRollbackToCurrentSnapshot() { table.manageSnapshots().rollbackTo(currentSnapshotId).commit(); } - @Test + @TestTemplate public void testSnapshotManagerThroughTransaction() { table.newAppend().appendFile(FILE_A).commit(); Snapshot snapshotAfterFirstAppend = readMetadata().currentSnapshot(); @@ -708,40 +694,43 @@ public void testSnapshotManagerThroughTransaction() { table.newAppend().appendFile(FILE_B).commit(); validateSnapshot(snapshotAfterFirstAppend, readMetadata().currentSnapshot(), FILE_B); - Assert.assertEquals("Table should be on version 2 after appending twice", 2, (int) version()); + assertThat(version()).as("Table should be on version 2 after appending twice").isEqualTo(2); TableMetadata base = readMetadata(); Transaction txn = table.newTransaction(); - Assert.assertSame( - "Base metadata should not change when transaction is created", base, readMetadata()); - Assert.assertEquals( - "Table should be on version 2 after creating transaction", 2, (int) version()); + assertThat(readMetadata()) + .as("Base metadata should not change when transaction is created") + .isSameAs(base); + assertThat(version()) + .as("Table should be on version 2 after creating transaction") + .isEqualTo(2); ManageSnapshots manageSnapshots = txn.manageSnapshots(); - Assert.assertNotNull(manageSnapshots); + assertThat(manageSnapshots).isNotNull(); - Assert.assertSame( - "Base metadata should not change when manageSnapshots is created", base, readMetadata()); - Assert.assertEquals( - "Table should be on version 2 after creating manageSnapshots", 2, (int) version()); + assertThat(readMetadata()) + .as("Base metadata should not change when manageSnapshots is created") + .isSameAs(base); + assertThat(version()) + .as("Table should be on version 2 after creating manageSnapshots") + .isEqualTo(2); manageSnapshots.rollbackTo(snapshotAfterFirstAppend.snapshotId()).commit(); - Assert.assertSame( - "Base metadata should not change when invoking rollbackTo", base, readMetadata()); - Assert.assertEquals( - "Table should be on version 2 after invoking rollbackTo", 2, (int) version()); + assertThat(readMetadata()) + .as("Base metadata should not change when invoking rollbackTo") + .isSameAs(base); + assertThat(version()).as("Table should be on version 2 after invoking rollbackTo").isEqualTo(2); txn.commitTransaction(); - Assert.assertEquals(snapshotAfterFirstAppend, readMetadata().currentSnapshot()); + assertThat(readMetadata().currentSnapshot()).isEqualTo(snapshotAfterFirstAppend); validateSnapshot(null, snapshotAfterFirstAppend, FILE_A); - Assert.assertEquals( - "Table should be on version 3 after invoking rollbackTo", 3, (int) version()); + assertThat(version()).as("Table should be on version 3 after invoking rollbackTo").isEqualTo(3); } - @Test + @TestTemplate public void testSnapshotManagerThroughTransactionMultiOperation() { table.newAppend().appendFile(FILE_A).commit(); Snapshot snapshotAfterFirstAppend = readMetadata().currentSnapshot(); @@ -749,32 +738,30 @@ public void testSnapshotManagerThroughTransactionMultiOperation() { table.newAppend().appendFile(FILE_B).commit(); validateSnapshot(snapshotAfterFirstAppend, readMetadata().currentSnapshot(), FILE_B); - Assert.assertEquals("Table should be on version 2 after appending twice", 2, (int) version()); + assertThat(version()).as("Table should be on version 2 after appending twice").isEqualTo(2); TableMetadata base = readMetadata(); Transaction txn = table.newTransaction(); txn.manageSnapshots().rollbackTo(snapshotAfterFirstAppend.snapshotId()).commit(); txn.updateProperties().set("some_prop", "some_prop_value").commit(); - Assert.assertSame( - "Base metadata should not change when transaction is not committed", base, readMetadata()); - Assert.assertEquals( - "Table should remain on version 2 when transaction is not committed", 2, (int) version()); + assertThat(readMetadata()) + .as("Base metadata should not change when transaction is not committed") + .isSameAs(base); + assertThat(version()) + .as("Table should remain on version 2 when transaction is not committed") + .isEqualTo(2); txn.commitTransaction(); - Assert.assertEquals(snapshotAfterFirstAppend, readMetadata().currentSnapshot()); - Assert.assertEquals( - "Table should be on version 3 after invoking rollbackTo", 3, (int) version()); + assertThat(readMetadata().currentSnapshot()).isEqualTo(snapshotAfterFirstAppend); + assertThat(version()).as("Table should be on version 3 after invoking rollbackTo").isEqualTo(3); } - @Test - public void testSnapshotManagerInvalidParameters() throws Exception { - Assert.assertThrows( - "Incorrect input transaction: null", - IllegalArgumentException.class, - () -> { - new SnapshotManager(null); - }); + @TestTemplate + public void testSnapshotManagerInvalidParameters() { + assertThatThrownBy(() -> new SnapshotManager(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid input transaction: null"); } } diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java b/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java index d8940367e1a5..52e937a7745e 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java @@ -18,9 +18,10 @@ */ package org.apache.iceberg; -import org.assertj.core.api.Assertions; -import org.junit.Assert; -import org.junit.Test; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; public class TestSnapshotRefParser { @@ -28,24 +29,27 @@ public class TestSnapshotRefParser { public void testTagToJsonDefault() { String json = "{\"snapshot-id\":1,\"type\":\"tag\"}"; SnapshotRef ref = SnapshotRef.tagBuilder(1L).build(); - Assert.assertEquals( - "Should be able to serialize default tag", json, SnapshotRefParser.toJson(ref)); + assertThat(SnapshotRefParser.toJson(ref)) + .as("Should be able to serialize default tag") + .isEqualTo(json); } @Test public void testTagToJsonAllFields() { String json = "{\"snapshot-id\":1,\"type\":\"tag\",\"max-ref-age-ms\":1}"; SnapshotRef ref = SnapshotRef.tagBuilder(1L).maxRefAgeMs(1L).build(); - Assert.assertEquals( - "Should be able to serialize tag with all fields", json, SnapshotRefParser.toJson(ref)); + assertThat(SnapshotRefParser.toJson(ref)) + .as("Should be able to serialize tag with all fields") + .isEqualTo(json); } @Test public void testBranchToJsonDefault() { String json = "{\"snapshot-id\":1,\"type\":\"branch\"}"; SnapshotRef ref = SnapshotRef.branchBuilder(1L).build(); - Assert.assertEquals( - "Should be able to serialize default branch", json, SnapshotRefParser.toJson(ref)); + assertThat(SnapshotRefParser.toJson(ref)) + .as("Should be able to serialize default branch") + .isEqualTo(json); } @Test @@ -59,32 +63,36 @@ public void testBranchToJsonAllFields() { .maxSnapshotAgeMs(3L) .maxRefAgeMs(4L) .build(); - Assert.assertEquals( - "Should be able to serialize branch with all fields", json, SnapshotRefParser.toJson(ref)); + assertThat(SnapshotRefParser.toJson(ref)) + .as("Should be able to serialize branch with all fields") + .isEqualTo(json); } @Test public void testTagFromJsonDefault() { String json = "{\"snapshot-id\":1,\"type\":\"tag\"}"; SnapshotRef ref = SnapshotRef.tagBuilder(1L).build(); - Assert.assertEquals( - "Should be able to deserialize default tag", ref, SnapshotRefParser.fromJson(json)); + assertThat(SnapshotRefParser.fromJson(json)) + .as("Should be able to deserialize default tag") + .isEqualTo(ref); } @Test public void testTagFromJsonAllFields() { String json = "{\"snapshot-id\":1,\"type\":\"tag\",\"max-ref-age-ms\":1}"; SnapshotRef ref = SnapshotRef.tagBuilder(1L).maxRefAgeMs(1L).build(); - Assert.assertEquals( - "Should be able to deserialize tag with all fields", ref, SnapshotRefParser.fromJson(json)); + assertThat(SnapshotRefParser.fromJson(json)) + .as("Should be able to deserialize tag with all fields") + .isEqualTo(ref); } @Test public void testBranchFromJsonDefault() { String json = "{\"snapshot-id\":1,\"type\":\"branch\"}"; SnapshotRef ref = SnapshotRef.branchBuilder(1L).build(); - Assert.assertEquals( - "Should be able to deserialize default branch", ref, SnapshotRefParser.fromJson(json)); + assertThat(SnapshotRefParser.fromJson(json)) + .as("Should be able to deserialize default branch") + .isEqualTo(ref); } @Test @@ -98,21 +106,20 @@ public void testBranchFromJsonAllFields() { .maxSnapshotAgeMs(3L) .maxRefAgeMs(4L) .build(); - Assert.assertEquals( - "Should be able to deserialize branch with all fields", - ref, - SnapshotRefParser.fromJson(json)); + assertThat(SnapshotRefParser.fromJson(json)) + .as("Should be able to deserialize branch with all fields") + .isEqualTo(ref); } @Test public void testFailParsingWhenNullOrEmptyJson() { String nullJson = null; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(nullJson)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(nullJson)) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Cannot parse snapshot ref from invalid JSON"); String emptyJson = ""; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(emptyJson)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(emptyJson)) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Cannot parse snapshot ref from invalid JSON"); } @@ -120,12 +127,12 @@ public void testFailParsingWhenNullOrEmptyJson() { @Test public void testFailParsingWhenMissingRequiredFields() { String refMissingType = "{\"snapshot-id\":1}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(refMissingType)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(refMissingType)) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Cannot parse missing string"); String refMissingSnapshotId = "{\"type\":\"branch\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(refMissingSnapshotId)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(refMissingSnapshotId)) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Cannot parse missing long"); } @@ -134,31 +141,31 @@ public void testFailParsingWhenMissingRequiredFields() { public void testFailWhenFieldsHaveInvalidValues() { String invalidSnapshotId = "{\"snapshot-id\":\"invalid-snapshot-id\",\"type\":\"not-a-valid-tag-type\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidSnapshotId)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidSnapshotId)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse to a long value: snapshot-id: \"invalid-snapshot-id\""); String invalidTagType = "{\"snapshot-id\":1,\"type\":\"not-a-valid-tag-type\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidTagType)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidTagType)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Invalid snapshot ref type: not-a-valid-tag-type"); String invalidRefAge = "{\"snapshot-id\":1,\"type\":\"tag\",\"max-ref-age-ms\":\"not-a-valid-value\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidRefAge)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidRefAge)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse to a long value: max-ref-age-ms: \"not-a-valid-value\""); String invalidSnapshotsToKeep = "{\"snapshot-id\":1,\"type\":\"branch\", " + "\"min-snapshots-to-keep\":\"invalid-number\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidSnapshotsToKeep)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidSnapshotsToKeep)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse to an integer value: min-snapshots-to-keep: \"invalid-number\""); String invalidMaxSnapshotAge = "{\"snapshot-id\":1,\"type\":\"branch\", " + "\"max-snapshot-age-ms\":\"invalid-age\"}"; - Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidMaxSnapshotAge)) + assertThatThrownBy(() -> SnapshotRefParser.fromJson(invalidMaxSnapshotAge)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot parse to a long value: max-snapshot-age-ms: \"invalid-age\""); } diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotSelection.java b/core/src/test/java/org/apache/iceberg/TestSnapshotSelection.java index 1a9f4646e81a..7ce59e9df1c9 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotSelection.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotSelection.java @@ -18,29 +18,27 @@ */ package org.apache.iceberg; +import static org.assertj.core.api.Assertions.assertThat; + import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.Arrays; +import java.util.List; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -@RunWith(Parameterized.class) -public class TestSnapshotSelection 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; - public TestSnapshotSelection(int formatVersion) { - super(formatVersion); +@ExtendWith(ParameterizedTestExtension.class) +public class TestSnapshotSelection extends TestBase { + @Parameters(name = "formatVersion = {0}") + protected static List parameters() { + return Arrays.asList(1, 2); } - @Test + @TestTemplate public void testSnapshotSelectionById() { - Assert.assertEquals("Table should start empty", 0, listManifestFiles().size()); + assertThat(listManifestFiles()).hasSize(0); table.newFastAppend().appendFile(FILE_A).commit(); Snapshot firstSnapshot = table.currentSnapshot(); @@ -48,12 +46,12 @@ public void testSnapshotSelectionById() { table.newFastAppend().appendFile(FILE_B).commit(); Snapshot secondSnapshot = table.currentSnapshot(); - Assert.assertEquals("Table should have two snapshots", 2, Iterables.size(table.snapshots())); + assertThat(table.snapshots()).hasSize(2); validateSnapshot(null, table.snapshot(firstSnapshot.snapshotId()), FILE_A); validateSnapshot(firstSnapshot, table.snapshot(secondSnapshot.snapshotId()), FILE_B); } - @Test + @TestTemplate public void testSnapshotStatsForAddedFiles() { DataFile fileWithStats = DataFiles.builder(SPEC) @@ -76,12 +74,12 @@ public void testSnapshotStatsForAddedFiles() { Snapshot snapshot = table.currentSnapshot(); Iterable addedFiles = snapshot.addedDataFiles(table.io()); - Assert.assertEquals(1, Iterables.size(addedFiles)); + assertThat(addedFiles).hasSize(1); DataFile dataFile = Iterables.getOnlyElement(addedFiles); - Assert.assertNotNull("Value counts should be not null", dataFile.valueCounts()); - Assert.assertNotNull("Null value counts should be not null", dataFile.nullValueCounts()); - Assert.assertNotNull("Lower bounds should be not null", dataFile.lowerBounds()); - Assert.assertNotNull("Upper bounds should be not null", dataFile.upperBounds()); + assertThat(dataFile.valueCounts()).isNotNull(); + assertThat(dataFile.nullValueCounts()).isNotNull(); + assertThat(dataFile.lowerBounds()).isNotNull(); + assertThat(dataFile.upperBounds()).isNotNull(); } private ByteBuffer longToBuffer(long value) { diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotSummary.java b/core/src/test/java/org/apache/iceberg/TestSnapshotSummary.java index e5653b4c65d7..75b98bd4dca1 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotSummary.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotSummary.java @@ -18,40 +18,41 @@ */ package org.apache.iceberg; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; +import java.util.List; import java.util.Map; -import org.junit.Assert; -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 TestSnapshotSummary extends TableTestBase { - public TestSnapshotSummary(int formatVersion) { - super(formatVersion); - } +@ExtendWith(ParameterizedTestExtension.class) +public class TestSnapshotSummary extends TestBase { - @Parameterized.Parameters(name = "formatVersion = {0}") - public static Object[] parameters() { - return new Object[] {1, 2}; + @Parameters(name = "formatVersion = {0}") + protected static List parameters() { + return Arrays.asList(1, 2); } - @Test + @TestTemplate public void testFileSizeSummary() { - Assert.assertEquals("Table should start empty", 0, listManifestFiles().size()); + assertThat(listManifestFiles()).hasSize(0); // fast append table.newFastAppend().appendFile(FILE_A).commit(); Map summary = table.currentSnapshot().summary(); - Assert.assertEquals("10", summary.get(SnapshotSummary.ADDED_FILE_SIZE_PROP)); - Assert.assertNull(summary.get(SnapshotSummary.REMOVED_FILE_SIZE_PROP)); - Assert.assertEquals("10", summary.get(SnapshotSummary.TOTAL_FILE_SIZE_PROP)); + assertThat(summary) + .containsEntry(SnapshotSummary.ADDED_FILE_SIZE_PROP, "10") + .containsEntry(SnapshotSummary.TOTAL_FILE_SIZE_PROP, "10") + .doesNotContainKey(SnapshotSummary.REMOVED_FILE_SIZE_PROP); // merge append table.newAppend().appendFile(FILE_B).commit(); summary = table.currentSnapshot().summary(); - Assert.assertEquals("10", summary.get(SnapshotSummary.ADDED_FILE_SIZE_PROP)); - Assert.assertNull(summary.get(SnapshotSummary.REMOVED_FILE_SIZE_PROP)); - Assert.assertEquals("20", summary.get(SnapshotSummary.TOTAL_FILE_SIZE_PROP)); + assertThat(summary) + .containsEntry(SnapshotSummary.ADDED_FILE_SIZE_PROP, "10") + .containsEntry(SnapshotSummary.TOTAL_FILE_SIZE_PROP, "20") + .doesNotContainKey(SnapshotSummary.REMOVED_FILE_SIZE_PROP); table .newOverwrite() @@ -62,18 +63,20 @@ public void testFileSizeSummary() { .addFile(FILE_D) .commit(); summary = table.currentSnapshot().summary(); - Assert.assertEquals("30", summary.get(SnapshotSummary.ADDED_FILE_SIZE_PROP)); - Assert.assertEquals("20", summary.get(SnapshotSummary.REMOVED_FILE_SIZE_PROP)); - Assert.assertEquals("30", summary.get(SnapshotSummary.TOTAL_FILE_SIZE_PROP)); + assertThat(summary) + .containsEntry(SnapshotSummary.ADDED_FILE_SIZE_PROP, "30") + .containsEntry(SnapshotSummary.REMOVED_FILE_SIZE_PROP, "20") + .containsEntry(SnapshotSummary.TOTAL_FILE_SIZE_PROP, "30"); table.newDelete().deleteFile(FILE_C).deleteFile(FILE_D).commit(); summary = table.currentSnapshot().summary(); - Assert.assertNull(summary.get(SnapshotSummary.ADDED_FILE_SIZE_PROP)); - Assert.assertEquals("20", summary.get(SnapshotSummary.REMOVED_FILE_SIZE_PROP)); - Assert.assertEquals("10", summary.get(SnapshotSummary.TOTAL_FILE_SIZE_PROP)); + assertThat(summary) + .containsEntry(SnapshotSummary.REMOVED_FILE_SIZE_PROP, "20") + .containsEntry(SnapshotSummary.TOTAL_FILE_SIZE_PROP, "10") + .doesNotContainKey(SnapshotSummary.ADDED_FILE_SIZE_PROP); } - @Test + @TestTemplate public void testFileSizeSummaryWithDeletes() { if (formatVersion == 1) { return; @@ -83,7 +86,8 @@ public void testFileSizeSummaryWithDeletes() { table.refresh(); Map summary = table.currentSnapshot().summary(); - Assert.assertEquals("1", summary.get(SnapshotSummary.ADD_EQ_DELETE_FILES_PROP)); - Assert.assertEquals("1", summary.get(SnapshotSummary.ADD_POS_DELETE_FILES_PROP)); + assertThat(summary) + .containsEntry(SnapshotSummary.ADD_EQ_DELETE_FILES_PROP, "1") + .containsEntry(SnapshotSummary.ADD_POS_DELETE_FILES_PROP, "1"); } } diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 826f3ad1e778..1d7d754d8df2 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -61,9 +61,8 @@ import org.apache.iceberg.util.JsonUtil; import org.assertj.core.api.Assertions; import org.junit.Assert; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; public class TestTableMetadata { private static final String TEST_LOCATION = "s3://bucket/test/location"; @@ -87,7 +86,7 @@ public class TestTableMetadata { .desc(Expressions.bucket("z", 4), NullOrder.NULLS_LAST) .build(); - @Rule public TemporaryFolder temp = new TemporaryFolder(); + @TempDir private Path temp; public TableOperations ops = new LocalTableOperations(temp); @@ -1731,7 +1730,7 @@ public void testNoTrailingLocationSlash() { private String createManifestListWithManifestFile( long snapshotId, Long parentSnapshotId, String manifestFile) throws IOException { - File manifestList = temp.newFile("manifests" + UUID.randomUUID()); + File manifestList = File.createTempFile("manifests", null, temp.toFile()); manifestList.deleteOnExit(); try (ManifestListWriter writer =