This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 1557016393 Core: Remove deprecated AssertHelpers usage (#7994)
1557016393 is described below
commit 155701639313bf644c776197a6327f7ae0656fc9
Author: Liu Xiao <[email protected]>
AuthorDate: Thu Jul 6 14:32:40 2023 +0800
Core: Remove deprecated AssertHelpers usage (#7994)
---
.../java/org/apache/iceberg/TestMergeAppend.java | 44 +--
.../iceberg/TestOverwriteWithValidation.java | 162 ++++-----
.../org/apache/iceberg/TestRemoveSnapshots.java | 53 ++-
.../org/apache/iceberg/TestReplacePartitions.java | 281 ++++++++-------
.../org/apache/iceberg/TestReplaceTransaction.java | 41 +--
.../java/org/apache/iceberg/TestRewriteFiles.java | 268 +++++++-------
.../org/apache/iceberg/TestRewriteManifests.java | 83 ++---
.../test/java/org/apache/iceberg/TestRowDelta.java | 289 +++++++--------
.../org/apache/iceberg/TestSnapshotRefParser.java | 73 ++--
.../java/org/apache/iceberg/TestTableMetadata.java | 390 ++++++++++-----------
10 files changed, 782 insertions(+), 902 deletions(-)
diff --git a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
index 46dd9ecbc5..892d92634c 100644
--- a/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
+++ b/core/src/test/java/org/apache/iceberg/TestMergeAppend.java
@@ -33,6 +33,7 @@ import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -945,11 +946,9 @@ public class TestMergeAppend extends TableTestBase {
ids(pending.snapshotId(), baseId),
concat(files(FILE_B), files(initialManifest)));
- AssertHelpers.assertThrows(
- "Should retry 4 times and throw last failure",
- CommitFailedException.class,
- "Injected failure",
- () -> commit(table, append, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, append, branch))
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
V2Assert.assertEquals(
"Last sequence number should be 1", 1,
readMetadata().lastSequenceNumber());
@@ -983,11 +982,9 @@ public class TestMergeAppend extends TableTestBase {
ManifestFile newManifest = pending.allManifests(table.io()).get(0);
Assert.assertTrue("Should create new manifest", new
File(newManifest.path()).exists());
- AssertHelpers.assertThrows(
- "Should retry 4 times and throw last failure",
- CommitFailedException.class,
- "Injected failure",
- () -> commit(table, append, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, append, branch))
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
V2Assert.assertEquals(
"Last sequence number should be 0", 0,
readMetadata().lastSequenceNumber());
V1Assert.assertEquals(
@@ -1186,11 +1183,9 @@ public class TestMergeAppend extends TableTestBase {
AppendFiles append = table.newAppend();
append.appendManifest(manifest);
- AssertHelpers.assertThrows(
- "Should reject commit",
- CommitFailedException.class,
- "Injected failure",
- () -> commit(table, append, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, append, branch))
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertEquals("Last sequence number should be 0", 0,
readMetadata().lastSequenceNumber());
Assert.assertTrue("Append manifest should not be deleted", new
File(manifest.path()).exists());
@@ -1205,20 +1200,19 @@ public class TestMergeAppend extends TableTestBase {
ManifestFile manifestWithExistingFiles =
writeManifest("manifest-file-1.avro", manifestEntry(Status.EXISTING,
null, FILE_A));
- AssertHelpers.assertThrows(
- "Should reject commit",
- IllegalArgumentException.class,
- "Cannot append manifest with existing files",
- () -> commit(table,
table.newAppend().appendManifest(manifestWithExistingFiles), branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(table,
table.newAppend().appendManifest(manifestWithExistingFiles), branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot append manifest with existing files");
Assert.assertEquals("Last sequence number should be 0", 0,
readMetadata().lastSequenceNumber());
ManifestFile manifestWithDeletedFiles =
writeManifest("manifest-file-2.avro", manifestEntry(Status.DELETED,
null, FILE_A));
- AssertHelpers.assertThrows(
- "Should reject commit",
- IllegalArgumentException.class,
- "Cannot append manifest with deleted files",
- () -> commit(table,
table.newAppend().appendManifest(manifestWithDeletedFiles), branch));
+ Assertions.assertThatThrownBy(
+ () -> commit(table,
table.newAppend().appendManifest(manifestWithDeletedFiles), branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot append manifest with deleted files");
Assert.assertEquals("Last sequence number should be 0", 0,
readMetadata().lastSequenceNumber());
}
diff --git
a/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java
b/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java
index b7d194377e..a4ccb4018c 100644
--- a/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java
+++ b/core/src/test/java/org/apache/iceberg/TestOverwriteWithValidation.java
@@ -36,6 +36,7 @@ import org.apache.iceberg.expressions.Expression;
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.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;
@@ -339,11 +340,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newAppend().appendFile(FILE_DAY_1), branch);
long committedSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found conflicting files",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting files");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -415,11 +414,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newAppend().appendFile(FILE_DAY_2), branch);
long committedSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found conflicting files",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting files");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -446,11 +443,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newDelete().deleteFile(FILE_DAY_2), branch);
long committedSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Missing required files to delete:",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Missing required files to delete:");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -558,11 +553,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
table.expireSnapshots().expireSnapshotId(2L).commit();
long committedSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Cannot determine history",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot determine history");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -588,11 +581,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
table.expireSnapshots().expireSnapshotId(1L).commit();
long committedSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Cannot determine history",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot determine history");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -681,11 +672,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, overwrite, branch);
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found conflicting files",
- txn::commitTransaction);
+ Assertions.assertThatThrownBy(txn::commitTransaction)
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting files");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -715,11 +704,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newRowDelta().addDeletes(FILE_DAY_2_POS_DELETES),
branch);
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "found new delete",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, found new delete for replaced
data file");
}
@Test
@@ -744,11 +731,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newRowDelta().addDeletes(FILE_DAY_2_POS_DELETES),
branch);
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found new conflicting delete",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found new conflicting delete");
}
@Test
@@ -770,11 +755,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newOverwrite().deleteFile(FILE_DAY_2), branch);
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found conflicting deleted files",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting deleted files");
}
@Test
@@ -879,11 +862,9 @@ public class TestOverwriteWithValidation extends
TableTestBase {
commit(table, table.newRowDelta().addDeletes(FILE_DAY_2_EQ_DELETES),
branch);
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "found new delete",
- () -> commit(table, overwrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, overwrite, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, found new delete for replaced
data file");
}
@Test
@@ -953,50 +934,47 @@ public class TestOverwriteWithValidation extends
TableTestBase {
Expression rowFilter = equal("dAtE", "2018-06-09");
- AssertHelpers.assertThrows(
- "Should use case sensitive binding by default",
- ValidationException.class,
- "Cannot find field 'dAtE'",
- () ->
- commit(
- table,
- table
- .newOverwrite()
- .addFile(FILE_DAY_2_MODIFIED)
- .conflictDetectionFilter(rowFilter)
- .validateNoConflictingData(),
- branch));
-
- AssertHelpers.assertThrows(
- "Should fail with case sensitive binding",
- ValidationException.class,
- "Cannot find field 'dAtE'",
- () ->
- commit(
- table,
- table
- .newOverwrite()
- .caseSensitive(true)
- .addFile(FILE_DAY_2_MODIFIED)
- .conflictDetectionFilter(rowFilter)
- .validateNoConflictingData(),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newOverwrite()
+ .addFile(FILE_DAY_2_MODIFIED)
+ .conflictDetectionFilter(rowFilter)
+ .validateNoConflictingData(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot find field 'dAtE'");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newOverwrite()
+ .caseSensitive(true)
+ .addFile(FILE_DAY_2_MODIFIED)
+ .conflictDetectionFilter(rowFilter)
+ .validateNoConflictingData(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot find field 'dAtE'");
// binding should succeed and trigger the validation
- AssertHelpers.assertThrows(
- "Should trigger the validation",
- ValidationException.class,
- "Found conflicting files",
- () ->
- commit(
- table,
- table
- .newOverwrite()
- .caseSensitive(false)
- .addFile(FILE_DAY_2_MODIFIED)
- .conflictDetectionFilter(rowFilter)
- .validateNoConflictingData(),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newOverwrite()
+ .caseSensitive(false)
+ .addFile(FILE_DAY_2_MODIFIED)
+ .conflictDetectionFilter(rowFilter)
+ .validateNoConflictingData(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting files");
}
@Test
diff --git a/core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
b/core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
index bad0635293..71455c5712 100644
--- a/core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
+++ b/core/src/test/java/org/apache/iceberg/TestRemoveSnapshots.java
@@ -671,11 +671,9 @@ public class TestRemoveSnapshots extends TableTestBase {
@Test
public void testRetainZeroSnapshots() {
- AssertHelpers.assertThrows(
- "Should fail retain 0 snapshots " + "because number of snapshots to
retain cannot be zero",
- IllegalArgumentException.class,
- "Number of snapshots to retain must be at least 1, cannot be: 0",
- () -> removeSnapshots(table).retainLast(0).commit());
+ Assertions.assertThatThrownBy(() ->
removeSnapshots(table).retainLast(0).commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Number of snapshots to retain must be at least 1, cannot
be: 0");
}
@Test
@@ -1039,11 +1037,9 @@ public class TestRemoveSnapshots extends TableTestBase {
table.newAppend().appendFile(FILE_A).commit();
- AssertHelpers.assertThrows(
- "Should complain about expiring snapshots",
- ValidationException.class,
- "Cannot expire snapshots: GC is disabled",
- () -> table.expireSnapshots());
+ Assertions.assertThatThrownBy(() -> table.expireSnapshots())
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot expire snapshots: GC is disabled");
}
@Test
@@ -1231,16 +1227,15 @@ public class TestRemoveSnapshots extends TableTestBase {
waitUntilAfter(table.currentSnapshot().timestampMillis());
RemoveSnapshots removeSnapshots = (RemoveSnapshots)
table.expireSnapshots();
- AssertHelpers.assertThrows(
- "Should fail removing snapshots and files when there is more than 1
ref",
- UnsupportedOperationException.class,
- "Cannot incrementally clean files for tables with more than 1 ref",
- () ->
- removeSnapshots
- .withIncrementalCleanup(true)
- .expireOlderThan(table.currentSnapshot().timestampMillis())
- .cleanExpiredFiles(true)
- .commit());
+ Assertions.assertThatThrownBy(
+ () ->
+ removeSnapshots
+ .withIncrementalCleanup(true)
+ .expireOlderThan(table.currentSnapshot().timestampMillis())
+ .cleanExpiredFiles(true)
+ .commit())
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage("Cannot incrementally clean files for tables with more
than 1 ref");
}
@Test
@@ -1330,11 +1325,10 @@ public class TestRemoveSnapshots extends TableTestBase {
table.manageSnapshots().createBranch("branch", snapshotId).commit();
- AssertHelpers.assertThrows(
- "Should fail removing snapshot when it is still referenced",
- IllegalArgumentException.class,
- "Cannot expire 2. Still referenced by refs: [branch]",
- () -> removeSnapshots(table).expireSnapshotId(snapshotId).commit());
+ Assertions.assertThatThrownBy(
+ () -> removeSnapshots(table).expireSnapshotId(snapshotId).commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot expire 2. Still referenced by refs: [branch]");
}
@Test
@@ -1348,11 +1342,10 @@ public class TestRemoveSnapshots extends TableTestBase {
// commit another snapshot so the first one isn't referenced by main
table.newAppend().appendFile(FILE_B).commit();
- AssertHelpers.assertThrows(
- "Should fail removing snapshot when it is still referenced",
- IllegalArgumentException.class,
- "Cannot expire 1. Still referenced by refs: [tag]",
- () -> removeSnapshots(table).expireSnapshotId(snapshotId).commit());
+ Assertions.assertThatThrownBy(
+ () -> removeSnapshots(table).expireSnapshotId(snapshotId).commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot expire 1. Still referenced by refs: [tag]");
}
@Test
diff --git a/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
b/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
index 0fe760d96e..e657e7fc43 100644
--- a/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
+++ b/core/src/test/java/org/apache/iceberg/TestReplacePartitions.java
@@ -24,6 +24,7 @@ import java.io.File;
import java.io.IOException;
import org.apache.iceberg.ManifestEntry.Status;
import org.apache.iceberg.exceptions.ValidationException;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
@@ -244,11 +245,9 @@ public class TestReplacePartitions extends TableTestBase {
ReplacePartitions replace =
table.newReplacePartitions().addFile(FILE_F).addFile(FILE_G).validateAppendOnly();
- AssertHelpers.assertThrows(
- "Should reject commit with file not matching delete expression",
- ValidationException.class,
- "Cannot commit file that conflicts with existing partition",
- () -> commit(table, replace, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, replace, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit file that conflicts with
existing partition");
Assert.assertEquals(
"Should not create a new snapshot",
@@ -332,20 +331,20 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Replace Partitions should fail with ValidationException
ReplacePartitions replace = table.newReplacePartitions();
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting files that can contain records matching partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]",
- () ->
- commit(
- table,
- replace
- .addFile(FILE_A)
- .addFile(FILE_B)
- .validateNoConflictingData()
- .validateNoConflictingDeletes(),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ replace
+ .addFile(FILE_A)
+ .addFile(FILE_B)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting files that can contain records matching
partitions "
+ + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]");
}
@Test
@@ -358,22 +357,22 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Replace Partitions should fail with ValidationException
commit(table, table.newReplacePartitions().addFile(FILE_A), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting files that can contain records matching partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]",
- () ->
- commit(
- table,
- table
- .newReplacePartitions()
- .validateFromSnapshot(baseId)
- .addFile(FILE_A)
- .addFile(FILE_B)
- .validateNoConflictingData()
- .validateNoConflictingDeletes(),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .addFile(FILE_A)
+ .addFile(FILE_B)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting files that can contain records matching
partitions "
+ + "[data_bucket=0, data_bucket=1]: [/path/to/data-a.parquet]");
}
@Test
@@ -427,21 +426,21 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent ReplacePartitions should fail with ValidationException
commit(table,
unpartitioned.newReplacePartitions().addFile(FILE_UNPARTITIONED_A), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting files that can contain records matching true: "
- + "[/path/to/data-unpartitioned-a.parquet]",
- () ->
- commit(
- table,
- unpartitioned
- .newReplacePartitions()
- .validateFromSnapshot(replaceBaseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_UNPARTITIONED_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ unpartitioned
+ .newReplacePartitions()
+ .validateFromSnapshot(replaceBaseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_UNPARTITIONED_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting files that can contain records matching true: "
+ + "[/path/to/data-unpartitioned-a.parquet]");
}
@Test
@@ -454,22 +453,22 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Append and ReplacePartition should fail with
ValidationException
commit(table, table.newFastAppend().appendFile(FILE_B), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting files that can contain records matching partitions "
- + "[data_bucket=0, data_bucket=1]: [/path/to/data-b.parquet]",
- () ->
- commit(
- table,
- table
- .newReplacePartitions()
- .validateFromSnapshot(baseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_A)
- .addFile(FILE_B),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_A)
+ .addFile(FILE_B),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting files that can contain records matching
partitions "
+ + "[data_bucket=0, data_bucket=1]: [/path/to/data-b.parquet]");
}
@Test
@@ -529,21 +528,21 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Append and ReplacePartitions should fail with
ValidationException
commit(table, unpartitioned.newAppend().appendFile(FILE_UNPARTITIONED_A),
branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting files that can contain records matching true: "
- + "[/path/to/data-unpartitioned-a.parquet]",
- () ->
- commit(
- table,
- unpartitioned
- .newReplacePartitions()
- .validateFromSnapshot(replaceBaseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_UNPARTITIONED_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ unpartitioned
+ .newReplacePartitions()
+ .validateFromSnapshot(replaceBaseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_UNPARTITIONED_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting files that can contain records matching true: "
+ + "[/path/to/data-unpartitioned-a.parquet]");
}
@Test
@@ -558,21 +557,21 @@ public class TestReplacePartitions extends TableTestBase {
commit(
table,
table.newRowDelta().addDeletes(FILE_A_DELETES).validateFromSnapshot(baseId),
branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found new conflicting delete files that can apply to records matching
"
- + "[data_bucket=0]: [/path/to/data-a-deletes.parquet]",
- () ->
- commit(
- table,
- table
- .newReplacePartitions()
- .validateFromSnapshot(baseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found new conflicting delete files that can apply to records
matching "
+ + "[data_bucket=0]: [/path/to/data-a-deletes.parquet]");
}
@Test
@@ -590,21 +589,21 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Delete and ReplacePartitions should fail with
ValidationException
commit(table,
unpartitioned.newRowDelta().addDeletes(FILE_UNPARTITIONED_A_DELETES), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found new conflicting delete files that can apply to records matching
true: "
- + "[/path/to/data-unpartitioned-a-deletes.parquet]",
- () ->
- commit(
- table,
- unpartitioned
- .newReplacePartitions()
- .validateFromSnapshot(replaceBaseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_UNPARTITIONED_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ unpartitioned
+ .newReplacePartitions()
+ .validateFromSnapshot(replaceBaseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_UNPARTITIONED_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found new conflicting delete files that can apply to records
matching true: "
+ + "[/path/to/data-unpartitioned-a-deletes.parquet]");
}
@Test
@@ -673,21 +672,21 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Overwrite and ReplacePartition should fail with
ValidationException
commit(table, table.newOverwrite().deleteFile(FILE_A), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting deleted files that can apply to records matching "
- + "[data_bucket=0]: [/path/to/data-a.parquet]",
- () ->
- commit(
- table,
- table
- .newReplacePartitions()
- .validateFromSnapshot(baseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newReplacePartitions()
+ .validateFromSnapshot(baseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting deleted files that can apply to records
matching "
+ + "[data_bucket=0]: [/path/to/data-a.parquet]");
}
@Test
@@ -746,21 +745,21 @@ public class TestReplacePartitions extends TableTestBase {
// Concurrent Overwrite and ReplacePartitions should fail with
ValidationException
commit(table,
unpartitioned.newOverwrite().deleteFile(FILE_UNPARTITIONED_A), branch);
- AssertHelpers.assertThrows(
- "Should reject commit with file matching partitions replaced",
- ValidationException.class,
- "Found conflicting deleted files that can contain records matching
true: "
- + "[/path/to/data-unpartitioned-a.parquet]",
- () ->
- commit(
- table,
- unpartitioned
- .newReplacePartitions()
- .validateFromSnapshot(replaceBaseId)
- .validateNoConflictingData()
- .validateNoConflictingDeletes()
- .addFile(FILE_UNPARTITIONED_A),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ unpartitioned
+ .newReplacePartitions()
+ .validateFromSnapshot(replaceBaseId)
+ .validateNoConflictingData()
+ .validateNoConflictingDeletes()
+ .addFile(FILE_UNPARTITIONED_A),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage(
+ "Found conflicting deleted files that can contain records matching
true: "
+ + "[/path/to/data-unpartitioned-a.parquet]");
}
@Test
diff --git a/core/src/test/java/org/apache/iceberg/TestReplaceTransaction.java
b/core/src/test/java/org/apache/iceberg/TestReplaceTransaction.java
index fd01b5f7db..b338d00696 100644
--- a/core/src/test/java/org/apache/iceberg/TestReplaceTransaction.java
+++ b/core/src/test/java/org/apache/iceberg/TestReplaceTransaction.java
@@ -37,6 +37,7 @@ import org.apache.iceberg.transforms.Transform;
import org.apache.iceberg.transforms.Transforms;
import org.apache.iceberg.types.TypeUtil;
import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
@@ -246,11 +247,9 @@ public class TestReplaceTransaction extends TableTestBase {
.appendFile(FILE_C)
.appendFile(FILE_D);
- AssertHelpers.assertThrows(
- "Should reject commit when last operation has not committed",
- IllegalStateException.class,
- "Cannot commit transaction: last operation has not committed",
- replace::commitTransaction);
+ Assertions.assertThatThrownBy(replace::commitTransaction)
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot commit transaction: last operation has not
committed");
Assert.assertEquals("Version should be 0", 0L, (long) version());
}
@@ -268,11 +267,9 @@ public class TestReplaceTransaction extends TableTestBase {
.appendFile(FILE_C)
.appendFile(FILE_D);
- AssertHelpers.assertThrows(
- "Should reject commit when last operation has not committed",
- IllegalStateException.class,
- "Cannot commit transaction: last operation has not committed",
- replace::commitTransaction);
+ Assertions.assertThatThrownBy(replace::commitTransaction)
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Cannot commit transaction: last operation has not
committed");
Assert.assertEquals("Version should be 0", 0L, (long) version());
}
@@ -327,11 +324,9 @@ public class TestReplaceTransaction extends TableTestBase {
// keep failing to trigger eventual transaction failure
((TestTables.TestTableOperations) ((BaseTransaction)
replace).ops()).failCommits(100);
- AssertHelpers.assertThrows(
- "Should reject commit when retries are exhausted",
- CommitFailedException.class,
- "Injected failure",
- replace::commitTransaction);
+ Assertions.assertThatThrownBy(replace::commitTransaction)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertEquals("Version should be 1", 1L, (long) version());
@@ -414,11 +409,9 @@ public class TestReplaceTransaction extends TableTestBase {
replace.newAppend().appendFile(FILE_B).commit();
- AssertHelpers.assertThrows(
- "Transaction commit should fail with CommitStateUnknownException",
- CommitStateUnknownException.class,
- "datacenter on fire",
- () -> replace.commitTransaction());
+ Assertions.assertThatThrownBy(replace::commitTransaction)
+ .isInstanceOf(CommitStateUnknownException.class)
+ .hasMessageStartingWith("datacenter on fire");
table.refresh();
@@ -466,11 +459,9 @@ public class TestReplaceTransaction extends TableTestBase {
TestTables.readMetadata("test_append"));
Assert.assertNull("Should have no metadata version",
TestTables.metadataVersion("test_append"));
- AssertHelpers.assertThrows(
- "Transaction commit should fail with CommitStateUnknownException",
- CommitStateUnknownException.class,
- "datacenter on fire",
- () -> replace.commitTransaction());
+ Assertions.assertThatThrownBy(replace::commitTransaction)
+ .isInstanceOf(CommitStateUnknownException.class)
+ .hasMessageStartingWith("datacenter on fire");
TableMetadata meta = TestTables.readMetadata("test_append");
Assert.assertNotNull("Table metadata should be created after transaction
commits", meta);
diff --git a/core/src/test/java/org/apache/iceberg/TestRewriteFiles.java
b/core/src/test/java/org/apache/iceberg/TestRewriteFiles.java
index 256c0c94cc..5b868d3d36 100644
--- a/core/src/test/java/org/apache/iceberg/TestRewriteFiles.java
+++ b/core/src/test/java/org/apache/iceberg/TestRewriteFiles.java
@@ -30,6 +30,7 @@ import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
@@ -64,119 +65,113 @@ public class TestRewriteFiles extends TableTestBase {
TableMetadata base = readMetadata();
Assert.assertNull("Should not have a current snapshot", base.ref(branch));
- AssertHelpers.assertThrows(
- "Expected an exception",
- ValidationException.class,
- "Missing required files to delete: /path/to/data-a.parquet",
- () ->
- commit(
- table,
- table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Sets.newSet(FILE_B)),
- branch));
-
- AssertHelpers.assertThrows(
- "Expected an exception",
- ValidationException.class,
- "Missing required files to delete: /path/to/data-a-deletes.parquet",
- () ->
- commit(
- table,
- table
- .newRewrite()
- .rewriteFiles(
- ImmutableSet.of(),
- ImmutableSet.of(FILE_A_DELETES),
- ImmutableSet.of(),
- ImmutableSet.of(FILE_B_DELETES)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Sets.newSet(FILE_B)),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Missing required files to delete:
/path/to/data-a.parquet");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_A_DELETES),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_B_DELETES)),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Missing required files to delete:
/path/to/data-a-deletes.parquet");
}
@Test
public void testAddOnly() {
Assert.assertEquals("Table should start empty", 0,
listManifestFiles().size());
- AssertHelpers.assertThrows(
- "Expected an exception",
- ValidationException.class,
- "Missing required files to delete: /path/to/data-a.parquet",
- () ->
- apply(
- table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Collections.emptySet()),
- branch));
-
- AssertHelpers.assertThrows(
- "Expected an exception",
- IllegalArgumentException.class,
- "Delete files to add must be empty because there's no delete file to
be rewritten",
- () ->
- apply(
- table
- .newRewrite()
- .rewriteFiles(
- ImmutableSet.of(FILE_A),
- ImmutableSet.of(),
- ImmutableSet.of(),
- ImmutableSet.of(FILE_A_DELETES)),
- branch));
-
- AssertHelpers.assertThrows(
- "Expected an exception",
- IllegalArgumentException.class,
- "Delete files to add must be empty because there's no delete file to
be rewritten",
- () ->
- apply(
- table
- .newRewrite()
- .rewriteFiles(
- ImmutableSet.of(FILE_A),
- ImmutableSet.of(),
- ImmutableSet.of(FILE_B),
- ImmutableSet.of(FILE_B_DELETES)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Collections.emptySet()),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Missing required files to delete:
/path/to/data-a.parquet");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(FILE_A),
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_A_DELETES)),
+ branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Delete files to add must be empty because there's no delete file
to be rewritten");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(FILE_A),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_B),
+ ImmutableSet.of(FILE_B_DELETES)),
+ branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Delete files to add must be empty because there's no delete file
to be rewritten");
}
@Test
public void testDeleteOnly() {
Assert.assertEquals("Table should start empty", 0,
listManifestFiles().size());
- AssertHelpers.assertThrows(
- "Expected an exception",
- IllegalArgumentException.class,
- "Files to delete cannot be empty",
- () ->
- apply(
- table.newRewrite().rewriteFiles(Collections.emptySet(),
Sets.newSet(FILE_A)),
- branch));
-
- AssertHelpers.assertThrows(
- "Expected an exception",
- IllegalArgumentException.class,
- "Files to delete cannot be empty",
- () ->
- apply(
- table
- .newRewrite()
- .rewriteFiles(
- ImmutableSet.of(),
- ImmutableSet.of(),
- ImmutableSet.of(),
- ImmutableSet.of(FILE_A_DELETES)),
- branch));
-
- AssertHelpers.assertThrows(
- "Expected an exception",
- IllegalArgumentException.class,
- "Files to delete cannot be empty",
- () ->
- apply(
- table
- .newRewrite()
- .rewriteFiles(
- ImmutableSet.of(),
- ImmutableSet.of(),
- ImmutableSet.of(FILE_A),
- ImmutableSet.of(FILE_A_DELETES)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table.newRewrite().rewriteFiles(Collections.emptySet(),
Sets.newSet(FILE_A)),
+ branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Files to delete cannot be empty");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_A_DELETES)),
+ branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Files to delete cannot be empty");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table
+ .newRewrite()
+ .rewriteFiles(
+ ImmutableSet.of(),
+ ImmutableSet.of(),
+ ImmutableSet.of(FILE_A),
+ ImmutableSet.of(FILE_A_DELETES)),
+ branch))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Files to delete cannot be empty");
}
@Test
@@ -432,11 +427,9 @@ public class TestRewriteFiles extends TableTestBase {
validateManifestEntries(manifest1, ids(pending.snapshotId()),
files(FILE_B), statuses(ADDED));
validateManifestEntries(manifest2, ids(pending.snapshotId()),
files(FILE_A), statuses(DELETED));
- AssertHelpers.assertThrows(
- "Should retry 4 times and throw last failure",
- CommitFailedException.class,
- "Injected failure",
- () -> commit(table, rewrite, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rewrite, branch))
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertFalse("Should clean up new manifest", new
File(manifest1.path()).exists());
Assert.assertFalse("Should clean up new manifest", new
File(manifest2.path()).exists());
@@ -500,11 +493,9 @@ public class TestRewriteFiles extends TableTestBase {
files(FILE_A_DELETES, FILE_B_DELETES),
statuses(DELETED, DELETED));
- AssertHelpers.assertThrows(
- "Should retry 4 times and throw last failure",
- CommitFailedException.class,
- "Injected failure",
- rewrite::commit);
+ Assertions.assertThatThrownBy(rewrite::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertFalse("Should clean up new manifest", new
File(manifest1.path()).exists());
Assert.assertFalse("Should clean up new manifest", new
File(manifest2.path()).exists());
@@ -731,15 +722,14 @@ public class TestRewriteFiles extends TableTestBase {
1,
latestSnapshot(base, branch).allManifests(table.io()).size());
- AssertHelpers.assertThrows(
- "Expected an exception",
- ValidationException.class,
- "Missing required files to delete: /path/to/data-c.parquet",
- () ->
- commit(
- table,
- table.newRewrite().rewriteFiles(Sets.newSet(FILE_C),
Sets.newSet(FILE_D)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table.newRewrite().rewriteFiles(Sets.newSet(FILE_C),
Sets.newSet(FILE_D)),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Missing required files to delete:
/path/to/data-c.parquet");
Assert.assertEquals("Only 1 manifests should exist", 1,
listManifestFiles().size());
}
@@ -775,15 +765,14 @@ public class TestRewriteFiles extends TableTestBase {
commit(table, rewrite, branch);
- AssertHelpers.assertThrows(
- "Expected an exception",
- ValidationException.class,
- "Missing required files to delete: /path/to/data-a.parquet",
- () ->
- commit(
- table,
- table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Sets.newSet(FILE_D)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table.newRewrite().rewriteFiles(Sets.newSet(FILE_A),
Sets.newSet(FILE_D)),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Missing required files to delete:
/path/to/data-a.parquet");
Assert.assertEquals("Only 3 manifests should exist", 3,
listManifestFiles().size());
}
@@ -800,17 +789,16 @@ public class TestRewriteFiles extends TableTestBase {
long snapshotAfterDeletes = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail because deletes were added after the starting snapshot",
- ValidationException.class,
- "Cannot commit, found new delete for replaced data file",
- () ->
- apply(
- table
- .newRewrite()
- .validateFromSnapshot(snapshotBeforeDeletes)
- .rewriteFiles(Sets.newSet(FILE_A), Sets.newSet(FILE_A2)),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ apply(
+ table
+ .newRewrite()
+ .validateFromSnapshot(snapshotBeforeDeletes)
+ .rewriteFiles(Sets.newSet(FILE_A),
Sets.newSet(FILE_A2)),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, found new delete for replaced
data file");
// the rewrite should be valid when validating from the snapshot after the
deletes
apply(
diff --git a/core/src/test/java/org/apache/iceberg/TestRewriteManifests.java
b/core/src/test/java/org/apache/iceberg/TestRewriteManifests.java
index f6717345cc..dbda87baf7 100644
--- a/core/src/test/java/org/apache/iceberg/TestRewriteManifests.java
+++ b/core/src/test/java/org/apache/iceberg/TestRewriteManifests.java
@@ -36,6 +36,7 @@ import org.apache.iceberg.exceptions.RuntimeIOException;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.expressions.Expressions;
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;
@@ -822,11 +823,9 @@ public class TestRewriteManifests extends TableTestBase {
table.newDelete().deleteFile(FILE_A).commit();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Manifest is missing",
- rewriteManifests::commit);
+ Assertions.assertThatThrownBy(rewriteManifests::commit)
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Manifest is missing");
}
@Test
@@ -966,16 +965,15 @@ public class TestRewriteManifests extends TableTestBase {
ManifestFile invalidAddedFileManifest =
writeManifest("manifest-file-2.avro", appendEntry);
- AssertHelpers.assertThrows(
- "Should reject commit",
- IllegalArgumentException.class,
- "Cannot add manifest with added files",
- () ->
- table
- .rewriteManifests()
- .deleteManifest(manifest)
- .addManifest(invalidAddedFileManifest)
- .commit());
+ Assertions.assertThatThrownBy(
+ () ->
+ table
+ .rewriteManifests()
+ .deleteManifest(manifest)
+ .addManifest(invalidAddedFileManifest)
+ .commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot add manifest with added files");
ManifestEntry<DataFile> deleteEntry =
manifestEntry(ManifestEntry.Status.DELETED, snapshot.snapshotId(),
FILE_A);
@@ -984,22 +982,20 @@ public class TestRewriteManifests extends TableTestBase {
ManifestFile invalidDeletedFileManifest =
writeManifest("manifest-file-3.avro", deleteEntry);
- AssertHelpers.assertThrows(
- "Should reject commit",
- IllegalArgumentException.class,
- "Cannot add manifest with deleted files",
- () ->
- table
- .rewriteManifests()
- .deleteManifest(manifest)
- .addManifest(invalidDeletedFileManifest)
- .commit());
-
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "must have the same number of active files",
- () -> table.rewriteManifests().deleteManifest(manifest).commit());
+ Assertions.assertThatThrownBy(
+ () ->
+ table
+ .rewriteManifests()
+ .deleteManifest(manifest)
+ .addManifest(invalidDeletedFileManifest)
+ .commit())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot add manifest with deleted files");
+
+ Assertions.assertThatThrownBy(() ->
table.rewriteManifests().deleteManifest(manifest).commit())
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith(
+ "Replaced and created manifests must have the same number of
active files");
}
@Test
@@ -1035,11 +1031,9 @@ public class TestRewriteManifests extends TableTestBase {
rewriteManifests.deleteManifest(secondSnapshotManifest);
rewriteManifests.addManifest(newManifest);
- AssertHelpers.assertThrows(
- "Should reject commit",
- CommitFailedException.class,
- "Injected failure",
- rewriteManifests::commit);
+ Assertions.assertThatThrownBy(rewriteManifests::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertTrue("New manifest should not be deleted", new
File(newManifest.path()).exists());
}
@@ -1079,11 +1073,9 @@ public class TestRewriteManifests extends TableTestBase {
rewriteManifests.deleteManifest(secondSnapshotManifest);
rewriteManifests.addManifest(newManifest);
- AssertHelpers.assertThrows(
- "Should reject commit",
- CommitFailedException.class,
- "Injected failure",
- rewriteManifests::commit);
+ Assertions.assertThatThrownBy(rewriteManifests::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessage("Injected failure");
Assert.assertTrue("New manifest should not be deleted", new
File(newManifest.path()).exists());
}
@@ -1095,11 +1087,10 @@ public class TestRewriteManifests extends TableTestBase
{
Assert.assertEquals(1,
table.currentSnapshot().allManifests(table.io()).size());
- AssertHelpers.assertThrows(
- "Should reject committing rewrite manifests to branch",
- UnsupportedOperationException.class,
- "Cannot commit to branch someBranch:
org.apache.iceberg.BaseRewriteManifests does not support branch commits",
- () -> table.rewriteManifests().toBranch("someBranch").commit());
+ Assertions.assertThatThrownBy(() ->
table.rewriteManifests().toBranch("someBranch").commit())
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage(
+ "Cannot commit to branch someBranch:
org.apache.iceberg.BaseRewriteManifests does not support branch commits");
}
private void validateSummary(
diff --git a/core/src/test/java/org/apache/iceberg/TestRowDelta.java
b/core/src/test/java/org/apache/iceberg/TestRowDelta.java
index ab2ec5f713..fa04f36d36 100644
--- a/core/src/test/java/org/apache/iceberg/TestRowDelta.java
+++ b/core/src/test/java/org/apache/iceberg/TestRowDelta.java
@@ -38,6 +38,7 @@ 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.ImmutableSet;
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -115,19 +116,18 @@ public class TestRowDelta extends V2TableTestBase {
long deleteSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateFromSnapshot(validateFromSnapshotId)
- .validateDataFilesExist(ImmutableList.of(FILE_A.path())),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateFromSnapshot(validateFromSnapshotId)
+
.validateDataFilesExist(ImmutableList.of(FILE_A.path())),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -174,19 +174,18 @@ public class TestRowDelta extends V2TableTestBase {
long deleteSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateFromSnapshot(validateFromSnapshotId)
- .validateDataFilesExist(ImmutableList.of(FILE_A.path())),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateFromSnapshot(validateFromSnapshotId)
+
.validateDataFilesExist(ImmutableList.of(FILE_A.path())),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -211,19 +210,18 @@ public class TestRowDelta extends V2TableTestBase {
long deleteSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateFromSnapshot(validateFromSnapshotId)
- .validateDataFilesExist(ImmutableList.of(FILE_A.path())),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateFromSnapshot(validateFromSnapshotId)
+
.validateDataFilesExist(ImmutableList.of(FILE_A.path())),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -310,19 +308,18 @@ public class TestRowDelta extends V2TableTestBase {
long deleteSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateFromSnapshot(validateFromSnapshotId)
- .validateDataFilesExist(ImmutableList.of(FILE_A.path())),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateFromSnapshot(validateFromSnapshotId)
+
.validateDataFilesExist(ImmutableList.of(FILE_A.path())),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -347,20 +344,19 @@ public class TestRowDelta extends V2TableTestBase {
long deleteSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateDeletedFiles()
- .validateFromSnapshot(validateFromSnapshotId)
- .validateDataFilesExist(ImmutableList.of(FILE_A.path())),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateDeletedFiles()
+ .validateFromSnapshot(validateFromSnapshotId)
+
.validateDataFilesExist(ImmutableList.of(FILE_A.path())),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -385,20 +381,20 @@ public class TestRowDelta extends V2TableTestBase {
long appendSnapshotId = latestSnapshot(table, branch).snapshotId();
- AssertHelpers.assertThrows(
- "Should fail to add FILE_A_DELETES because FILE_A2 was added",
- ValidationException.class,
- "Found conflicting files",
- () ->
- commit(
- table,
- table
- .newRowDelta()
- .addDeletes(FILE_A_DELETES)
- .validateFromSnapshot(validateFromSnapshotId)
- .conflictDetectionFilter(Expressions.equal("data", "u"))
// bucket16("u") -> 0
- .validateNoConflictingDataFiles(),
- branch));
+ Assertions.assertThatThrownBy(
+ () ->
+ commit(
+ table,
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES)
+ .validateFromSnapshot(validateFromSnapshotId)
+ .conflictDetectionFilter(
+ Expressions.equal("data", "u")) // bucket16("u")
-> 0
+ .validateNoConflictingDataFiles(),
+ branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found conflicting files");
Assert.assertEquals(
"Table state should not be modified by failed RowDelta operation",
@@ -885,11 +881,9 @@ public class TestRowDelta extends V2TableTestBase {
// concurrently delete the file for partition A
commit(table, table.newDelete().deleteFile(dataFile1), branch);
- AssertHelpers.assertThrows(
- "Should fail to add deletes because data file is missing",
- ValidationException.class,
- "Cannot commit, missing data files",
- () -> commit(table, rowDelta, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rowDelta, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
}
@Test
@@ -1133,11 +1127,9 @@ public class TestRowDelta extends V2TableTestBase {
// perform a conflicting concurrent operation
commit(table, table.newDelete().deleteFile(firstSnapshotDataFile), branch);
- AssertHelpers.assertThrows(
- "Should fail to commit row delta",
- ValidationException.class,
- "Cannot commit, missing data files",
- () -> commit(table, rowDelta, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rowDelta, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, missing data files");
// we should clean up 1 manifest list and 2 delete manifests
Assert.assertEquals("Should delete 3 files", 3, deletedFiles.size());
@@ -1172,11 +1164,9 @@ public class TestRowDelta extends V2TableTestBase {
.validateNoConflictingDataFiles()
.commit();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found new conflicting delete files",
- () -> commit(table, rowDelta, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rowDelta, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found new conflicting delete files");
}
@Test
@@ -1205,11 +1195,9 @@ public class TestRowDelta extends V2TableTestBase {
.validateNoConflictingDataFiles()
.commit();
- AssertHelpers.assertThrows(
- "Should reject commit",
- ValidationException.class,
- "Found new conflicting delete files",
- () -> commit(table, rowDelta, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rowDelta, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found new conflicting delete files");
}
@Test
@@ -1445,11 +1433,9 @@ public class TestRowDelta extends V2TableTestBase {
commit(table, rowDelta, branch);
- AssertHelpers.assertThrows(
- "Should not allow any new position delete associated with the data
file",
- ValidationException.class,
- "Cannot commit, found new position delete for replaced data file",
- () -> commit(table, rewriteFiles, branch));
+ Assertions.assertThatThrownBy(() -> commit(table, rewriteFiles, branch))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot commit, found new position delete for
replaced data file");
}
@Test
@@ -1462,55 +1448,52 @@ public class TestRowDelta extends V2TableTestBase {
Expression conflictDetectionFilter =
Expressions.equal(Expressions.bucket("dAtA", 16), 0);
- AssertHelpers.assertThrows(
- "Should use case sensitive binding by default",
- ValidationException.class,
- "Cannot find field 'dAtA'",
- () ->
- table
- .newRowDelta()
- .toBranch(branch)
- .addRows(FILE_B)
- .addDeletes(FILE_A2_DELETES)
- .validateFromSnapshot(firstSnapshot.snapshotId())
- .conflictDetectionFilter(conflictDetectionFilter)
- .validateNoConflictingDataFiles()
- .validateNoConflictingDeleteFiles()
- .commit());
-
- AssertHelpers.assertThrows(
- "Should fail with case sensitive binding",
- ValidationException.class,
- "Cannot find field 'dAtA'",
- () ->
- table
- .newRowDelta()
- .toBranch(branch)
- .caseSensitive(true)
- .addRows(FILE_B)
- .addDeletes(FILE_A2_DELETES)
- .validateFromSnapshot(firstSnapshot.snapshotId())
- .conflictDetectionFilter(conflictDetectionFilter)
- .validateNoConflictingDataFiles()
- .validateNoConflictingDeleteFiles()
- .commit());
+ Assertions.assertThatThrownBy(
+ () ->
+ table
+ .newRowDelta()
+ .toBranch(branch)
+ .addRows(FILE_B)
+ .addDeletes(FILE_A2_DELETES)
+ .validateFromSnapshot(firstSnapshot.snapshotId())
+ .conflictDetectionFilter(conflictDetectionFilter)
+ .validateNoConflictingDataFiles()
+ .validateNoConflictingDeleteFiles()
+ .commit())
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot find field 'dAtA'");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ table
+ .newRowDelta()
+ .toBranch(branch)
+ .caseSensitive(true)
+ .addRows(FILE_B)
+ .addDeletes(FILE_A2_DELETES)
+ .validateFromSnapshot(firstSnapshot.snapshotId())
+ .conflictDetectionFilter(conflictDetectionFilter)
+ .validateNoConflictingDataFiles()
+ .validateNoConflictingDeleteFiles()
+ .commit())
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Cannot find field 'dAtA'");
// binding should succeed and trigger the validation
- AssertHelpers.assertThrows(
- "Should reject case sensitive binding",
- ValidationException.class,
- "Found new conflicting delete files",
- () ->
- table
- .newRowDelta()
- .toBranch(branch)
- .caseSensitive(false)
- .addRows(FILE_B)
- .addDeletes(FILE_A2_DELETES)
- .validateFromSnapshot(firstSnapshot.snapshotId())
- .conflictDetectionFilter(conflictDetectionFilter)
- .validateNoConflictingDataFiles()
- .validateNoConflictingDeleteFiles()
- .commit());
+ Assertions.assertThatThrownBy(
+ () ->
+ table
+ .newRowDelta()
+ .toBranch(branch)
+ .caseSensitive(false)
+ .addRows(FILE_B)
+ .addDeletes(FILE_A2_DELETES)
+ .validateFromSnapshot(firstSnapshot.snapshotId())
+ .conflictDetectionFilter(conflictDetectionFilter)
+ .validateNoConflictingDataFiles()
+ .validateNoConflictingDeleteFiles()
+ .commit())
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Found new conflicting delete files");
}
}
diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java
b/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java
index d193671d61..d8940367e1 100644
--- a/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java
+++ b/core/src/test/java/org/apache/iceberg/TestSnapshotRefParser.java
@@ -18,6 +18,7 @@
*/
package org.apache.iceberg;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
@@ -106,77 +107,59 @@ public class TestSnapshotRefParser {
@Test
public void testFailParsingWhenNullOrEmptyJson() {
String nullJson = null;
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize null JSON string",
- IllegalArgumentException.class,
- "Cannot parse snapshot ref from invalid JSON",
- () -> SnapshotRefParser.fromJson(nullJson));
+ Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(nullJson))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Cannot parse snapshot ref from invalid JSON");
String emptyJson = "";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize empty JSON string",
- IllegalArgumentException.class,
- "Cannot parse snapshot ref from invalid JSON",
- () -> SnapshotRefParser.fromJson(emptyJson));
+ Assertions.assertThatThrownBy(() -> SnapshotRefParser.fromJson(emptyJson))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Cannot parse snapshot ref from invalid JSON");
}
@Test
public void testFailParsingWhenMissingRequiredFields() {
String refMissingType = "{\"snapshot-id\":1}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with missing type",
- IllegalArgumentException.class,
- "Cannot parse missing string",
- () -> SnapshotRefParser.fromJson(refMissingType));
+ Assertions.assertThatThrownBy(() ->
SnapshotRefParser.fromJson(refMissingType))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Cannot parse missing string");
String refMissingSnapshotId = "{\"type\":\"branch\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with missing
snapshot id",
- IllegalArgumentException.class,
- "Cannot parse missing long",
- () -> SnapshotRefParser.fromJson(refMissingSnapshotId));
+ Assertions.assertThatThrownBy(() ->
SnapshotRefParser.fromJson(refMissingSnapshotId))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Cannot parse missing long");
}
@Test
public void testFailWhenFieldsHaveInvalidValues() {
String invalidSnapshotId =
"{\"snapshot-id\":\"invalid-snapshot-id\",\"type\":\"not-a-valid-tag-type\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with invalid
snapshot id",
- IllegalArgumentException.class,
- "Cannot parse to a long value: snapshot-id: \"invalid-snapshot-id\"",
- () -> SnapshotRefParser.fromJson(invalidSnapshotId));
+ Assertions.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\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with invalid tag",
- IllegalArgumentException.class,
- "Invalid snapshot ref type: not-a-valid-tag-type",
- () -> SnapshotRefParser.fromJson(invalidTagType));
+ Assertions.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\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with invalid ref
age",
- IllegalArgumentException.class,
- "Cannot parse to a long value: max-ref-age-ms: \"not-a-valid-value\"",
- () -> SnapshotRefParser.fromJson(invalidRefAge));
+ Assertions.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\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with missing
snapshot id",
- IllegalArgumentException.class,
- "Cannot parse to an integer value: min-snapshots-to-keep:
\"invalid-number\"",
- () -> SnapshotRefParser.fromJson(invalidSnapshotsToKeep));
+ Assertions.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\"}";
- AssertHelpers.assertThrows(
- "SnapshotRefParser should fail to deserialize ref with missing
snapshot id",
- IllegalArgumentException.class,
- "Cannot parse to a long value: max-snapshot-age-ms: \"invalid-age\"",
- () -> SnapshotRefParser.fromJson(invalidMaxSnapshotAge));
+ Assertions.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/TestTableMetadata.java
b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
index aad46bd56c..879d4e73e2 100644
--- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
+++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java
@@ -406,35 +406,34 @@ public class TestTableMetadata {
Map<String, SnapshotRef> refs =
ImmutableMap.of("main",
SnapshotRef.branchBuilder(previousSnapshotId).build());
- AssertHelpers.assertThrows(
- "Should fail if main branch snapshot ID does not match
currentSnapshotId",
- IllegalArgumentException.class,
- "Current snapshot ID does not match main branch",
- () ->
- new TableMetadata(
- null,
- 2,
- UUID.randomUUID().toString(),
- TEST_LOCATION,
- SEQ_NO,
- System.currentTimeMillis(),
- 3,
- 7,
- ImmutableList.of(TEST_SCHEMA, schema),
- 5,
- ImmutableList.of(SPEC_5),
- SPEC_5.lastAssignedFieldId(),
- 3,
- ImmutableList.of(SORT_ORDER_3),
- ImmutableMap.of("property", "value"),
- currentSnapshotId,
- Arrays.asList(previousSnapshot, currentSnapshot),
- null,
- snapshotLog,
- ImmutableList.of(),
- refs,
- ImmutableList.of(),
- ImmutableList.of()));
+ Assertions.assertThatThrownBy(
+ () ->
+ new TableMetadata(
+ null,
+ 2,
+ UUID.randomUUID().toString(),
+ TEST_LOCATION,
+ SEQ_NO,
+ System.currentTimeMillis(),
+ 3,
+ 7,
+ ImmutableList.of(TEST_SCHEMA, schema),
+ 5,
+ ImmutableList.of(SPEC_5),
+ SPEC_5.lastAssignedFieldId(),
+ 3,
+ ImmutableList.of(SORT_ORDER_3),
+ ImmutableMap.of("property", "value"),
+ currentSnapshotId,
+ Arrays.asList(previousSnapshot, currentSnapshot),
+ null,
+ snapshotLog,
+ ImmutableList.of(),
+ refs,
+ ImmutableList.of(),
+ ImmutableList.of()))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Current snapshot ID does not match main
branch");
}
@Test
@@ -451,35 +450,34 @@ public class TestTableMetadata {
Map<String, SnapshotRef> refs =
ImmutableMap.of("main", SnapshotRef.branchBuilder(snapshotId).build());
- AssertHelpers.assertThrows(
- "Should fail if main branch snapshot ID does not match
currentSnapshotId",
- IllegalArgumentException.class,
- "Current snapshot is not set, but main branch exists",
- () ->
- new TableMetadata(
- null,
- 2,
- UUID.randomUUID().toString(),
- TEST_LOCATION,
- SEQ_NO,
- System.currentTimeMillis(),
- 3,
- 7,
- ImmutableList.of(TEST_SCHEMA, schema),
- 5,
- ImmutableList.of(SPEC_5),
- SPEC_5.lastAssignedFieldId(),
- 3,
- ImmutableList.of(SORT_ORDER_3),
- ImmutableMap.of("property", "value"),
- -1,
- ImmutableList.of(snapshot),
- null,
- ImmutableList.of(),
- ImmutableList.of(),
- refs,
- ImmutableList.of(),
- ImmutableList.of()));
+ Assertions.assertThatThrownBy(
+ () ->
+ new TableMetadata(
+ null,
+ 2,
+ UUID.randomUUID().toString(),
+ TEST_LOCATION,
+ SEQ_NO,
+ System.currentTimeMillis(),
+ 3,
+ 7,
+ ImmutableList.of(TEST_SCHEMA, schema),
+ 5,
+ ImmutableList.of(SPEC_5),
+ SPEC_5.lastAssignedFieldId(),
+ 3,
+ ImmutableList.of(SORT_ORDER_3),
+ ImmutableMap.of("property", "value"),
+ -1,
+ ImmutableList.of(snapshot),
+ null,
+ ImmutableList.of(),
+ ImmutableList.of(),
+ refs,
+ ImmutableList.of(),
+ ImmutableList.of()))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Current snapshot is not set, but main branch
exists");
}
@Test
@@ -491,35 +489,34 @@ public class TestTableMetadata {
Map<String, SnapshotRef> refs =
ImmutableMap.of("main", SnapshotRef.branchBuilder(snapshotId).build());
- AssertHelpers.assertThrows(
- "Should fail if main branch snapshot ID does not match
currentSnapshotId",
- IllegalArgumentException.class,
- "does not exist in the existing snapshots list",
- () ->
- new TableMetadata(
- null,
- 2,
- UUID.randomUUID().toString(),
- TEST_LOCATION,
- SEQ_NO,
- System.currentTimeMillis(),
- 3,
- 7,
- ImmutableList.of(TEST_SCHEMA, schema),
- 5,
- ImmutableList.of(SPEC_5),
- SPEC_5.lastAssignedFieldId(),
- 3,
- ImmutableList.of(SORT_ORDER_3),
- ImmutableMap.of("property", "value"),
- -1,
- ImmutableList.of(),
- null,
- ImmutableList.of(),
- ImmutableList.of(),
- refs,
- ImmutableList.of(),
- ImmutableList.of()));
+ Assertions.assertThatThrownBy(
+ () ->
+ new TableMetadata(
+ null,
+ 2,
+ UUID.randomUUID().toString(),
+ TEST_LOCATION,
+ SEQ_NO,
+ System.currentTimeMillis(),
+ 3,
+ 7,
+ ImmutableList.of(TEST_SCHEMA, schema),
+ 5,
+ ImmutableList.of(SPEC_5),
+ SPEC_5.lastAssignedFieldId(),
+ 3,
+ ImmutableList.of(SORT_ORDER_3),
+ ImmutableMap.of("property", "value"),
+ -1,
+ ImmutableList.of(),
+ null,
+ ImmutableList.of(),
+ ImmutableList.of(),
+ refs,
+ ImmutableList.of(),
+ ImmutableList.of()))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageEndingWith("does not exist in the existing snapshots list");
}
private static String toJsonWithoutSpecAndSchemaList(TableMetadata metadata)
{
@@ -922,69 +919,67 @@ public class TestTableMetadata {
@Test
public void testV2UUIDValidation() {
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without a UUID",
- IllegalArgumentException.class,
- "UUID is required in format v2",
- () ->
- new TableMetadata(
- null,
- 2,
- null,
- TEST_LOCATION,
- SEQ_NO,
- System.currentTimeMillis(),
- LAST_ASSIGNED_COLUMN_ID,
- 7,
- ImmutableList.of(TEST_SCHEMA),
- SPEC_5.specId(),
- ImmutableList.of(SPEC_5),
- SPEC_5.lastAssignedFieldId(),
- 3,
- ImmutableList.of(SORT_ORDER_3),
- ImmutableMap.of(),
- -1L,
- ImmutableList.of(),
- null,
- ImmutableList.of(),
- ImmutableList.of(),
- ImmutableMap.of(),
- ImmutableList.of(),
- ImmutableList.of()));
+ Assertions.assertThatThrownBy(
+ () ->
+ new TableMetadata(
+ null,
+ 2,
+ null,
+ TEST_LOCATION,
+ SEQ_NO,
+ System.currentTimeMillis(),
+ LAST_ASSIGNED_COLUMN_ID,
+ 7,
+ ImmutableList.of(TEST_SCHEMA),
+ SPEC_5.specId(),
+ ImmutableList.of(SPEC_5),
+ SPEC_5.lastAssignedFieldId(),
+ 3,
+ ImmutableList.of(SORT_ORDER_3),
+ ImmutableMap.of(),
+ -1L,
+ ImmutableList.of(),
+ null,
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableMap.of(),
+ ImmutableList.of(),
+ ImmutableList.of()))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("UUID is required in format v2");
}
@Test
public void testVersionValidation() {
int unsupportedVersion = TableMetadata.SUPPORTED_TABLE_FORMAT_VERSION + 1;
- AssertHelpers.assertThrows(
- "Should reject unsupported metadata",
- IllegalArgumentException.class,
- "Unsupported format version: v" + unsupportedVersion,
- () ->
- new TableMetadata(
- null,
- unsupportedVersion,
- null,
- TEST_LOCATION,
- SEQ_NO,
- System.currentTimeMillis(),
- LAST_ASSIGNED_COLUMN_ID,
- 7,
- ImmutableList.of(TEST_SCHEMA),
- SPEC_5.specId(),
- ImmutableList.of(SPEC_5),
- SPEC_5.lastAssignedFieldId(),
- 3,
- ImmutableList.of(SORT_ORDER_3),
- ImmutableMap.of(),
- -1L,
- ImmutableList.of(),
- null,
- ImmutableList.of(),
- ImmutableList.of(),
- ImmutableMap.of(),
- ImmutableList.of(),
- ImmutableList.of()));
+ Assertions.assertThatThrownBy(
+ () ->
+ new TableMetadata(
+ null,
+ unsupportedVersion,
+ null,
+ TEST_LOCATION,
+ SEQ_NO,
+ System.currentTimeMillis(),
+ LAST_ASSIGNED_COLUMN_ID,
+ 7,
+ ImmutableList.of(TEST_SCHEMA),
+ SPEC_5.specId(),
+ ImmutableList.of(SPEC_5),
+ SPEC_5.lastAssignedFieldId(),
+ 3,
+ ImmutableList.of(SORT_ORDER_3),
+ ImmutableMap.of(),
+ -1L,
+ ImmutableList.of(),
+ null,
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableMap.of(),
+ ImmutableList.of(),
+ ImmutableList.of()))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Unsupported format version: v" + unsupportedVersion);
}
@Test
@@ -998,63 +993,51 @@ public class TestTableMetadata {
Assert.assertNotNull("Should successfully read supported metadata
version", parsed2);
String unsupportedVersion =
readTableMetadataInputFile("TableMetadataUnsupportedVersion.json");
- AssertHelpers.assertThrows(
- "Should not read unsupported metadata",
- IllegalArgumentException.class,
- "Cannot read unsupported version",
- () -> TableMetadataParser.fromJson(unsupportedVersion));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupportedVersion))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith("Cannot read unsupported version");
}
@Test
public void testParserV2PartitionSpecsValidation() throws Exception {
String unsupportedVersion =
readTableMetadataInputFile("TableMetadataV2MissingPartitionSpecs.json");
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without partition specs",
- IllegalArgumentException.class,
- "partition-specs must exist in format v2",
- () -> TableMetadataParser.fromJson(unsupportedVersion));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupportedVersion))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("partition-specs must exist in format v2");
}
@Test
public void testParserV2LastAssignedFieldIdValidation() throws Exception {
String unsupportedVersion =
readTableMetadataInputFile("TableMetadataV2MissingLastPartitionId.json");
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without last assigned partition field id",
- IllegalArgumentException.class,
- "last-partition-id must exist in format v2",
- () -> TableMetadataParser.fromJson(unsupportedVersion));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupportedVersion))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("last-partition-id must exist in format v2");
}
@Test
public void testParserV2SortOrderValidation() throws Exception {
String unsupportedVersion =
readTableMetadataInputFile("TableMetadataV2MissingSortOrder.json");
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without sort order",
- IllegalArgumentException.class,
- "sort-orders must exist in format v2",
- () -> TableMetadataParser.fromJson(unsupportedVersion));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupportedVersion))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("sort-orders must exist in format v2");
}
@Test
public void testParserV2CurrentSchemaIdValidation() throws Exception {
String unsupported =
readTableMetadataInputFile("TableMetadataV2CurrentSchemaNotFound.json");
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without valid schema id",
- IllegalArgumentException.class,
- "Cannot find schema with current-schema-id=2 from schemas",
- () -> TableMetadataParser.fromJson(unsupported));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupported))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot find schema with current-schema-id=2 from
schemas");
}
@Test
public void testParserV2SchemasValidation() throws Exception {
String unsupported =
readTableMetadataInputFile("TableMetadataV2MissingSchemas.json");
- AssertHelpers.assertThrows(
- "Should reject v2 metadata without schemas",
- IllegalArgumentException.class,
- "schemas must exist in format v2",
- () -> TableMetadataParser.fromJson(unsupported));
+ Assertions.assertThatThrownBy(() ->
TableMetadataParser.fromJson(unsupported))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("schemas must exist in format v2");
}
private String readTableMetadataInputFile(String fileName) throws Exception {
@@ -1105,11 +1088,9 @@ public class TestTableMetadata {
TableMetadata.newTableMetadata(
schema, PartitionSpec.unpartitioned(), location,
ImmutableMap.of());
- AssertHelpers.assertThrows(
- "Should fail to update an invalid partition spec",
- ValidationException.class,
- "Spec does not use sequential IDs that are required in v1",
- () -> metadata.updatePartitionSpec(spec));
+ Assertions.assertThatThrownBy(() -> metadata.updatePartitionSpec(spec))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageStartingWith("Spec does not use sequential IDs that are
required in v1");
}
@Test
@@ -1547,31 +1528,30 @@ public class TestTableMetadata {
public void testNoReservedPropertyForTableMetadataCreation() {
Schema schema = new Schema(Types.NestedField.required(10, "x",
Types.StringType.get()));
- AssertHelpers.assertThrows(
- "should not allow reserved table property when creating table
metadata",
- IllegalArgumentException.class,
- "Table properties should not contain reserved properties, but got
{format-version=1}",
- () ->
- TableMetadata.newTableMetadata(
- schema,
- PartitionSpec.unpartitioned(),
- null,
- "/tmp",
- ImmutableMap.of(TableProperties.FORMAT_VERSION, "1"),
- 1));
-
- AssertHelpers.assertThrows(
- "should not allow reserved table property when creating table
metadata",
- IllegalArgumentException.class,
- "Table properties should not contain reserved properties, but got
{uuid=uuid}",
- () ->
- TableMetadata.newTableMetadata(
- schema,
- PartitionSpec.unpartitioned(),
- null,
- "/tmp",
- ImmutableMap.of(TableProperties.UUID, "uuid"),
- 1));
+ Assertions.assertThatThrownBy(
+ () ->
+ TableMetadata.newTableMetadata(
+ schema,
+ PartitionSpec.unpartitioned(),
+ null,
+ "/tmp",
+ ImmutableMap.of(TableProperties.FORMAT_VERSION, "1"),
+ 1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Table properties should not contain reserved properties, but got
{format-version=1}");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ TableMetadata.newTableMetadata(
+ schema,
+ PartitionSpec.unpartitioned(),
+ null,
+ "/tmp",
+ ImmutableMap.of(TableProperties.UUID, "uuid"),
+ 1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Table properties should not contain reserved properties,
but got {uuid=uuid}");
}
@Test
