diff --git a/core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java b/core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java index 6c00f8b38c33..04cb64403523 100644 --- a/core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java +++ b/core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java @@ -30,6 +30,7 @@ import org.apache.iceberg.types.TypeUtil; import org.apache.iceberg.types.Types; import org.apache.iceberg.util.Pair; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; @@ -303,11 +304,10 @@ public void testUpdateFailure() { } String typeChange = fromType.toString() + " -> " + toType.toString(); - AssertHelpers.assertThrows( - "Should reject update: " + typeChange, - IllegalArgumentException.class, - "change column type: col: " + typeChange, - () -> new SchemaUpdate(fromSchema, 1).updateColumn("col", toType)); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(fromSchema, 1).updateColumn("col", toType)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot change column type: col: " + typeChange); } } } @@ -586,11 +586,10 @@ public void testAddRequiredColumn() { required(1, "id", Types.IntegerType.get()), required(2, "data", Types.StringType.get())); - AssertHelpers.assertThrows( - "Should reject add required column if incompatible changes are not allowed", - IllegalArgumentException.class, - "Incompatible change: cannot add required column: data", - () -> new SchemaUpdate(schema, 1).addRequiredColumn("data", Types.StringType.get())); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 1).addRequiredColumn("data", Types.StringType.get())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Incompatible change: cannot add required column: data"); Schema result = new SchemaUpdate(schema, 1) @@ -605,16 +604,15 @@ public void testAddRequiredColumn() { public void testAddRequiredColumnCaseInsensitive() { Schema schema = new Schema(required(1, "id", Types.IntegerType.get())); - AssertHelpers.assertThrows( - "Should reject add required column if same column name different case found", - IllegalArgumentException.class, - "Cannot add column, name already exists: ID", - () -> - new SchemaUpdate(schema, 1) - .caseSensitive(false) - .allowIncompatibleChanges() - .addRequiredColumn("ID", Types.StringType.get()) - .apply()); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 1) + .caseSensitive(false) + .allowIncompatibleChanges() + .addRequiredColumn("ID", Types.StringType.get()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add column, name already exists: ID"); } @Test @@ -633,11 +631,9 @@ public void testRequireColumn() { Schema schema = new Schema(optional(1, "id", Types.IntegerType.get())); Schema expected = new Schema(required(1, "id", Types.IntegerType.get())); - AssertHelpers.assertThrows( - "Should reject change to required if incompatible changes are not allowed", - IllegalArgumentException.class, - "Cannot change column nullability: id: optional -> required", - () -> new SchemaUpdate(schema, 1).requireColumn("id")); + Assertions.assertThatThrownBy(() -> new SchemaUpdate(schema, 1).requireColumn("id")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot change column nullability: id: optional -> required"); // required to required is not an incompatible change new SchemaUpdate(expected, 1).requireColumn("id").apply(); @@ -739,34 +735,32 @@ public void testMixedChanges() { @Test public void testAmbiguousAdd() { // preferences.booleans could be top-level or a field of preferences - AssertHelpers.assertThrows( - "Should reject ambiguous column name", - IllegalArgumentException.class, - "ambiguous name: preferences.booleans", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.addColumn("preferences.booleans", Types.BooleanType.get()); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.addColumn("preferences.booleans", Types.BooleanType.get()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add column with ambiguous name: preferences.booleans"); } @Test public void testAddAlreadyExists() { - AssertHelpers.assertThrows( - "Should reject column name that already exists", - IllegalArgumentException.class, - "already exists: preferences.feature1", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.addColumn("preferences", "feature1", Types.BooleanType.get()); - }); - AssertHelpers.assertThrows( - "Should reject column name that already exists", - IllegalArgumentException.class, - "already exists: preferences", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.addColumn("preferences", Types.BooleanType.get()); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.addColumn("preferences", "feature1", Types.BooleanType.get()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add column, name already exists: preferences.feature1"); + + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.addColumn("preferences", Types.BooleanType.get()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add column, name already exists: preferences"); } @Test @@ -839,153 +833,141 @@ public void testDeleteThenAddNested() { @Test public void testDeleteMissingColumn() { - AssertHelpers.assertThrows( - "Should reject delete missing column", - IllegalArgumentException.class, - "missing column: col", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.deleteColumn("col"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.deleteColumn("col"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete missing column: col"); } @Test public void testAddDeleteConflict() { - AssertHelpers.assertThrows( - "Should reject add then delete", - IllegalArgumentException.class, - "missing column: col", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.addColumn("col", Types.IntegerType.get()).deleteColumn("col"); - }); - AssertHelpers.assertThrows( - "Should reject add then delete", - IllegalArgumentException.class, - "column that has additions: preferences", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update - .addColumn("preferences", "feature3", Types.IntegerType.get()) - .deleteColumn("preferences"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.addColumn("col", Types.IntegerType.get()).deleteColumn("col"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete missing column: col"); + + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update + .addColumn("preferences", "feature3", Types.IntegerType.get()) + .deleteColumn("preferences"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete a column that has additions: preferences"); } @Test public void testRenameMissingColumn() { - AssertHelpers.assertThrows( - "Should reject rename missing column", - IllegalArgumentException.class, - "missing column: col", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.renameColumn("col", "fail"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.renameColumn("col", "fail"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot rename missing column: col"); } @Test public void testRenameDeleteConflict() { - AssertHelpers.assertThrows( - "Should reject rename then delete", - IllegalArgumentException.class, - "column that has updates: id", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.renameColumn("id", "col").deleteColumn("id"); - }); - AssertHelpers.assertThrows( - "Should reject rename then delete", - IllegalArgumentException.class, - "missing column: col", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.renameColumn("id", "col").deleteColumn("col"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.renameColumn("id", "col").deleteColumn("id"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete a column that has updates: id"); + + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.renameColumn("id", "col").deleteColumn("col"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete missing column: col"); } @Test public void testDeleteRenameConflict() { - AssertHelpers.assertThrows( - "Should reject delete then rename", - IllegalArgumentException.class, - "column that will be deleted: id", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.deleteColumn("id").renameColumn("id", "identifier"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.deleteColumn("id").renameColumn("id", "identifier"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot rename a column that will be deleted: id"); } @Test public void testUpdateMissingColumn() { - AssertHelpers.assertThrows( - "Should reject rename missing column", - IllegalArgumentException.class, - "missing column: col", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.updateColumn("col", Types.DateType.get()); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.updateColumn("col", Types.DateType.get()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot update missing column: col"); } @Test public void testUpdateDeleteConflict() { - AssertHelpers.assertThrows( - "Should reject update then delete", - IllegalArgumentException.class, - "column that has updates: id", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.updateColumn("id", Types.LongType.get()).deleteColumn("id"); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.updateColumn("id", Types.LongType.get()).deleteColumn("id"); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete a column that has updates: id"); } @Test public void testDeleteUpdateConflict() { - AssertHelpers.assertThrows( - "Should reject delete then update", - IllegalArgumentException.class, - "column that will be deleted: id", - () -> { - UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); - update.deleteColumn("id").updateColumn("id", Types.LongType.get()); - }); + Assertions.assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.deleteColumn("id").updateColumn("id", Types.LongType.get()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot update a column that will be deleted: id"); } @Test public void testDeleteMapKey() { - AssertHelpers.assertThrows( - "Should reject delete map key", - IllegalArgumentException.class, - "Cannot delete map keys", - () -> { - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).deleteColumn("locations.key").apply(); - }); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .deleteColumn("locations.key") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot delete map keys"); } @Test public void testAddFieldToMapKey() { - AssertHelpers.assertThrows( - "Should reject add sub-field to map key", - IllegalArgumentException.class, - "Cannot add fields to map keys", - () -> { - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) - .addColumn("locations.key", "address_line_2", Types.StringType.get()) - .apply(); - }); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .addColumn("locations.key", "address_line_2", Types.StringType.get()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add fields to map keys"); } @Test public void testAlterMapKey() { - AssertHelpers.assertThrows( - "Should reject alter sub-field of map key", - IllegalArgumentException.class, - "Cannot alter map keys", - () -> { - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) - .updateColumn("locations.key.zip", Types.LongType.get()) - .apply(); - }); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .updateColumn("locations.key.zip", Types.LongType.get()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot alter map keys"); } @Test @@ -996,40 +978,36 @@ public void testUpdateMapKey() { 1, "m", Types.MapType.ofOptional(2, 3, Types.IntegerType.get(), Types.DoubleType.get()))); - AssertHelpers.assertThrows( - "Should reject update map key", - IllegalArgumentException.class, - "Cannot update map keys", - () -> { - new SchemaUpdate(schema, 3).updateColumn("m.key", Types.LongType.get()).apply(); - }); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 3).updateColumn("m.key", Types.LongType.get()).apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot update map keys: map"); } @Test public void testUpdateAddedColumnDoc() { Schema schema = new Schema(required(1, "i", Types.IntegerType.get())); - AssertHelpers.assertThrows( - "Should reject add and update doc", - IllegalArgumentException.class, - "Cannot update missing column", - () -> { - new SchemaUpdate(schema, 3) - .addColumn("value", Types.LongType.get()) - .updateColumnDoc("value", "a value") - .apply(); - }); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 3) + .addColumn("value", Types.LongType.get()) + .updateColumnDoc("value", "a value") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot update missing column: value"); } @Test public void testUpdateDeletedColumnDoc() { Schema schema = new Schema(required(1, "i", Types.IntegerType.get())); - AssertHelpers.assertThrows( - "Should reject add and update doc", - IllegalArgumentException.class, - "Cannot update a column that will be deleted", - () -> { - new SchemaUpdate(schema, 3).deleteColumn("i").updateColumnDoc("i", "a value").apply(); - }); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 3) + .deleteColumn("i") + .updateColumnDoc("i", "a value") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot update a column that will be deleted: i"); } @Test @@ -1454,17 +1432,13 @@ public void testMoveSelfReferenceFails() { new Schema( required(1, "id", Types.LongType.get()), required(2, "data", Types.StringType.get())); - AssertHelpers.assertThrows( - "Should fail move for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move id before itself", - () -> new SchemaUpdate(schema, 2).moveBefore("id", "id").apply()); + Assertions.assertThatThrownBy(() -> new SchemaUpdate(schema, 2).moveBefore("id", "id").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move id before itself"); - AssertHelpers.assertThrows( - "Should fail move for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move id after itself", - () -> new SchemaUpdate(schema, 2).moveAfter("id", "id").apply()); + Assertions.assertThatThrownBy(() -> new SchemaUpdate(schema, 2).moveAfter("id", "id").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move id after itself"); } @Test @@ -1473,23 +1447,19 @@ public void testMoveMissingColumnFails() { new Schema( required(1, "id", Types.LongType.get()), required(2, "data", Types.StringType.get())); - AssertHelpers.assertThrows( - "Should fail move for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move missing column", - () -> new SchemaUpdate(schema, 2).moveFirst("items").apply()); + Assertions.assertThatThrownBy(() -> new SchemaUpdate(schema, 2).moveFirst("items").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: items"); - AssertHelpers.assertThrows( - "Should fail move for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move missing column", - () -> new SchemaUpdate(schema, 2).moveBefore("items", "id").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 2).moveBefore("items", "id").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: items"); - AssertHelpers.assertThrows( - "Should fail move for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move missing column", - () -> new SchemaUpdate(schema, 2).moveAfter("items", "data").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 2).moveAfter("items", "data").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: items"); } @Test @@ -1498,35 +1468,32 @@ public void testMoveBeforeAddFails() { new Schema( required(1, "id", Types.LongType.get()), required(2, "data", Types.StringType.get())); - AssertHelpers.assertThrows( - "Should fail move for a field that has not been added yet", - IllegalArgumentException.class, - "Cannot move missing column", - () -> - new SchemaUpdate(schema, 2) - .moveFirst("ts") - .addColumn("ts", Types.TimestampType.withZone()) - .apply()); - - AssertHelpers.assertThrows( - "Should fail move for a field that has not been added yet", - IllegalArgumentException.class, - "Cannot move missing column", - () -> - new SchemaUpdate(schema, 2) - .moveBefore("ts", "id") - .addColumn("ts", Types.TimestampType.withZone()) - .apply()); - - AssertHelpers.assertThrows( - "Should fail move for a field that has not been added yet", - IllegalArgumentException.class, - "Cannot move missing column", - () -> - new SchemaUpdate(schema, 2) - .moveAfter("ts", "data") - .addColumn("ts", Types.TimestampType.withZone()) - .apply()); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 2) + .moveFirst("ts") + .addColumn("ts", Types.TimestampType.withZone()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: ts"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 2) + .moveBefore("ts", "id") + .addColumn("ts", Types.TimestampType.withZone()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: ts"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schema, 2) + .moveAfter("ts", "data") + .addColumn("ts", Types.TimestampType.withZone()) + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move missing column: ts"); } @Test @@ -1535,17 +1502,15 @@ public void testMoveMissingReferenceColumnFails() { new Schema( required(1, "id", Types.LongType.get()), required(2, "data", Types.StringType.get())); - AssertHelpers.assertThrows( - "Should fail move before a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move id before missing column", - () -> new SchemaUpdate(schema, 2).moveBefore("id", "items").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 2).moveBefore("id", "items").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move id before missing column: items"); - AssertHelpers.assertThrows( - "Should fail move after for a field that is not in the schema", - IllegalArgumentException.class, - "Cannot move data after missing column", - () -> new SchemaUpdate(schema, 2).moveAfter("data", "items").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 2).moveAfter("data", "items").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move data after missing column: items"); } @Test @@ -1559,11 +1524,10 @@ public void testMovePrimitiveMapKeyFails() { "map", Types.MapType.ofRequired(4, 5, Types.StringType.get(), Types.StringType.get()))); - AssertHelpers.assertThrows( - "Should fail move for map key", - IllegalArgumentException.class, - "Cannot move fields in non-struct type", - () -> new SchemaUpdate(schema, 5).moveBefore("map.key", "map.value").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 5).moveBefore("map.key", "map.value").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: map"); } @Test @@ -1577,11 +1541,10 @@ public void testMovePrimitiveMapValueFails() { "map", Types.MapType.ofRequired(4, 5, Types.StringType.get(), Types.StructType.of()))); - AssertHelpers.assertThrows( - "Should fail move for map value", - IllegalArgumentException.class, - "Cannot move fields in non-struct type", - () -> new SchemaUpdate(schema, 5).moveBefore("map.value", "map.key").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 5).moveBefore("map.value", "map.key").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: map>"); } @Test @@ -1592,11 +1555,10 @@ public void testMovePrimitiveListElementFails() { required(2, "data", Types.StringType.get()), optional(3, "list", Types.ListType.ofRequired(4, Types.StringType.get()))); - AssertHelpers.assertThrows( - "Should fail move for list element", - IllegalArgumentException.class, - "Cannot move fields in non-struct type", - () -> new SchemaUpdate(schema, 4).moveBefore("list.element", "list").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 4).moveBefore("list.element", "list").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move fields in non-struct type: list"); } @Test @@ -1612,11 +1574,10 @@ public void testMoveTopLevelBetweenStructsFails() { required(4, "x", Types.IntegerType.get()), required(5, "y", Types.IntegerType.get())))); - AssertHelpers.assertThrows( - "Should fail move between separate structs", - IllegalArgumentException.class, - "Cannot move field a to a different struct", - () -> new SchemaUpdate(schema, 5).moveBefore("a", "struct.x").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 5).moveBefore("a", "struct.x").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move field a to a different struct"); } @Test @@ -1636,11 +1597,10 @@ public void testMoveBetweenStructsFails() { required(5, "x", Types.IntegerType.get()), required(6, "y", Types.IntegerType.get())))); - AssertHelpers.assertThrows( - "Should fail move between separate structs", - IllegalArgumentException.class, - "Cannot move field s2.x to a different struct", - () -> new SchemaUpdate(schema, 6).moveBefore("s2.x", "s1.a").apply()); + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(schema, 6).moveBefore("s2.x", "s1.a").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot move field s2.x to a different struct"); } @Test @@ -1800,69 +1760,71 @@ public void testSetIdentifierFieldsFails() { required(2, "float", Types.FloatType.get()), required(3, "double", Types.DoubleType.get())); - AssertHelpers.assertThrows( - "Creating schema with nonexistent identifier fieldId should fail", - IllegalArgumentException.class, - "Cannot add fieldId 999 as an identifier field: field does not exist", - () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(999))); - - AssertHelpers.assertThrows( - "Creating schema with optional identifier field should fail", - IllegalArgumentException.class, - "Cannot add field id as an identifier field: not a required field", - () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(1))); - - AssertHelpers.assertThrows( - "Creating schema with float identifier field should fail", - IllegalArgumentException.class, - "Cannot add field float as an identifier field: must not be float or double field", - () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(2))); - - AssertHelpers.assertThrows( - "Creating schema with double identifier field should fail", - IllegalArgumentException.class, - "Cannot add field double as an identifier field: must not be float or double field", - () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(3))); - - AssertHelpers.assertThrows( - "add a field with name not exist should fail", - IllegalArgumentException.class, - "not found in current schema or added columns", - () -> - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("unknown").apply()); - - AssertHelpers.assertThrows( - "add a field of non-primitive type should fail", - IllegalArgumentException.class, - "not a primitive type field", - () -> - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) - .setIdentifierFields("locations") - .apply()); - - AssertHelpers.assertThrows( - "add an optional field should fail", - IllegalArgumentException.class, - "not a required field", - () -> new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("data").apply()); - - AssertHelpers.assertThrows( - "add a map key nested field should fail", - IllegalArgumentException.class, - "must not be nested in " + SCHEMA.findField("locations"), - () -> - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) - .setIdentifierFields("locations.key.zip") - .apply()); - - AssertHelpers.assertThrows( - "add a nested field in list should fail", - IllegalArgumentException.class, - "must not be nested in " + SCHEMA.findField("points"), - () -> - new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) - .setIdentifierFields("points.element.x") - .apply()); + Assertions.assertThatThrownBy( + () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(999))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add fieldId 999 as an identifier field: field does not exist"); + + Assertions.assertThatThrownBy( + () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(1))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add field id as an identifier field: not a required field"); + + Assertions.assertThatThrownBy( + () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(2))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field float as an identifier field: must not be float or double field"); + + Assertions.assertThatThrownBy( + () -> new Schema(testSchema.asStruct().fields(), ImmutableSet.of(3))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field double as an identifier field: must not be float or double field"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("unknown") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field unknown as an identifier field: not found in current schema or added columns"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("locations") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field locations as an identifier field: not a primitive type field"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("data").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot add field data as an identifier field: not a required field"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("locations.key.zip") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field zip as an identifier field: must not be nested in " + + SCHEMA.findField("locations")); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) + .setIdentifierFields("points.element.x") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field x as an identifier field: must not be nested in " + + SCHEMA.findField("points")); Schema newSchema = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID) @@ -1903,53 +1865,57 @@ public void testSetIdentifierFieldsFails() { int lastColId = SCHEMA_LAST_COLUMN_ID + 15; - AssertHelpers.assertThrows( - "add a nested field in list should fail", - IllegalArgumentException.class, - "must not be nested in " + newSchema.findField("required_list"), - () -> - new SchemaUpdate(newSchema, lastColId) - .setIdentifierFields("required_list.element.x") - .apply()); - - AssertHelpers.assertThrows( - "add a double field should fail", - IllegalArgumentException.class, - "must not be float or double field", - () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_double").apply()); - - AssertHelpers.assertThrows( - "add a float field should fail", - IllegalArgumentException.class, - "must not be float or double field", - () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_float").apply()); - - AssertHelpers.assertThrows( - "add a map value nested field should fail", - IllegalArgumentException.class, - "must not be nested in " + newSchema.findField("new_map"), - () -> - new SchemaUpdate(newSchema, lastColId) - .setIdentifierFields("new_map.value.val_col") - .apply()); - - AssertHelpers.assertThrows( - "add a nested field in struct of a list should fail", - IllegalArgumentException.class, - "must not be nested in " + newSchema.findField("new.fields"), - () -> - new SchemaUpdate(newSchema, lastColId) - .setIdentifierFields("new.fields.element.nested") - .apply()); - - AssertHelpers.assertThrows( - "add a nested field in an optional struct should fail", - IllegalArgumentException.class, - "must not be nested in an optional field " + newSchema.findField("preferences"), - () -> - new SchemaUpdate(newSchema, lastColId) - .setIdentifierFields("preferences.feature1") - .apply()); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("required_list.element.x") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field x as an identifier field: must not be nested in " + + newSchema.findField("required_list")); + + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_double").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field col_double as an identifier field: must not be float or double field"); + + Assertions.assertThatThrownBy( + () -> new SchemaUpdate(newSchema, lastColId).setIdentifierFields("col_float").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field col_float as an identifier field: must not be float or double field"); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("new_map.value.val_col") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field val_col as an identifier field: must not be nested in " + + newSchema.findField("new_map")); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("new.fields.element.nested") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith( + "Cannot add field nested as an identifier field: must not be nested in " + + newSchema.findField("new.fields")); + + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, lastColId) + .setIdentifierFields("preferences.feature1") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot add field feature1 as an identifier field: must not be nested in an optional field " + + newSchema.findField("preferences")); } @Test @@ -1981,15 +1947,14 @@ public void testDeleteIdentifierFieldColumnsFails() { Schema schemaWithIdentifierFields = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID).setIdentifierFields("id").apply(); - AssertHelpers.assertThrows( - "delete an identifier column without setting identifier fields should fail", - IllegalArgumentException.class, - "Cannot delete identifier field 1: id: required int. To force deletion, " - + "also call setIdentifierFields to update identifier fields.", - () -> - new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) - .deleteColumn("id") - .apply()); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(schemaWithIdentifierFields, SCHEMA_LAST_COLUMN_ID) + .deleteColumn("id") + .apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot delete identifier field 1: id: required int. To force deletion, also call setIdentifierFields to update identifier fields."); } @Test @@ -2005,12 +1970,13 @@ public void testDeleteContainingNestedIdentifierFieldColumnsFails() { .setIdentifierFields("out.nested") .apply(); - AssertHelpers.assertThrows( - "delete a struct with a nested identifier column without setting identifier fields should fail", - IllegalArgumentException.class, - "Cannot delete field 24: out: required struct<25: nested: required string> " - + "as it will delete nested identifier field 25: nested: required string", - () -> new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID + 2).deleteColumn("out").apply()); + Assertions.assertThatThrownBy( + () -> + new SchemaUpdate(newSchema, SCHEMA_LAST_COLUMN_ID + 2).deleteColumn("out").apply()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot delete field 24: out: required struct<25: nested: required string> " + + "as it will delete nested identifier field 25: nested: required string"); } @Test diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java index 5c248bb59f2d..ba4e85f494ef 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotManager.java @@ -20,6 +20,7 @@ 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; @@ -115,11 +116,10 @@ public void testCherryPickDynamicOverwriteConflict() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - AssertHelpers.assertThrows( - "Should reject partition replacement when a partition has been modified", - ValidationException.class, - "Cannot cherry-pick replace partitions with changed partition", - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()); + Assertions.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", @@ -147,11 +147,10 @@ public void testCherryPickDynamicOverwriteDeleteConflict() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - AssertHelpers.assertThrows( - "Should reject partition replacement when a partition has been modified", - ValidationException.class, - "Missing required files to delete", - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()); + Assertions.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", @@ -178,11 +177,11 @@ public void testCherryPickFromBranch() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - AssertHelpers.assertThrows( - "Should reject partition replacement when a partition has been modified", - ValidationException.class, - "Cannot cherry-pick overwrite not based on an ancestor of the current state", - () -> table.manageSnapshots().cherrypick(replaceSnapshotId).commit()); + Assertions.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", @@ -207,11 +206,10 @@ public void testCherryPickOverwrite() { long lastSnapshotId = table.currentSnapshot().snapshotId(); // pick the snapshot into the current state - AssertHelpers.assertThrows( - "Should reject partition replacement when a partition has been modified", - ValidationException.class, - "not append, dynamic overwrite, or fast-forward", - () -> table.manageSnapshots().cherrypick(staged.snapshotId()).commit()); + Assertions.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", @@ -238,23 +236,21 @@ public void testCreateBranchFailsWhenRefAlreadyExists() { long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); // Trying to create a branch with an existing name should fail - AssertHelpers.assertThrows( - "Creating branch which already exists should fail", - IllegalArgumentException.class, - "Ref branch1 already exists", - () -> table.manageSnapshots().createBranch("branch1", snapshotId).commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().createBranch("branch1", snapshotId).commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Ref branch1 already exists"); // Trying to create another branch within the same chain - AssertHelpers.assertThrows( - "Creating branch which already exists should fail", - IllegalArgumentException.class, - "Ref branch2 already exists", - () -> - table - .manageSnapshots() - .createBranch("branch2", snapshotId) - .createBranch("branch2", snapshotId) - .commit()); + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .createBranch("branch2", snapshotId) + .createBranch("branch2", snapshotId) + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Ref branch2 already exists"); } @Test @@ -276,23 +272,21 @@ public void testCreateTagFailsWhenRefAlreadyExists() { table.manageSnapshots().createTag("tag1", snapshotId).commit(); // Trying to create a tag with an existing name should fail - AssertHelpers.assertThrows( - "Creating tag which already exists should fail", - IllegalArgumentException.class, - "Ref tag1 already exists", - () -> table.manageSnapshots().createTag("tag1", snapshotId).commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().createTag("tag1", snapshotId).commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Ref tag1 already exists"); // Trying to create another tag within the same chain - AssertHelpers.assertThrows( - "Creating branch which already exists should fail", - IllegalArgumentException.class, - "Ref tag2 already exists", - () -> - table - .manageSnapshots() - .createTag("tag2", snapshotId) - .createTag("tag2", snapshotId) - .commit()); + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .createTag("tag2", snapshotId) + .createTag("tag2", snapshotId) + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Ref tag2 already exists"); } @Test @@ -315,20 +309,18 @@ public void testRemoveBranch() { @Test public void testRemovingNonExistingBranchFails() { - AssertHelpers.assertThrows( - "Trying to remove non-existent branch should fail", - IllegalArgumentException.class, - "Branch does not exist: non-existing", - () -> table.manageSnapshots().removeBranch("non-existing").commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().removeBranch("non-existing").commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Branch does not exist: non-existing"); } @Test public void testRemovingMainBranchFails() { - AssertHelpers.assertThrows( - "Removing main should fail", - IllegalArgumentException.class, - "Cannot remove main branch", - () -> table.manageSnapshots().removeBranch(SnapshotRef.MAIN_BRANCH).commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().removeBranch(SnapshotRef.MAIN_BRANCH).commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot remove main branch"); } @Test @@ -350,11 +342,9 @@ public void testRemoveTag() { @Test public void testRemovingNonExistingTagFails() { - AssertHelpers.assertThrows( - "Removing a non-existing tag should fail", - IllegalArgumentException.class, - "Tag does not exist: non-existing", - () -> table.manageSnapshots().removeTag("non-existing").commit()); + Assertions.assertThatThrownBy(() -> table.manageSnapshots().removeTag("non-existing").commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tag does not exist: non-existing"); } @Test @@ -372,11 +362,10 @@ public void testReplaceBranch() { @Test public void testReplaceBranchNonExistingTargetBranchFails() { - AssertHelpers.assertThrows( - "Replacing a non-existing branch should fail", - IllegalArgumentException.class, - "Target branch does not exist: non-existing", - () -> table.manageSnapshots().replaceBranch("non-existing", "other-branch").commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().replaceBranch("non-existing", "other-branch").commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Target branch does not exist: non-existing"); } @Test @@ -384,11 +373,10 @@ public void testReplaceBranchNonExistingSourceFails() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); table.manageSnapshots().createBranch("branch1", snapshotId).commit(); - AssertHelpers.assertThrows( - "Replacing where the source ref does not exist should fail", - IllegalArgumentException.class, - "Ref does not exist: non-existing", - () -> table.manageSnapshots().replaceBranch("branch1", "non-existing").commit()); + Assertions.assertThatThrownBy( + () -> table.manageSnapshots().replaceBranch("branch1", "non-existing").commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Ref does not exist: non-existing"); } @Test @@ -422,12 +410,15 @@ public void testFastForwardWhenTargetIsNotAncestorFails() { final String newBranch = "new-branch-at-staged-snapshot"; table.manageSnapshots().createBranch(newBranch, snapshot).commit(); - AssertHelpers.assertThrows( - "Fast-forward should fail if target is not an ancestor of the source", - IllegalArgumentException.class, - "Cannot fast-forward: main is not an ancestor of new-branch-at-staged-snapshot", - () -> - table.manageSnapshots().fastForwardBranch(SnapshotRef.MAIN_BRANCH, newBranch).commit()); + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .fastForwardBranch(SnapshotRef.MAIN_BRANCH, newBranch) + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot fast-forward: main is not an ancestor of new-branch-at-staged-snapshot"); } @Test @@ -473,26 +464,25 @@ public void testSettingBranchRetentionOnTagFails() { table.newAppend().appendFile(FILE_A).commit(); long snapshotId = table.currentSnapshot().snapshotId(); - AssertHelpers.assertThrows( - "Setting minSnapshotsToKeep should fail for tags", - IllegalArgumentException.class, - "Tags do not support setting minSnapshotsToKeep", - () -> - table - .manageSnapshots() - .createTag("tag1", snapshotId) - .setMinSnapshotsToKeep("tag1", 10) - .commit()); - AssertHelpers.assertThrows( - "Setting maxSnapshotAgeMs should fail for tags", - IllegalArgumentException.class, - "Tags do not support setting maxSnapshotAgeMs", - () -> - table - .manageSnapshots() - .createTag("tag1", snapshotId) - .setMaxSnapshotAgeMs("tag1", 10) - .commit()); + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .createTag("tag1", snapshotId) + .setMinSnapshotsToKeep("tag1", 10) + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tags do not support setting minSnapshotsToKeep"); + + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .createTag("tag1", snapshotId) + .setMaxSnapshotAgeMs("tag1", 10) + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Tags do not support setting maxSnapshotAgeMs"); } @Test @@ -558,21 +548,23 @@ public void testRenameBranch() { @Test public void testFailRenamingMainBranch() { - AssertHelpers.assertThrows( - "Renaming main branch should fail", - IllegalArgumentException.class, - "Cannot rename main branch", - () -> - table.manageSnapshots().renameBranch(SnapshotRef.MAIN_BRANCH, "some-branch").commit()); + Assertions.assertThatThrownBy( + () -> + table + .manageSnapshots() + .renameBranch(SnapshotRef.MAIN_BRANCH, "some-branch") + .commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot rename main branch"); } @Test public void testRenamingNonExistingBranchFails() { - AssertHelpers.assertThrows( - "Renaming non-existent branch should fail", - IllegalArgumentException.class, - "Branch does not exist: some-missing-branch", - () -> table.manageSnapshots().renameBranch("some-missing-branch", "some-branch").commit()); + Assertions.assertThatThrownBy( + () -> + table.manageSnapshots().renameBranch("some-missing-branch", "some-branch").commit()) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Branch does not exist: some-missing-branch"); } @Test diff --git a/core/src/test/java/org/apache/iceberg/TestUpdatePartitionSpec.java b/core/src/test/java/org/apache/iceberg/TestUpdatePartitionSpec.java index 97210e51b5ba..926272e60f98 100644 --- a/core/src/test/java/org/apache/iceberg/TestUpdatePartitionSpec.java +++ b/core/src/test/java/org/apache/iceberg/TestUpdatePartitionSpec.java @@ -29,6 +29,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.transforms.Transforms; 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; @@ -419,71 +420,65 @@ public void testAddDeletedName() { @Test public void testRemoveNewlyAddedFieldByName() { - AssertHelpers.assertThrows( - "Should fail trying to remove unknown field", - IllegalArgumentException.class, - "Cannot delete newly added field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("prefix", truncate("data", 4)) - .removeField("prefix")); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("prefix", truncate("data", 4)) + .removeField("prefix")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot delete newly added field"); } @Test public void testRemoveNewlyAddedFieldByTransform() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot delete newly added field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("prefix", truncate("data", 4)) - .removeField(truncate("data", 4))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("prefix", truncate("data", 4)) + .removeField(truncate("data", 4))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot delete newly added field"); } @Test public void testAddAlreadyAddedFieldByTransform() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("prefix", truncate("data", 4)) - .addField(truncate("data", 4))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("prefix", truncate("data", 4)) + .addField(truncate("data", 4))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testAddAlreadyAddedFieldByName() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("prefix", truncate("data", 4)) - .addField("prefix", truncate("data", 6))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("prefix", truncate("data", 4)) + .addField("prefix", truncate("data", 6))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testAddRedundantTimePartition() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add redundant partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, UNPARTITIONED) - .addField(day("ts")) - .addField(hour("ts"))); // conflicts with hour - - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add redundant partition", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField(hour("ts")) // does not conflict with day because day already exists - .addField(month("ts"))); // conflicts with hour + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, UNPARTITIONED) + .addField(day("ts")) + .addField(hour("ts"))) // conflicts with hour + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add redundant partition field"); + + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField(hour("ts")) // does not conflict with day because day already exists + .addField(month("ts"))) // conflicts with hour + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add redundant partition"); } @Test @@ -532,106 +527,99 @@ public void testGenerateNewSpecAddDeletedSameFieldWithDifferentName() { @Test public void testAddDuplicateByName() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField("category")); + Assertions.assertThatThrownBy( + () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField("category")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testAddDuplicateByRef() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField(ref("category"))); + Assertions.assertThatThrownBy( + () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField(ref("category"))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testAddDuplicateTransform() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField(bucket("id", 16))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).addField(bucket("id", 16))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testAddNamedDuplicate() { - AssertHelpers.assertThrows( - "Should fail adding a duplicate field", - IllegalArgumentException.class, - "Cannot add duplicate partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("b16", bucket("id", 16))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("b16", bucket("id", 16))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add duplicate partition field"); } @Test public void testRemoveUnknownFieldByName() { - AssertHelpers.assertThrows( - "Should fail trying to remove unknown field", - IllegalArgumentException.class, - "Cannot find partition field to remove", - () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).removeField("moon")); + Assertions.assertThatThrownBy( + () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).removeField("moon")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot find partition field to remove"); } @Test public void testRemoveUnknownFieldByEquivalent() { - AssertHelpers.assertThrows( - "Should fail trying to remove unknown field", - IllegalArgumentException.class, - "Cannot find partition field to remove", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .removeField(hour("ts")) // day(ts) exists - ); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .removeField(hour("ts")) // day(ts) exists + ) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot find partition field to remove"); } @Test public void testRenameUnknownField() { - AssertHelpers.assertThrows( - "Should fail trying to rename an unknown field", - IllegalArgumentException.class, - "Cannot find partition field to rename", - () -> new BaseUpdatePartitionSpec(formatVersion, PARTITIONED).renameField("shake", "seal")); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .renameField("shake", "seal")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot find partition field to rename: shake"); } @Test public void testRenameAfterAdd() { - AssertHelpers.assertThrows( - "Should fail trying to rename an added field", - IllegalArgumentException.class, - "Cannot rename newly added partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .addField("data_trunc", truncate("data", 4)) - .renameField("data_trunc", "prefix")); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .addField("data_trunc", truncate("data", 4)) + .renameField("data_trunc", "prefix")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot rename newly added partition field: data_trunc"); } @Test public void testDeleteAndRename() { - AssertHelpers.assertThrows( - "Should fail trying to rename a deleted field", - IllegalArgumentException.class, - "Cannot rename and delete partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .renameField("shard", "id_bucket") - .removeField(bucket("id", 16))); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .renameField("shard", "id_bucket") + .removeField(bucket("id", 16))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot rename and delete partition field: shard"); } @Test public void testRenameAndDelete() { - AssertHelpers.assertThrows( - "Should fail trying to delete a renamed field", - IllegalArgumentException.class, - "Cannot delete and rename partition field", - () -> - new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) - .removeField(bucket("id", 16)) - .renameField("shard", "id_bucket")); + Assertions.assertThatThrownBy( + () -> + new BaseUpdatePartitionSpec(formatVersion, PARTITIONED) + .removeField(bucket("id", 16)) + .renameField("shard", "id_bucket")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot delete and rename partition field: shard"); } @Test