From b61bced6f53deea4e35200f7df6a2d587c71af26 Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Sat, 15 Jun 2024 14:14:29 +0200 Subject: [PATCH 1/3] Replace usages of deprecated AssertHelpers --- .../extensions/TestAddFilesProcedure.java | 114 ++- .../extensions/TestAlterTableSchema.java | 55 +- .../extensions/TestAncestorsOfProcedure.java | 34 +- .../spark/extensions/TestBranchDDL.java | 103 +- .../extensions/TestCallStatementParser.java | 11 +- .../spark/extensions/TestChangelogTable.java | 10 +- .../TestCherrypickSnapshotProcedure.java | 69 +- .../extensions/TestConflictValidation.java | 291 +++--- .../iceberg/spark/extensions/TestDelete.java | 20 +- .../TestExpireSnapshotsProcedure.java | 143 ++- .../iceberg/spark/extensions/TestMerge.java | 958 +++++++++--------- .../extensions/TestMergeOnReadDelete.java | 17 +- .../extensions/TestMigrateTableProcedure.java | 50 +- .../TestPublishChangesProcedure.java | 58 +- .../TestRemoveOrphanFilesProcedure.java | 169 ++- .../spark/extensions/TestReplaceBranch.java | 31 +- .../TestRequiredDistributionAndOrdering.java | 30 +- .../TestRewriteDataFilesProcedure.java | 222 ++-- .../TestRewriteManifestsProcedure.java | 70 +- .../TestRollbackToSnapshotProcedure.java | 99 +- .../TestRollbackToTimestampProcedure.java | 97 +- .../TestSetCurrentSnapshotProcedure.java | 111 +- .../TestSnapshotTableProcedure.java | 63 +- .../iceberg/spark/extensions/TestTagDDL.java | 129 ++- .../iceberg/spark/extensions/TestUpdate.java | 145 ++- .../spark/extensions/TestWriteAborts.java | 36 +- .../iceberg/spark/TestFunctionCatalog.java | 42 +- .../TestDeleteReachableFilesAction.java | 22 +- .../actions/TestExpireSnapshotsAction.java | 23 +- .../actions/TestRemoveOrphanFilesAction.java | 58 +- .../actions/TestRewriteDataFilesAction.java | 111 +- .../actions/TestRewriteManifestsAction.java | 13 +- .../TestParquetVectorizedReads.java | 39 +- .../spark/source/TestDataFrameWriterV2.java | 50 +- .../spark/source/TestDataSourceOptions.java | 83 +- .../source/TestForwardCompatibility.java | 31 +- .../source/TestIcebergSourceTablesBase.java | 23 +- ...tMetadataTablesWithPartitionEvolution.java | 11 +- .../TestRequiredDistributionAndOrdering.java | 52 +- .../spark/source/TestSparkDataWrite.java | 30 +- .../source/TestSparkMetadataColumns.java | 20 +- .../source/TestStructuredStreamingRead3.java | 23 +- .../source/TestTimestampWithoutZone.java | 42 +- .../spark/source/TestWriteMetricsConfig.java | 11 +- .../iceberg/spark/sql/TestAlterTable.java | 38 +- .../iceberg/spark/sql/TestCreateTable.java | 28 +- .../iceberg/spark/sql/TestDeleteFrom.java | 12 +- .../iceberg/spark/sql/TestDropTable.java | 12 +- .../iceberg/spark/sql/TestNamespaceSQL.java | 11 +- .../apache/iceberg/spark/sql/TestSelect.java | 104 +- .../spark/sql/TestSparkBucketFunction.java | 181 ++-- .../spark/sql/TestSparkDaysFunction.java | 42 +- .../spark/sql/TestSparkHoursFunction.java | 42 +- .../spark/sql/TestSparkMonthsFunction.java | 41 +- .../spark/sql/TestSparkTruncateFunction.java | 174 ++-- .../spark/sql/TestSparkYearsFunction.java | 41 +- .../spark/sql/TestTimestampWithoutZone.java | 19 +- .../sql/UnpartitionedWritesTestBase.java | 29 +- 58 files changed, 2298 insertions(+), 2295 deletions(-) diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java index a48deae7421f..aab2ae65d010 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAddFilesProcedure.java @@ -34,7 +34,6 @@ import org.apache.avro.generic.GenericDatumWriter; import org.apache.avro.generic.GenericRecord; import org.apache.avro.io.DatumWriter; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.TableProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -48,6 +47,7 @@ import org.apache.spark.sql.types.Metadata; import org.apache.spark.sql.types.StructField; import org.apache.spark.sql.types.StructType; +import org.assertj.core.api.Assertions; import org.joda.time.DateTime; import org.junit.After; import org.junit.Assert; @@ -688,23 +688,23 @@ public void invalidDataImport() { createIcebergTable("id Integer, name String, dept String, subdept String"); - AssertHelpers.assertThrows( - "Should forbid adding of partitioned data to unpartitioned table", - IllegalArgumentException.class, - "Cannot use partition filter with an unpartitioned table", - () -> - scalarSql( - "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('id', 1))", - catalogName, tableName, fileTableDir.getAbsolutePath())); - - AssertHelpers.assertThrows( - "Should forbid adding of partitioned data to unpartitioned table", - IllegalArgumentException.class, - "Cannot add partitioned files to an unpartitioned table", - () -> - scalarSql( - "CALL %s.system.add_files('%s', '`parquet`.`%s`')", - catalogName, tableName, fileTableDir.getAbsolutePath())); + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('id', 1))", + catalogName, tableName, fileTableDir.getAbsolutePath())) + .as("Should forbid adding of partitioned data to unpartitioned table") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot use partition filter with an unpartitioned table"); + + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files('%s', '`parquet`.`%s`')", + catalogName, tableName, fileTableDir.getAbsolutePath())) + .as("Should forbid adding of partitioned data to unpartitioned table") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot add partitioned files to an unpartitioned table"); } @Test @@ -714,23 +714,24 @@ public void invalidDataImportPartitioned() { createIcebergTable( "id Integer, name String, dept String, subdept String", "PARTITIONED BY (id)"); - AssertHelpers.assertThrows( - "Should forbid adding with a mismatching partition spec", - IllegalArgumentException.class, - "is greater than the number of partitioned columns", - () -> - scalarSql( - "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('x', '1', 'y', '2'))", - catalogName, tableName, fileTableDir.getAbsolutePath())); - - AssertHelpers.assertThrows( - "Should forbid adding with partition spec with incorrect columns", - IllegalArgumentException.class, - "specified partition filter refers to columns that are not partitioned", - () -> - scalarSql( - "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('dept', '2'))", - catalogName, tableName, fileTableDir.getAbsolutePath())); + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('x', '1', 'y', '2'))", + catalogName, tableName, fileTableDir.getAbsolutePath())) + .as("Should forbid adding with a mismatching partition spec") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("is greater than the number of partitioned columns"); + + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files('%s', '`parquet`.`%s`', map('dept', '2'))", + catalogName, tableName, fileTableDir.getAbsolutePath())) + .as("Should forbid adding with partition spec with incorrect columns") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "specified partition filter refers to columns that are not partitioned"); } @Test @@ -782,18 +783,19 @@ public void duplicateDataPartitioned() { + "partition_filter => map('id', 1))", catalogName, tableName, sourceTableName); - AssertHelpers.assertThrows( - "Should not allow adding duplicate files", - IllegalStateException.class, - "Cannot complete import because data files to be imported already" - + " exist within the target table", - () -> - scalarSql( - "CALL %s.system.add_files(" - + "table => '%s', " - + "source_table => '%s', " - + "partition_filter => map('id', 1))", - catalogName, tableName, sourceTableName)); + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files(" + + "table => '%s', " + + "source_table => '%s', " + + "partition_filter => map('id', 1))", + catalogName, tableName, sourceTableName)) + .as("Should not allow adding duplicate files") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining( + "Cannot complete import because data files to be imported already" + + " exist within the target table"); } @Test @@ -841,14 +843,16 @@ public void duplicateDataUnpartitioned() { sql("CALL %s.system.add_files('%s', '%s')", catalogName, tableName, sourceTableName); - AssertHelpers.assertThrows( - "Should not allow adding duplicate files", - IllegalStateException.class, - "Cannot complete import because data files to be imported already" - + " exist within the target table", - () -> - scalarSql( - "CALL %s.system.add_files('%s', '%s')", catalogName, tableName, sourceTableName)); + Assertions.assertThatThrownBy( + () -> + scalarSql( + "CALL %s.system.add_files('%s', '%s')", + catalogName, tableName, sourceTableName)) + .as("Should not allow adding duplicate files") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining( + "Cannot complete import because data files to be imported already" + + " exist within the target table"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAlterTableSchema.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAlterTableSchema.java index c993c213dc5e..acbf5c7a632d 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAlterTableSchema.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAlterTableSchema.java @@ -19,9 +19,9 @@ package org.apache.iceberg.spark.extensions; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.Sets; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -77,17 +77,16 @@ public void testSetInvalidIdentifierFields() { Table table = validationCatalog.loadTable(tableIdent); Assert.assertTrue( "Table should start without identifier", table.schema().identifierFieldIds().isEmpty()); - AssertHelpers.assertThrows( - "should not allow setting unknown fields", - IllegalArgumentException.class, - "not found in current schema or added columns", - () -> sql("ALTER TABLE %s SET IDENTIFIER FIELDS unknown", tableName)); - - AssertHelpers.assertThrows( - "should not allow setting optional fields", - IllegalArgumentException.class, - "not a required field", - () -> sql("ALTER TABLE %s SET IDENTIFIER FIELDS id2", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s SET IDENTIFIER FIELDS unknown", tableName)) + .as("should not allow setting unknown fields") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not found in current schema or added columns"); + + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s SET IDENTIFIER FIELDS id2", tableName)) + .as("should not allow setting optional fields") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("not a required field"); } @Test @@ -140,23 +139,23 @@ public void testDropInvalidIdentifierFields() { Table table = validationCatalog.loadTable(tableIdent); Assert.assertTrue( "Table should start without identifier", table.schema().identifierFieldIds().isEmpty()); - AssertHelpers.assertThrows( - "should not allow dropping unknown fields", - IllegalArgumentException.class, - "field unknown not found", - () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS unknown", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS unknown", tableName)) + .as("should not allow dropping unknown fields") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("field unknown not found"); sql("ALTER TABLE %s SET IDENTIFIER FIELDS id", tableName); - AssertHelpers.assertThrows( - "should not allow dropping a field that is not an identifier", - IllegalArgumentException.class, - "data is not an identifier field", - () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS data", tableName)); - - AssertHelpers.assertThrows( - "should not allow dropping a nested field that is not an identifier", - IllegalArgumentException.class, - "location.lon is not an identifier field", - () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS location.lon", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS data", tableName)) + .as("should not allow dropping a field that is not an identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("data is not an identifier field"); + + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s DROP IDENTIFIER FIELDS location.lon", tableName)) + .as("should not allow dropping a nested field that is not an identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("location.lon is not an identifier field"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAncestorsOfProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAncestorsOfProcedure.java index ae591821e21a..13fa96d1e08a 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAncestorsOfProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestAncestorsOfProcedure.java @@ -20,10 +20,10 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Test; @@ -147,22 +147,20 @@ public void testAncestorOfUsingNamedArgs() { @Test public void testInvalidAncestorOfCases() { - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.ancestors_of()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier for parameter 'table'", - () -> sql("CALL %s.system.ancestors_of('')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type for snapshot_id: cannot cast", - () -> sql("CALL %s.system.ancestors_of('%s', 1.1)", catalogName, tableIdent)); + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.ancestors_of()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.ancestors_of('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier for parameter 'table'"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.ancestors_of('%s', 1.1)", catalogName, tableIdent)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type for snapshot_id: cannot cast"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java index e6ac4cfc4dac..8f3e88f3c811 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestBranchDDL.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.SnapshotRef; import org.apache.iceberg.Table; @@ -86,11 +85,11 @@ public void testCreateBranch() throws NoSuchTableException { Assert.assertEquals(TimeUnit.DAYS.toMillis(maxSnapshotAge), ref.maxSnapshotAgeMs().longValue()); Assert.assertEquals(TimeUnit.DAYS.toMillis(maxRefAge), ref.maxRefAgeMs().longValue()); - AssertHelpers.assertThrows( - "Cannot create an existing branch", - IllegalArgumentException.class, - "Ref b1 already exists", - () -> sql("ALTER TABLE %s CREATE BRANCH %s", tableName, branchName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s CREATE BRANCH %s", tableName, branchName)) + .as("Cannot create an existing branch") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ref b1 already exists"); } @Test @@ -196,12 +195,14 @@ public void testCreateBranchUseCustomMinSnapshotsToKeepAndMaxSnapshotAge() Assert.assertEquals(TimeUnit.DAYS.toMillis(maxSnapshotAge), ref.maxSnapshotAgeMs().longValue()); Assert.assertNull(ref.maxRefAgeMs()); - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "no viable alternative at input 'WITH SNAPSHOT RETENTION'", - () -> - sql("ALTER TABLE %s CREATE BRANCH %s WITH SNAPSHOT RETENTION", tableName, branchName)); + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s CREATE BRANCH %s WITH SNAPSHOT RETENTION", + tableName, branchName)) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("no viable alternative at input 'WITH SNAPSHOT RETENTION'"); } @Test @@ -217,26 +218,27 @@ public void testCreateBranchUseCustomMaxRefAge() throws NoSuchTableException { Assert.assertNull(ref.maxSnapshotAgeMs()); Assert.assertEquals(TimeUnit.DAYS.toMillis(maxRefAge), ref.maxRefAgeMs().longValue()); - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input", - () -> sql("ALTER TABLE %s CREATE BRANCH %s RETAIN", tableName, branchName)); - - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input", - () -> sql("ALTER TABLE %s CREATE BRANCH %s RETAIN %s DAYS", tableName, branchName, "abc")); - - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input 'SECONDS' expecting {'DAYS', 'HOURS', 'MINUTES'}", - () -> - sql( - "ALTER TABLE %s CREATE BRANCH %s RETAIN %d SECONDS", - tableName, branchName, maxRefAge)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s CREATE BRANCH %s RETAIN", tableName, branchName)) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input"); + + Assertions.assertThatThrownBy( + () -> + sql("ALTER TABLE %s CREATE BRANCH %s RETAIN %s DAYS", tableName, branchName, "abc")) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input"); + + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s CREATE BRANCH %s RETAIN %d SECONDS", + tableName, branchName, maxRefAge)) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input 'SECONDS' expecting {'DAYS', 'HOURS', 'MINUTES'}"); } @Test @@ -258,11 +260,11 @@ public void testDropBranch() throws NoSuchTableException { @Test public void testDropBranchDoesNotExist() { - AssertHelpers.assertThrows( - "Cannot perform drop branch on branch which does not exist", - IllegalArgumentException.class, - "Branch does not exist: nonExistingBranch", - () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "nonExistingBranch")); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "nonExistingBranch")) + .as("Cannot perform drop branch on branch which does not exist") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Branch does not exist: nonExistingBranch"); } @Test @@ -271,29 +273,26 @@ public void testDropBranchFailsForTag() throws NoSuchTableException { Table table = insertRows(); table.manageSnapshots().createTag(tagName, table.currentSnapshot().snapshotId()).commit(); - AssertHelpers.assertThrows( - "Cannot perform drop branch on tag", - IllegalArgumentException.class, - "Ref b1 is a tag not a branch", - () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, tagName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, tagName)) + .as("Cannot perform drop branch on tag") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ref b1 is a tag not a branch"); } @Test public void testDropBranchNonConformingName() { - AssertHelpers.assertThrows( - "Non-conforming branch name", - IcebergParseException.class, - "mismatched input '123'", - () -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "123")); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s DROP BRANCH %s", tableName, "123")) + .as("Non-conforming branch name") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input '123'"); } @Test public void testDropMainBranchFails() { - AssertHelpers.assertThrows( - "Cannot drop the main branch", - IllegalArgumentException.class, - "Cannot remove main branch", - () -> sql("ALTER TABLE %s DROP BRANCH main", tableName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s DROP BRANCH main", tableName)) + .as("Cannot drop the main branch") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot remove main branch"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCallStatementParser.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCallStatementParser.java index 9c2233ccb791..c0f7ff1f8f9b 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCallStatementParser.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCallStatementParser.java @@ -22,7 +22,6 @@ import java.sql.Timestamp; import java.time.Instant; import java.util.List; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.spark.sql.SparkSession; @@ -38,6 +37,7 @@ import org.apache.spark.sql.catalyst.plans.logical.PositionalArgument; import org.apache.spark.sql.types.DataType; import org.apache.spark.sql.types.DataTypes; +import org.assertj.core.api.Assertions; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -144,11 +144,10 @@ public void testCallWithVarSubstitution() throws ParseException { @Test public void testCallParseError() { - AssertHelpers.assertThrows( - "Should fail with a sensible parse error", - IcebergParseException.class, - "missing '(' at 'radish'", - () -> parser.parsePlan("CALL cat.system radish kebab")); + Assertions.assertThatThrownBy(() -> parser.parsePlan("CALL cat.system radish kebab")) + .as("Should fail with a sensible parse error") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("missing '(' at 'radish'"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java index cc81b4b3d323..c5404c24e47f 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg.spark.extensions; -import static org.apache.iceberg.AssertHelpers.assertThrows; import static org.apache.iceberg.TableProperties.FORMAT_VERSION; import static org.apache.iceberg.TableProperties.MANIFEST_MERGE_ENABLED; import static org.apache.iceberg.TableProperties.MANIFEST_MIN_MERGE_COUNT; @@ -35,6 +34,7 @@ import org.apache.iceberg.spark.source.SparkChangelogTable; import org.apache.spark.sql.DataFrameReader; import org.apache.spark.sql.Row; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -202,10 +202,10 @@ public void testTimeRangeValidation() { Snapshot snap3 = table.currentSnapshot(); long rightAfterSnap3 = waitUntilAfter(snap3.timestampMillis()); - assertThrows( - "Should fail if start time is after end time", - IllegalArgumentException.class, - () -> changelogRecords(snap3.timestampMillis(), snap2.timestampMillis())); + Assertions.assertThatThrownBy( + () -> changelogRecords(snap3.timestampMillis(), snap2.timestampMillis())) + .as("Should fail if start time is after end time") + .isInstanceOf(IllegalArgumentException.class); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCherrypickSnapshotProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCherrypickSnapshotProcedure.java index 7309a176b922..cbecfcef3ed6 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCherrypickSnapshotProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestCherrypickSnapshotProcedure.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; @@ -32,6 +31,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Test; @@ -161,43 +161,42 @@ public void testCherrypickSnapshotRefreshesRelationCache() { public void testCherrypickInvalidSnapshot() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - AssertHelpers.assertThrows( - "Should reject invalid snapshot id", - ValidationException.class, - "Cannot cherry-pick unknown snapshot ID", - () -> sql("CALL %s.system.cherrypick_snapshot('%s', -1L)", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.cherrypick_snapshot('%s', -1L)", catalogName, tableIdent)) + .as("Should reject invalid snapshot id") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot cherry-pick unknown snapshot ID"); } @Test public void testInvalidCherrypickSnapshotCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.cherrypick_snapshot('n', table => 't', 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.cherrypick_snapshot('n', 't', 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.cherrypick_snapshot('t')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.cherrypick_snapshot('', 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type for snapshot_id: cannot cast", - () -> sql("CALL %s.system.cherrypick_snapshot('t', 2.2)", catalogName)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.cherrypick_snapshot('n', table => 't', 1L)", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.cherrypick_snapshot('n', 't', 1L)", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.cherrypick_snapshot('t')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.cherrypick_snapshot('', 1L)", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.cherrypick_snapshot('t', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type for snapshot_id: cannot cast"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java index 6fcbf1f903be..7d183167572d 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestConflictValidation.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.IsolationLevel; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; @@ -31,6 +30,7 @@ import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.functions; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -69,21 +69,24 @@ public void testOverwriteFilterSerializableIsolation() throws Exception { // Validating from previous snapshot finds conflicts Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting new data files should throw exception", - ValidationException.class, - "Found conflicting files that can contain records matching ref(name=\"id\") == 1:", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) - .overwrite(functions.col("id").equalTo(1)); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option( + SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) + .overwrite(functions.col("id").equalTo(1)); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting new data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting files that can contain records matching ref(name=\"id\") == 1:"); // Validating from latest snapshot should succeed table.refresh(); @@ -111,21 +114,23 @@ public void testOverwriteFilterSerializableIsolation2() throws Exception { // Validating from previous snapshot finds conflicts List conflictingRecords = Lists.newArrayList(new SimpleRecord(1, "a")); Dataset conflictingDf = spark.createDataFrame(conflictingRecords, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting new delete files should throw exception", - ValidationException.class, - "Found new conflicting delete files that can apply to records matching ref(name=\"id\") == 1:", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) - .overwrite(functions.col("id").equalTo(1)); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) + .overwrite(functions.col("id").equalTo(1)); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting new delete files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found new conflicting delete files that can apply to records matching ref(name=\"id\") == 1:"); // Validating from latest snapshot should succeed table.refresh(); @@ -149,21 +154,24 @@ public void testOverwriteFilterSerializableIsolation3() throws Exception { // Validating from previous snapshot finds conflicts List conflictingRecords = Lists.newArrayList(new SimpleRecord(1, "a")); Dataset conflictingDf = spark.createDataFrame(conflictingRecords, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting deleted data files should throw exception", - ValidationException.class, - "Found conflicting deleted files that can contain records matching ref(name=\"id\") == 1:", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) - .overwrite(functions.col("id").equalTo(1)); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option( + SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) + .overwrite(functions.col("id").equalTo(1)); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting deleted data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting deleted files that can contain records matching ref(name=\"id\") == 1:"); // Validating from latest snapshot should succeed table.refresh(); @@ -184,20 +192,23 @@ public void testOverwriteFilterNoSnapshotIdValidation() throws Exception { // Validating from no snapshot id defaults to beginning snapshot id and finds conflicts Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting new data files should throw exception", - ValidationException.class, - "Found conflicting files that can contain records matching ref(name=\"id\") == 1:", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) - .overwrite(functions.col("id").equalTo(1)); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option( + SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) + .overwrite(functions.col("id").equalTo(1)); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting new data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting files that can contain records matching ref(name=\"id\") == 1:"); // Validating from latest snapshot should succeed table.refresh(); @@ -225,21 +236,23 @@ public void testOverwriteFilterSnapshotIsolation() throws Exception { // Validating from previous snapshot finds conflicts List conflictingRecords = Lists.newArrayList(new SimpleRecord(1, "a")); Dataset conflictingDf = spark.createDataFrame(conflictingRecords, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting new delete files should throw exception", - ValidationException.class, - "Found new conflicting delete files that can apply to records matching ref(name=\"id\") == 1:", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) - .overwrite(functions.col("id").equalTo(1)); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) + .overwrite(functions.col("id").equalTo(1)); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting new delete files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found new conflicting delete files that can apply to records matching ref(name=\"id\") == 1:"); // Validating from latest snapshot should succeed table.refresh(); @@ -278,21 +291,24 @@ public void testOverwritePartitionSerializableIsolation() throws Exception { // Validating from previous snapshot finds conflicts Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting deleted data files should throw exception", - ValidationException.class, - "Found conflicting files that can contain records matching partitions [id=1]", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) - .overwritePartitions(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option( + SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) + .overwritePartitions(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting deleted data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting files that can contain records matching partitions [id=1]"); // Validating from latest snapshot should succeed table.refresh(); @@ -318,21 +334,23 @@ public void testOverwritePartitionSnapshotIsolation() throws Exception { // Validating from previous snapshot finds conflicts Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting deleted data files should throw exception", - ValidationException.class, - "Found new conflicting delete files that can apply to records matching [id=1]", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) - .overwritePartitions(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) + .overwritePartitions(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting deleted data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found new conflicting delete files that can apply to records matching [id=1]"); // Validating from latest snapshot should succeed table.refresh(); @@ -357,21 +375,23 @@ public void testOverwritePartitionSnapshotIsolation2() throws Exception { spark.createDataFrame(records, SimpleRecord.class).coalesce(1).writeTo(tableName).append(); Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting deleted data files should throw exception", - ValidationException.class, - "Found conflicting deleted files that can apply to records matching [id=1]", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) - .overwritePartitions(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option(SparkWriteOptions.VALIDATE_FROM_SNAPSHOT_ID, String.valueOf(snapshotId)) + .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SNAPSHOT.toString()) + .overwritePartitions(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting deleted data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting deleted files that can apply to records matching [id=1]"); // Validating from latest snapshot should succeed table.refresh(); @@ -409,20 +429,23 @@ public void testOverwritePartitionNoSnapshotIdValidation() throws Exception { // Validating from null snapshot is equivalent to validating from beginning Dataset conflictingDf = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrowsCause( - "Conflicting deleted data files should throw exception", - ValidationException.class, - "Found conflicting files that can contain records matching partitions [id=1]", - () -> { - try { - conflictingDf - .writeTo(tableName) - .option(SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) - .overwritePartitions(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + conflictingDf + .writeTo(tableName) + .option( + SparkWriteOptions.ISOLATION_LEVEL, IsolationLevel.SERIALIZABLE.toString()) + .overwritePartitions(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Conflicting deleted data files should throw exception") + .cause() + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Found conflicting files that can contain records matching partitions [id=1]"); // Validating from latest snapshot should succeed table.refresh(); diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java index e5ea378e5aab..af66df5a9fd4 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java @@ -44,7 +44,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.iceberg.AppendFiles; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.ManifestFile; import org.apache.iceberg.RowLevelOperationMode; @@ -448,11 +447,11 @@ public void testDeleteWithNonDeterministicCondition() { sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware')", tableName); createBranchIfNeeded(); - AssertHelpers.assertThrows( - "Should complain about non-deterministic expressions", - AnalysisException.class, - "nondeterministic expressions are only allowed", - () -> sql("DELETE FROM %s WHERE id = 1 AND rand() > 0.5", commitTarget())); + Assertions.assertThatThrownBy( + () -> sql("DELETE FROM %s WHERE id = 1 AND rand() > 0.5", commitTarget())) + .as("Should complain about non-deterministic expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("nondeterministic expressions are only allowed"); } @Test @@ -737,11 +736,10 @@ public void testDeleteWithNotInSubquery() throws NoSuchTableException { public void testDeleteOnNonIcebergTableNotSupported() { createOrReplaceView("testtable", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Delete is supported only for Iceberg tables", - AnalysisException.class, - "DELETE is only supported with v2 tables.", - () -> sql("DELETE FROM %s WHERE c1 = -100", "testtable")); + Assertions.assertThatThrownBy(() -> sql("DELETE FROM %s WHERE c1 = -100", "testtable")) + .as("Delete is supported only for Iceberg tables") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("DELETE is only supported with v2 tables."); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestExpireSnapshotsProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestExpireSnapshotsProcedure.java index 6383521a44c2..f677facb9902 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestExpireSnapshotsProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestExpireSnapshotsProcedure.java @@ -31,7 +31,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.GenericBlobMetadata; import org.apache.iceberg.GenericStatisticsFile; import org.apache.iceberg.PartitionStatisticsFile; @@ -163,44 +162,42 @@ public void testExpireSnapshotsGCDisabled() { sql("ALTER TABLE %s SET TBLPROPERTIES ('%s' 'false')", tableName, GC_ENABLED); - AssertHelpers.assertThrows( - "Should reject call", - ValidationException.class, - "Cannot expire snapshots: GC is disabled", - () -> sql("CALL %s.system.expire_snapshots('%s')", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.expire_snapshots('%s')", catalogName, tableIdent)) + .as("Should reject call") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot expire snapshots: GC is disabled"); } @Test public void testInvalidExpireSnapshotsCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.expire_snapshots('n', table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.expire_snapshots('n', 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.expire_snapshots()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type", - () -> sql("CALL %s.system.expire_snapshots('n', 2.2)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.expire_snapshots('')", catalogName)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.expire_snapshots('n', table => 't')", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.expire_snapshots('n', 't')", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.expire_snapshots()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.expire_snapshots('n', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.expire_snapshots('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } @Test @@ -218,14 +215,14 @@ public void testResolvingTableInAnotherCatalog() throws IOException { "CREATE TABLE %s.%s (id bigint NOT NULL, data string) USING iceberg", anotherCatalog, tableIdent); - AssertHelpers.assertThrows( - "Should reject calls for a table in another catalog", - IllegalArgumentException.class, - "Cannot run procedure in catalog", - () -> - sql( - "CALL %s.system.expire_snapshots('%s')", - catalogName, anotherCatalog + "." + tableName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.expire_snapshots('%s')", + catalogName, anotherCatalog + "." + tableName)) + .as("Should reject calls for a table in another catalog") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot run procedure in catalog"); } @Test @@ -255,23 +252,23 @@ public void testConcurrentExpireSnapshots() { public void testConcurrentExpireSnapshotsWithInvalidInput() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - AssertHelpers.assertThrows( - "Should throw an error when max_concurrent_deletes = 0", - IllegalArgumentException.class, - "max_concurrent_deletes should have value > 0", - () -> - sql( - "CALL %s.system.expire_snapshots(table => '%s', max_concurrent_deletes => %s)", - catalogName, tableIdent, 0)); - - AssertHelpers.assertThrows( - "Should throw an error when max_concurrent_deletes < 0 ", - IllegalArgumentException.class, - "max_concurrent_deletes should have value > 0", - () -> - sql( - "CALL %s.system.expire_snapshots(table => '%s', max_concurrent_deletes => %s)", - catalogName, tableIdent, -1)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.expire_snapshots(table => '%s', max_concurrent_deletes => %s)", + catalogName, tableIdent, 0)) + .as("Should throw an error when max_concurrent_deletes = 0") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("max_concurrent_deletes should have value > 0"); + + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.expire_snapshots(table => '%s', max_concurrent_deletes => %s)", + catalogName, tableIdent, -1)) + .as("Should throw an error when max_concurrent_deletes < 0 ") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("max_concurrent_deletes should have value > 0"); } @Test @@ -404,19 +401,19 @@ public void testExpireSnapshotShouldFailForCurrentSnapshot() { Table table = validationCatalog.loadTable(tableIdent); Assert.assertEquals("Should be 2 snapshots", 2, Iterables.size(table.snapshots())); - AssertHelpers.assertThrows( - "Should reject call", - IllegalArgumentException.class, - "Cannot expire", - () -> - sql( - "CALL %s.system.expire_snapshots(" - + "table => '%s'," - + "snapshot_ids => ARRAY(%d, %d))", - catalogName, - tableIdent, - table.currentSnapshot().snapshotId(), - table.currentSnapshot().parentId())); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.expire_snapshots(" + + "table => '%s'," + + "snapshot_ids => ARRAY(%d, %d))", + catalogName, + tableIdent, + table.currentSnapshot().snapshotId(), + table.currentSnapshot().parentId())) + .as("Should reject call") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot expire"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMerge.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMerge.java index e855eb49a16c..ead477d71b20 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMerge.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMerge.java @@ -40,7 +40,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.apache.iceberg.AppendFiles; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DistributionMode; import org.apache.iceberg.RowLevelOperationMode; @@ -584,22 +583,23 @@ public void testMergeWithMultipleUpdatesForTargetRowSmallTargetLargeSource() { ds.union(ds).createOrReplaceTempView("source"); String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id == s.value " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET id = 10 " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE " - + "WHEN NOT MATCHED AND s.value = 2 THEN " - + " INSERT (id, dep) VALUES (s.value, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id == s.value " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET id = 10 " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE " + + "WHEN NOT MATCHED AND s.value = 2 THEN " + + " INSERT (id, dep) VALUES (s.value, null)", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); assertEquals( "Target should be unchanged", @@ -626,22 +626,23 @@ public void testMergeWithMultipleUpdatesForTargetRowSmallTargetLargeSource() { () -> { String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id == s.value " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET id = 10 " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE " - + "WHEN NOT MATCHED AND s.value = 2 THEN " - + " INSERT (id, dep) VALUES (s.value, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id == s.value " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET id = 10 " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE " + + "WHEN NOT MATCHED AND s.value = 2 THEN " + + " INSERT (id, dep) VALUES (s.value, null)", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); }); assertEquals( @@ -666,22 +667,23 @@ public void testMergeWithMultipleUpdatesForTargetRowSmallTargetLargeSourceNoEqua () -> { String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id > s.value " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET id = 10 " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE " - + "WHEN NOT MATCHED AND s.value = 2 THEN " - + " INSERT (id, dep) VALUES (s.value, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id > s.value " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET id = 10 " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE " + + "WHEN NOT MATCHED AND s.value = 2 THEN " + + " INSERT (id, dep) VALUES (s.value, null)", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); }); assertEquals( @@ -704,20 +706,21 @@ public void testMergeWithMultipleUpdatesForTargetRowSmallTargetLargeSourceNoNotM ds.union(ds).createOrReplaceTempView("source"); String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id == s.value " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET id = 10 " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id == s.value " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET id = 10 " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); assertEquals( "Target should be unchanged", @@ -738,20 +741,21 @@ public void testMergeWithMultipleUpdatesForTargetRowSmallTargetLargeSourceNoNotM ds.union(ds).createOrReplaceTempView("source"); String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id > s.value " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET id = 10 " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id > s.value " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET id = 10 " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); assertEquals( "Target should be unchanged", @@ -774,22 +778,23 @@ public void testMergeWithMultipleUpdatesForTargetRow() { + "{ \"id\": 6, \"dep\": \"emp-id-6\" }"); String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id == s.id " - + "WHEN MATCHED AND t.id = 1 THEN " - + " UPDATE SET * " - + "WHEN MATCHED AND t.id = 6 THEN " - + " DELETE " - + "WHEN NOT MATCHED AND s.id = 2 THEN " - + " INSERT *", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id == s.id " + + "WHEN MATCHED AND t.id = 1 THEN " + + " UPDATE SET * " + + "WHEN MATCHED AND t.id = 6 THEN " + + " DELETE " + + "WHEN NOT MATCHED AND s.id = 2 THEN " + + " INSERT *", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); assertEquals( "Target should be unchanged", @@ -845,20 +850,21 @@ public void testMergeWithSingleConditionalDelete() { + "{ \"id\": 6, \"dep\": \"emp-id-6\" }"); String errorMsg = "a single row from the target table with multiple rows of the source table"; - AssertHelpers.assertThrowsCause( - "Should complain about multiple matches", - SparkException.class, - errorMsg, - () -> { - sql( - "MERGE INTO %s AS t USING source AS s " - + "ON t.id == s.id " - + "WHEN MATCHED AND t.id = 1 THEN " - + " DELETE " - + "WHEN NOT MATCHED AND s.id = 2 THEN " - + " INSERT *", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s AS t USING source AS s " + + "ON t.id == s.id " + + "WHEN MATCHED AND t.id = 1 THEN " + + " DELETE " + + "WHEN NOT MATCHED AND s.id = 2 THEN " + + " INSERT *", + commitTarget()); + }) + .as("Should complain about multiple matches") + .cause() + .isInstanceOf(SparkException.class) + .hasMessageContaining(errorMsg); assertEquals( "Target should be unchanged", @@ -1985,46 +1991,46 @@ public void testMergeWithNonExistingColumns() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about the invalid top-level column", - AnalysisException.class, - "cannot resolve t.invalid_col", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.invalid_col = s.c2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.invalid_col = s.c2", + commitTarget()); + }) + .as("Should complain about the invalid top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("cannot resolve t.invalid_col"); - AssertHelpers.assertThrows( - "Should complain about the invalid nested column", - AnalysisException.class, - "No such struct field invalid_col", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n2.invalid_col = s.c2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n2.invalid_col = s.c2", + commitTarget()); + }) + .as("Should complain about the invalid nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("No such struct field invalid_col"); - AssertHelpers.assertThrows( - "Should complain about the invalid top-level column", - AnalysisException.class, - "cannot resolve invalid_col", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n2.dn1 = s.c2 " - + "WHEN NOT MATCHED THEN " - + " INSERT (id, invalid_col) VALUES (s.c1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n2.dn1 = s.c2 " + + "WHEN NOT MATCHED THEN " + + " INSERT (id, invalid_col) VALUES (s.c1, null)", + commitTarget()); + }) + .as("Should complain about the invalid top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("cannot resolve invalid_col"); } @Test @@ -2034,48 +2040,48 @@ public void testMergeWithInvalidColumnsInInsert() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about the nested column", - AnalysisException.class, - "Nested fields are not supported inside INSERT clauses", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n2.dn1 = s.c2 " - + "WHEN NOT MATCHED THEN " - + " INSERT (id, c.n2) VALUES (s.c1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n2.dn1 = s.c2 " + + "WHEN NOT MATCHED THEN " + + " INSERT (id, c.n2) VALUES (s.c1, null)", + commitTarget()); + }) + .as("Should complain about the nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Nested fields are not supported inside INSERT clauses"); - AssertHelpers.assertThrows( - "Should complain about duplicate columns", - AnalysisException.class, - "Duplicate column names inside INSERT clause", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n2.dn1 = s.c2 " - + "WHEN NOT MATCHED THEN " - + " INSERT (id, id) VALUES (s.c1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n2.dn1 = s.c2 " + + "WHEN NOT MATCHED THEN " + + " INSERT (id, id) VALUES (s.c1, null)", + commitTarget()); + }) + .as("Should complain about duplicate columns") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Duplicate column names inside INSERT clause"); - AssertHelpers.assertThrows( - "Should complain about missing columns", - AnalysisException.class, - "must provide values for all columns of the target table", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN NOT MATCHED THEN " - + " INSERT (id) VALUES (s.c1)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN NOT MATCHED THEN " + + " INSERT (id) VALUES (s.c1)", + commitTarget()); + }) + .as("Should complain about missing columns") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("must provide values for all columns of the target table"); } @Test @@ -2085,31 +2091,31 @@ public void testMergeWithInvalidUpdates() { "{ \"id\": 1, \"a\": [ { \"c1\": 2, \"c2\": 3 } ], \"m\": { \"k\": \"v\"} }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about updating an array column", - AnalysisException.class, - "Updating nested fields is only supported for structs", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.a.c1 = s.c2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.a.c1 = s.c2", + commitTarget()); + }) + .as("Should complain about updating an array column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updating nested fields is only supported for structs"); - AssertHelpers.assertThrows( - "Should complain about updating a map column", - AnalysisException.class, - "Updating nested fields is only supported for structs", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.m.key = 'new_key'", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.m.key = 'new_key'", + commitTarget()); + }) + .as("Should complain about updating a map column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updating nested fields is only supported for structs"); } @Test @@ -2119,44 +2125,44 @@ public void testMergeWithConflictingUpdates() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a top-level column", - AnalysisException.class, - "Updates are in conflict", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.id = 1, t.c.n1 = 2, t.id = 2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.id = 1, t.c.n1 = 2, t.id = 2", + commitTarget()); + }) + .as("Should complain about conflicting updates to a top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict"); - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a nested column", - AnalysisException.class, - "Updates are in conflict for these columns", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n1 = 1, t.id = 2, t.c.n1 = 2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n1 = 1, t.id = 2, t.c.n1 = 2", + commitTarget()); + }) + .as("Should complain about conflicting updates to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict for these columns"); - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a nested column", - AnalysisException.class, - "Updates are in conflict", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET c.n1 = 1, c = named_struct('n1', 1, 'n2', named_struct('dn1', 1, 'dn2', 2))", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET c.n1 = 1, c = named_struct('n1', 1, 'n2', named_struct('dn1', 1, 'dn2', 2))", + commitTarget()); + }) + .as("Should complain about conflicting updates to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict"); } @Test @@ -2173,70 +2179,70 @@ public void testMergeWithInvalidAssignments() { withSQLConf( ImmutableMap.of("spark.sql.storeAssignmentPolicy", policy), () -> { - AssertHelpers.assertThrows( - "Should complain about writing nulls to a top-level column", - AnalysisException.class, - "Cannot write nullable values to non-null column", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.id = NULL", - commitTarget()); - }); - - AssertHelpers.assertThrows( - "Should complain about writing nulls to a nested column", - AnalysisException.class, - "Cannot write nullable values to non-null column", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.s.n1 = NULL", - commitTarget()); - }); - - AssertHelpers.assertThrows( - "Should complain about writing missing fields in structs", - AnalysisException.class, - "missing fields", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.s = s.c2", - commitTarget()); - }); - - AssertHelpers.assertThrows( - "Should complain about writing invalid data types", - AnalysisException.class, - "Cannot safely cast", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.s.n1 = s.c3", - commitTarget()); - }); - - AssertHelpers.assertThrows( - "Should complain about writing incompatible structs", - AnalysisException.class, - "field name does not match", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.s.n2 = s.c4", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.id = NULL", + commitTarget()); + }) + .as("Should complain about writing nulls to a top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot write nullable values to non-null column"); + + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.s.n1 = NULL", + commitTarget()); + }) + .as("Should complain about writing nulls to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot write nullable values to non-null column"); + + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.s = s.c2", + commitTarget()); + }) + .as("Should complain about writing missing fields in structs") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("missing fields"); + + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.s.n1 = s.c3", + commitTarget()); + }) + .as("Should complain about writing invalid data types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot safely cast"); + + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.s.n2 = s.c4", + commitTarget()); + }) + .as("Should complain about writing incompatible structs") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("field name does not match"); }); } } @@ -2248,57 +2254,57 @@ public void testMergeWithNonDeterministicConditions() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about non-deterministic search conditions", - AnalysisException.class, - "Non-deterministic functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 AND rand() > t.id " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n1 = -1", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 AND rand() > t.id " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n1 = -1", + commitTarget()); + }) + .as("Should complain about non-deterministic search conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Non-deterministic functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about non-deterministic update conditions", - AnalysisException.class, - "Non-deterministic functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND rand() > t.id THEN " - + " UPDATE SET t.c.n1 = -1", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND rand() > t.id THEN " + + " UPDATE SET t.c.n1 = -1", + commitTarget()); + }) + .as("Should complain about non-deterministic update conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Non-deterministic functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about non-deterministic delete conditions", - AnalysisException.class, - "Non-deterministic functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND rand() > t.id THEN " - + " DELETE", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND rand() > t.id THEN " + + " DELETE", + commitTarget()); + }) + .as("Should complain about non-deterministic delete conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Non-deterministic functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about non-deterministic insert conditions", - AnalysisException.class, - "Non-deterministic functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN NOT MATCHED AND rand() > c1 THEN " - + " INSERT (id, c) VALUES (1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN NOT MATCHED AND rand() > c1 THEN " + + " INSERT (id, c) VALUES (1, null)", + commitTarget()); + }) + .as("Should complain about non-deterministic insert conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Non-deterministic functions are not supported"); } @Test @@ -2308,57 +2314,57 @@ public void testMergeWithAggregateExpressions() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about agg expressions in search conditions", - AnalysisException.class, - "Agg functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 AND max(t.id) == 1 " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n1 = -1", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 AND max(t.id) == 1 " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n1 = -1", + commitTarget()); + }) + .as("Should complain about agg expressions in search conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Agg functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about agg expressions in update conditions", - AnalysisException.class, - "Agg functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND sum(t.id) < 1 THEN " - + " UPDATE SET t.c.n1 = -1", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND sum(t.id) < 1 THEN " + + " UPDATE SET t.c.n1 = -1", + commitTarget()); + }) + .as("Should complain about agg expressions in update conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Agg functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about agg expressions in delete conditions", - AnalysisException.class, - "Agg functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND sum(t.id) THEN " - + " DELETE", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND sum(t.id) THEN " + + " DELETE", + commitTarget()); + }) + .as("Should complain about agg expressions in delete conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Agg functions are not supported"); - AssertHelpers.assertThrows( - "Should complain about agg expressions in insert conditions", - AnalysisException.class, - "Agg functions are not supported", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN NOT MATCHED AND sum(c1) < 1 THEN " - + " INSERT (id, c) VALUES (1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN NOT MATCHED AND sum(c1) < 1 THEN " + + " INSERT (id, c) VALUES (1, null)", + commitTarget()); + }) + .as("Should complain about agg expressions in insert conditions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Agg functions are not supported"); } @Test @@ -2368,57 +2374,57 @@ public void testMergeWithSubqueriesInConditions() { "{ \"id\": 1, \"c\": { \"n1\": 2, \"n2\": { \"dn1\": 3, \"dn2\": 4 } } }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain about subquery expressions", - AnalysisException.class, - "Subqueries are not supported in conditions", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 AND t.id < (SELECT max(c2) FROM source) " - + "WHEN MATCHED THEN " - + " UPDATE SET t.c.n1 = s.c2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 AND t.id < (SELECT max(c2) FROM source) " + + "WHEN MATCHED THEN " + + " UPDATE SET t.c.n1 = s.c2", + commitTarget()); + }) + .as("Should complain about subquery expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Subqueries are not supported in conditions"); - AssertHelpers.assertThrows( - "Should complain about subquery expressions", - AnalysisException.class, - "Subqueries are not supported in conditions", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND t.id < (SELECT max(c2) FROM source) THEN " - + " UPDATE SET t.c.n1 = s.c2", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND t.id < (SELECT max(c2) FROM source) THEN " + + " UPDATE SET t.c.n1 = s.c2", + commitTarget()); + }) + .as("Should complain about subquery expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Subqueries are not supported in conditions"); - AssertHelpers.assertThrows( - "Should complain about subquery expressions", - AnalysisException.class, - "Subqueries are not supported in conditions", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN MATCHED AND t.id NOT IN (SELECT c2 FROM source) THEN " - + " DELETE", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN MATCHED AND t.id NOT IN (SELECT c2 FROM source) THEN " + + " DELETE", + commitTarget()); + }) + .as("Should complain about subquery expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Subqueries are not supported in conditions"); - AssertHelpers.assertThrows( - "Should complain about subquery expressions", - AnalysisException.class, - "Subqueries are not supported in conditions", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.c1 " - + "WHEN NOT MATCHED AND s.c1 IN (SELECT c2 FROM source) THEN " - + " INSERT (id, c) VALUES (1, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.c1 " + + "WHEN NOT MATCHED AND s.c1 IN (SELECT c2 FROM source) THEN " + + " INSERT (id, c) VALUES (1, null)", + commitTarget()); + }) + .as("Should complain about subquery expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Subqueries are not supported in conditions"); } @Test @@ -2426,18 +2432,18 @@ public void testMergeWithTargetColumnsInInsertConditions() { createAndInitTable("id INT, c2 INT", "{ \"id\": 1, \"c2\": 2 }"); createOrReplaceView("source", "{ \"id\": 1, \"value\": 11 }"); - AssertHelpers.assertThrows( - "Should complain about the target column", - AnalysisException.class, - "Cannot resolve [c2]", - () -> { - sql( - "MERGE INTO %s t USING source s " - + "ON t.id == s.id " - + "WHEN NOT MATCHED AND c2 = 1 THEN " - + " INSERT (id, c2) VALUES (s.id, null)", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO %s t USING source s " + + "ON t.id == s.id " + + "WHEN NOT MATCHED AND c2 = 1 THEN " + + " INSERT (id, c2) VALUES (s.id, null)", + commitTarget()); + }) + .as("Should complain about the target column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot resolve [c2]"); } @Test @@ -2445,17 +2451,17 @@ public void testMergeWithNonIcebergTargetTableNotSupported() { createOrReplaceView("target", "{ \"c1\": -100, \"c2\": -200 }"); createOrReplaceView("source", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "Should complain non iceberg target table", - UnsupportedOperationException.class, - "MERGE INTO TABLE is not supported temporarily.", - () -> { - sql( - "MERGE INTO target t USING source s " - + "ON t.c1 == s.c1 " - + "WHEN MATCHED THEN " - + " UPDATE SET *"); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "MERGE INTO target t USING source s " + + "ON t.c1 == s.c1 " + + "WHEN MATCHED THEN " + + " UPDATE SET *"); + }) + .as("Should complain non iceberg target table") + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("MERGE INTO TABLE is not supported temporarily."); } /** diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMergeOnReadDelete.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMergeOnReadDelete.java index 6d8120127586..c156e4c77767 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMergeOnReadDelete.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMergeOnReadDelete.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.RowDelta; import org.apache.iceberg.RowLevelOperationMode; import org.apache.iceberg.Table; @@ -38,6 +37,7 @@ import org.apache.iceberg.spark.source.TestSparkCatalog; import org.apache.spark.SparkException; import org.apache.spark.sql.connector.catalog.Identifier; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; import org.junit.runners.Parameterized; @@ -115,13 +115,14 @@ public void testCommitUnknownException() { TestSparkCatalog.setTable(ident, sparkTable); // Although an exception is thrown here, write and commit have succeeded - AssertHelpers.assertThrowsWithCause( - "Should throw a Commit State Unknown Exception", - SparkException.class, - "Writing job aborted", - CommitStateUnknownException.class, - "Datacenter on Fire", - () -> sql("DELETE FROM %s WHERE id = 2", "dummy_catalog.default.table")); + Assertions.assertThatThrownBy( + () -> sql("DELETE FROM %s WHERE id = 2", "dummy_catalog.default.table")) + .as("Should throw a Commit State Unknown Exception") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted") + .cause() + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageContaining("Datacenter on Fire"); // Since write and commit succeeded, the rows should be readable assertEquals( diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMigrateTableProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMigrateTableProcedure.java index b8b5df099089..3a51e87be804 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMigrateTableProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMigrateTableProcedure.java @@ -20,11 +20,11 @@ import java.io.IOException; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -131,14 +131,14 @@ public void testMigrateWithInvalidMetricsConfig() throws IOException { "CREATE TABLE %s (id bigint NOT NULL, data string) USING parquet LOCATION '%s'", tableName, location); - AssertHelpers.assertThrows( - "Should reject invalid metrics config", - ValidationException.class, - "Invalid metrics config", - () -> { - String props = "map('write.metadata.metrics.column.x', 'X')"; - sql("CALL %s.system.migrate('%s', %s)", catalogName, tableName, props); - }); + Assertions.assertThatThrownBy( + () -> { + String props = "map('write.metadata.metrics.column.x', 'X')"; + sql("CALL %s.system.migrate('%s', %s)", catalogName, tableName, props); + }) + .as("Should reject invalid metrics config") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Invalid metrics config"); } @Test @@ -166,23 +166,21 @@ public void testMigrateWithConflictingProps() throws IOException { @Test public void testInvalidMigrateCases() { - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.migrate()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type", - () -> sql("CALL %s.system.migrate(map('foo','bar'))", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.migrate('')", catalogName)); + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.migrate()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.migrate(map('foo','bar'))", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.migrate('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestPublishChangesProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestPublishChangesProcedure.java index 2b74cd475fae..19a065234ed8 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestPublishChangesProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestPublishChangesProcedure.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; @@ -32,6 +31,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Test; @@ -157,37 +157,37 @@ public void testApplyWapChangesRefreshesRelationCache() { public void testApplyInvalidWapId() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - AssertHelpers.assertThrows( - "Should reject invalid wap id", - ValidationException.class, - "Cannot apply unknown WAP ID", - () -> sql("CALL %s.system.publish_changes('%s', 'not_valid')", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.publish_changes('%s', 'not_valid')", catalogName, tableIdent)) + .as("Should reject invalid wap id") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot apply unknown WAP ID"); } @Test public void testInvalidApplyWapChangesCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.publish_changes('n', table => 't', 'not_valid')", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.publish_changes('n', 't', 'not_valid')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.publish_changes('t')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.publish_changes('', 'not_valid')", catalogName)); + Assertions.assertThatThrownBy( + () -> + sql("CALL %s.system.publish_changes('n', table => 't', 'not_valid')", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.publish_changes('n', 't', 'not_valid')", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.publish_changes('t')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.publish_changes('', 'not_valid')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java index 670c4e565760..415141968664 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRemoveOrphanFilesProcedure.java @@ -34,7 +34,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.Files; @@ -227,11 +226,11 @@ public void testRemoveOrphanFilesGCDisabled() { sql("ALTER TABLE %s SET TBLPROPERTIES ('%s' 'false')", tableName, GC_ENABLED); - AssertHelpers.assertThrows( - "Should reject call", - ValidationException.class, - "Cannot delete orphan files: GC is disabled", - () -> sql("CALL %s.system.remove_orphan_files('%s')", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.remove_orphan_files('%s')", catalogName, tableIdent)) + .as("Should reject call") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot delete orphan files: GC is disabled"); // reset the property to enable the table purging in removeTable. sql("ALTER TABLE %s SET TBLPROPERTIES ('%s' 'true')", tableName, GC_ENABLED); @@ -258,35 +257,33 @@ public void testRemoveOrphanFilesWap() { @Test public void testInvalidRemoveOrphanFilesCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.remove_orphan_files('n', table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.remove_orphan_files('n', 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.remove_orphan_files()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type", - () -> sql("CALL %s.system.remove_orphan_files('n', 2.2)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.remove_orphan_files('')", catalogName)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.remove_orphan_files('n', table => 't')", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.remove_orphan_files('n', 't')", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.remove_orphan_files()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.remove_orphan_files('n', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.remove_orphan_files('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } @Test @@ -351,63 +348,63 @@ public void testConcurrentRemoveOrphanFiles() throws IOException { public void testConcurrentRemoveOrphanFilesWithInvalidInput() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - AssertHelpers.assertThrows( - "Should throw an error when max_concurrent_deletes = 0", - IllegalArgumentException.class, - "max_concurrent_deletes should have value > 0", - () -> - sql( - "CALL %s.system.remove_orphan_files(table => '%s', max_concurrent_deletes => %s)", - catalogName, tableIdent, 0)); - - AssertHelpers.assertThrows( - "Should throw an error when max_concurrent_deletes < 0 ", - IllegalArgumentException.class, - "max_concurrent_deletes should have value > 0", - () -> - sql( - "CALL %s.system.remove_orphan_files(table => '%s', max_concurrent_deletes => %s)", - catalogName, tableIdent, -1)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(table => '%s', max_concurrent_deletes => %s)", + catalogName, tableIdent, 0)) + .as("Should throw an error when max_concurrent_deletes = 0") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("max_concurrent_deletes should have value > 0"); + + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(table => '%s', max_concurrent_deletes => %s)", + catalogName, tableIdent, -1)) + .as("Should throw an error when max_concurrent_deletes < 0 ") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("max_concurrent_deletes should have value > 0"); String tempViewName = "file_list_test"; spark.emptyDataFrame().createOrReplaceTempView(tempViewName); - AssertHelpers.assertThrows( - "Should throw an error if file_list_view is missing required columns", - IllegalArgumentException.class, - "does not exist. Available:", - () -> - sql( - "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", - catalogName, tableIdent, tempViewName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", + catalogName, tableIdent, tempViewName)) + .as("Should throw an error if file_list_view is missing required columns") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("does not exist. Available:"); spark .createDataset(Lists.newArrayList(), Encoders.tuple(Encoders.INT(), Encoders.TIMESTAMP())) .toDF("file_path", "last_modified") .createOrReplaceTempView(tempViewName); - AssertHelpers.assertThrows( - "Should throw an error if file_path has wrong type", - IllegalArgumentException.class, - "Invalid file_path column", - () -> - sql( - "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", - catalogName, tableIdent, tempViewName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", + catalogName, tableIdent, tempViewName)) + .as("Should throw an error if file_path has wrong type") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid file_path column"); spark .createDataset(Lists.newArrayList(), Encoders.tuple(Encoders.STRING(), Encoders.STRING())) .toDF("file_path", "last_modified") .createOrReplaceTempView(tempViewName); - AssertHelpers.assertThrows( - "Should throw an error if last_modified has wrong type", - IllegalArgumentException.class, - "Invalid last_modified column", - () -> - sql( - "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", - catalogName, tableIdent, tempViewName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(table => '%s', file_list_view => '%s')", + catalogName, tableIdent, tempViewName)) + .as("Should throw an error if last_modified has wrong type") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid last_modified column"); } @Test @@ -679,16 +676,16 @@ public void testRemoveOrphanFilesProcedureWithPrefixMode() Assert.assertEquals(0, orphanFiles.size()); // Test with no equal schemes - AssertHelpers.assertThrows( - "Should complain about removing orphan files", - ValidationException.class, - "Conflicting authorities/schemes: [(file1, file)]", - () -> - sql( - "CALL %s.system.remove_orphan_files(" - + "table => '%s'," - + "file_list_view => '%s')", - catalogName, tableIdent, fileListViewName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.remove_orphan_files(" + + "table => '%s'," + + "file_list_view => '%s')", + catalogName, tableIdent, fileListViewName)) + .as("Should complain about removing orphan files") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Conflicting authorities/schemes: [(file1, file)]"); // Drop table in afterEach has purge and fails due to invalid scheme "file1" used in this test // Dropping the table here diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReplaceBranch.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReplaceBranch.java index b63826e543b8..ed428310b8f4 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReplaceBranch.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestReplaceBranch.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.SnapshotRef; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -30,6 +29,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -74,11 +74,14 @@ public void testReplaceBranchFailsForTag() throws NoSuchTableException { df.writeTo(tableName).append(); long second = table.currentSnapshot().snapshotId(); - AssertHelpers.assertThrows( - "Cannot perform replace branch on tags", - IllegalArgumentException.class, - "Ref tag1 is a tag not a branch", - () -> sql("ALTER TABLE %s REPLACE BRANCH %s AS OF VERSION %d", tableName, tagName, second)); + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s REPLACE BRANCH %s AS OF VERSION %d", + tableName, tagName, second)) + .as("Cannot perform replace branch on tags") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ref tag1 is a tag not a branch"); } @Test @@ -126,14 +129,14 @@ public void testReplaceBranchDoesNotExist() throws NoSuchTableException { df.writeTo(tableName).append(); Table table = validationCatalog.loadTable(tableIdent); - AssertHelpers.assertThrows( - "Cannot perform replace branch on branch which does not exist", - IllegalArgumentException.class, - "Branch does not exist", - () -> - sql( - "ALTER TABLE %s REPLACE BRANCH %s AS OF VERSION %d", - tableName, "someBranch", table.currentSnapshot().snapshotId())); + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s REPLACE BRANCH %s AS OF VERSION %d", + tableName, "someBranch", table.currentSnapshot().snapshotId())) + .as("Cannot perform replace branch on branch which does not exist") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Branch does not exist"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRequiredDistributionAndOrdering.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRequiredDistributionAndOrdering.java index 02ae0e6ac664..b459f3c24807 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRequiredDistributionAndOrdering.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRequiredDistributionAndOrdering.java @@ -21,7 +21,6 @@ import java.math.BigDecimal; import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.spark.SparkWriteOptions; import org.apache.iceberg.spark.source.ThreeColumnRecord; @@ -29,6 +28,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Test; @@ -187,20 +187,20 @@ public void testDisabledDistributionAndOrdering() { Dataset inputDF = ds.coalesce(1).sortWithinPartitions("c1"); // should fail if ordering is disabled - AssertHelpers.assertThrows( - "Should reject writes without ordering", - SparkException.class, - "Writing job aborted", - () -> { - try { - inputDF - .writeTo(tableName) - .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") - .append(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + inputDF + .writeTo(tableName) + .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") + .append(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Should reject writes without ordering") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java index 80cacbf376ac..4a4e4d7dfd7b 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteDataFilesProcedure.java @@ -24,7 +24,6 @@ import java.util.List; import java.util.Map; import java.util.stream.IntStream; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.SnapshotSummary; import org.apache.iceberg.TableProperties; import org.apache.iceberg.catalog.TableIdentifier; @@ -455,136 +454,137 @@ public void testRewriteDataFilesWithInvalidInputs() { insertData(2); // Test for invalid strategy - AssertHelpers.assertThrows( - "Should reject calls with unsupported strategy error message", - IllegalArgumentException.class, - "unsupported strategy: temp. Only binpack or sort is supported", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', options => map('min-input-files','2'), " - + "strategy => 'temp')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', options => map('min-input-files','2'), " + + "strategy => 'temp')", + catalogName, tableIdent)) + .as("Should reject calls with unsupported strategy error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("unsupported strategy: temp. Only binpack or sort is supported"); // Test for sort_order with binpack strategy - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Must use only one rewriter type (bin-pack, sort, zorder)", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'binpack', " - + "sort_order => 'c1 ASC NULLS FIRST')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'binpack', " + + "sort_order => 'c1 ASC NULLS FIRST')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Must use only one rewriter type (bin-pack, sort, zorder)"); // Test for sort strategy without any (default/user defined) sort_order - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Cannot sort data without a valid sort order", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot sort data without a valid sort order"); // Test for sort_order with invalid null order - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Unable to parse sortOrder:", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " - + "sort_order => 'c1 ASC none')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " + + "sort_order => 'c1 ASC none')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unable to parse sortOrder:"); // Test for sort_order with invalid sort direction - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Unable to parse sortOrder:", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " - + "sort_order => 'c1 none NULLS FIRST')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " + + "sort_order => 'c1 none NULLS FIRST')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unable to parse sortOrder:"); // Test for sort_order with invalid column name - AssertHelpers.assertThrows( - "Should reject calls with error message", - ValidationException.class, - "Cannot find field 'col1' in struct:" - + " struct<1: c1: optional int, 2: c2: optional string, 3: c3: optional string>", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " - + "sort_order => 'col1 DESC NULLS FIRST')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " + + "sort_order => 'col1 DESC NULLS FIRST')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Cannot find field 'col1' in struct:" + + " struct<1: c1: optional int, 2: c2: optional string, 3: c3: optional string>"); // Test with invalid filter column col1 - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Cannot parse predicates in where option: col1 = 3", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', " + "where => 'col1 = 3')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', " + "where => 'col1 = 3')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot parse predicates in where option: col1 = 3"); // Test for z_order with invalid column name - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Cannot find column 'col1' in table schema (case sensitive = false): " - + "struct<1: c1: optional int, 2: c2: optional string, 3: c3: optional string>", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " - + "sort_order => 'zorder(col1)')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " + + "sort_order => 'zorder(col1)')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Cannot find column 'col1' in table schema (case sensitive = false): " + + "struct<1: c1: optional int, 2: c2: optional string, 3: c3: optional string>"); // Test for z_order with sort_order - AssertHelpers.assertThrows( - "Should reject calls with error message", - IllegalArgumentException.class, - "Cannot mix identity sort columns and a Zorder sort expression:" + " c1,zorder(c2,c3)", - () -> - sql( - "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " - + "sort_order => 'c1,zorder(c2,c3)')", - catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rewrite_data_files(table => '%s', strategy => 'sort', " + + "sort_order => 'c1,zorder(c2,c3)')", + catalogName, tableIdent)) + .as("Should reject calls with error message") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Cannot mix identity sort columns and a Zorder sort expression:" + " c1,zorder(c2,c3)"); } @Test public void testInvalidCasesForRewriteDataFiles() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.rewrite_data_files('n', table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.rewrite_data_files('n', 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rewrite_data_files()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject duplicate arg names name", - AnalysisException.class, - "Duplicate procedure argument: table", - () -> sql("CALL %s.system.rewrite_data_files(table => 't', table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.rewrite_data_files('')", catalogName)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rewrite_data_files('n', table => 't')", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.rewrite_data_files('n', 't')", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.rewrite_data_files()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rewrite_data_files(table => 't', table => 't')", catalogName)) + .as("Should reject duplicate arg names name") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Duplicate procedure argument: table"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.rewrite_data_files('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java index 2675c1010baa..3afad8a7b839 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRewriteManifestsProcedure.java @@ -24,7 +24,6 @@ import java.sql.Timestamp; import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -32,6 +31,7 @@ import org.apache.spark.sql.RowFactory; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -276,41 +276,39 @@ public void testRewriteManifestsCaseInsensitiveArgs() { @Test public void testInvalidRewriteManifestsCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> sql("CALL %s.system.rewrite_manifests('n', table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.rewrite_manifests('n', 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rewrite_manifests()", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type", - () -> sql("CALL %s.system.rewrite_manifests('n', 2.2)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject duplicate arg names name", - AnalysisException.class, - "Duplicate procedure argument: table", - () -> sql("CALL %s.system.rewrite_manifests(table => 't', tAbLe => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.rewrite_manifests('')", catalogName)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rewrite_manifests('n', table => 't')", catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.rewrite_manifests('n', 't')", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.rewrite_manifests()", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rewrite_manifests('n', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rewrite_manifests(table => 't', tAbLe => 't')", catalogName)) + .as("Should reject duplicate arg names name") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Duplicate procedure argument: table"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.rewrite_manifests('')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToSnapshotProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToSnapshotProcedure.java index af94b456d02e..95d03a0104fd 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToSnapshotProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToSnapshotProcedure.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; @@ -29,6 +28,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assume; import org.junit.Test; @@ -240,58 +240,57 @@ public void testRollbackToSnapshotWithoutExplicitCatalog() { public void testRollbackToInvalidSnapshot() { sql("CREATE TABLE %s (id bigint NOT NULL, data string) USING iceberg", tableName); - AssertHelpers.assertThrows( - "Should reject invalid snapshot id", - ValidationException.class, - "Cannot roll back to unknown snapshot id", - () -> sql("CALL %s.system.rollback_to_snapshot('%s', -1L)", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_snapshot('%s', -1L)", catalogName, tableIdent)) + .as("Should reject invalid snapshot id") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot roll back to unknown snapshot id"); } @Test public void testInvalidRollbackToSnapshotCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> - sql( - "CALL %s.system.rollback_to_snapshot(namespace => 'n1', table => 't', 1L)", - catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.rollback_to_snapshot('n', 't', 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_snapshot('t')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_snapshot(1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_snapshot(table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type for snapshot_id: cannot cast", - () -> sql("CALL %s.system.rollback_to_snapshot('t', 2.2)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.rollback_to_snapshot('', 1L)", catalogName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rollback_to_snapshot(namespace => 'n1', table => 't', 1L)", + catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.rollback_to_snapshot('n', 't', 1L)", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_snapshot('t')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.rollback_to_snapshot(1L)", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_snapshot(table => 't')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_snapshot('t', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type for snapshot_id: cannot cast"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_snapshot('', 1L)", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToTimestampProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToTimestampProcedure.java index 6da3853bbe24..c11c7ab7e421 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToTimestampProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRollbackToTimestampProcedure.java @@ -21,7 +21,6 @@ import java.time.LocalDateTime; import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -29,6 +28,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assume; import org.junit.Test; @@ -255,50 +255,55 @@ public void testRollbackToTimestampWithoutExplicitCatalog() { public void testInvalidRollbackToTimestampCases() { String timestamp = "TIMESTAMP '2007-12-03T10:15:30'"; - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> - sql( - "CALL %s.system.rollback_to_timestamp(namespace => 'n1', 't', %s)", - catalogName, timestamp)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.rollback_to_timestamp('n', 't', %s)", catalogName, timestamp)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_timestamp('t')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_timestamp(timestamp => %s)", catalogName, timestamp)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.rollback_to_timestamp(table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with extra args", - AnalysisException.class, - "Too many arguments", - () -> - sql("CALL %s.system.rollback_to_timestamp('n', 't', %s, 1L)", catalogName, timestamp)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type for timestamp: cannot cast", - () -> sql("CALL %s.system.rollback_to_timestamp('t', 2.2)", catalogName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rollback_to_timestamp(namespace => 'n1', 't', %s)", + catalogName, timestamp)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.rollback_to_timestamp('n', 't', %s)", catalogName, timestamp)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_timestamp('t')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rollback_to_timestamp(timestamp => %s)", + catalogName, timestamp)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_timestamp(table => 't')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.rollback_to_timestamp('n', 't', %s, 1L)", + catalogName, timestamp)) + .as("Should reject calls with extra args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Too many arguments"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.rollback_to_timestamp('t', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type for timestamp: cannot cast"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetCurrentSnapshotProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetCurrentSnapshotProcedure.java index 8a8a974bbebe..d996e4b5c805 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetCurrentSnapshotProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSetCurrentSnapshotProcedure.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Snapshot; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.Namespace; @@ -31,6 +30,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Iterables; import org.apache.spark.sql.AnalysisException; import org.apache.spark.sql.catalyst.analysis.NoSuchProcedureException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assume; import org.junit.Test; @@ -193,64 +193,63 @@ public void testSetCurrentSnapshotToInvalidSnapshot() { Namespace namespace = tableIdent.namespace(); String tableName = tableIdent.name(); - AssertHelpers.assertThrows( - "Should reject invalid snapshot id", - ValidationException.class, - "Cannot roll back to unknown snapshot id", - () -> sql("CALL %s.system.set_current_snapshot('%s', -1L)", catalogName, tableIdent)); + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot('%s', -1L)", catalogName, tableIdent)) + .as("Should reject invalid snapshot id") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot roll back to unknown snapshot id"); } @Test public void testInvalidRollbackToSnapshotCases() { - AssertHelpers.assertThrows( - "Should not allow mixed args", - AnalysisException.class, - "Named and positional arguments cannot be mixed", - () -> - sql( - "CALL %s.system.set_current_snapshot(namespace => 'n1', table => 't', 1L)", - catalogName)); - - AssertHelpers.assertThrows( - "Should not resolve procedures in arbitrary namespaces", - NoSuchProcedureException.class, - "not found", - () -> sql("CALL %s.custom.set_current_snapshot('n', 't', 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.set_current_snapshot('t')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.set_current_snapshot(1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.set_current_snapshot(snapshot_id => 1L)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.set_current_snapshot(table => 't')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type for snapshot_id: cannot cast", - () -> sql("CALL %s.system.set_current_snapshot('t', 2.2)", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.set_current_snapshot('', 1L)", catalogName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.set_current_snapshot(namespace => 'n1', table => 't', 1L)", + catalogName)) + .as("Should not allow mixed args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Named and positional arguments cannot be mixed"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.custom.set_current_snapshot('n', 't', 1L)", catalogName)) + .as("Should not resolve procedures in arbitrary namespaces") + .isInstanceOf(NoSuchProcedureException.class) + .hasMessageContaining("not found"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot('t')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.set_current_snapshot(1L)", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot(snapshot_id => 1L)", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot(table => 't')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot('t', 2.2)", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type for snapshot_id: cannot cast"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.set_current_snapshot('', 1L)", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSnapshotTableProcedure.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSnapshotTableProcedure.java index d8e918d8aadd..d44f8d3721ca 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSnapshotTableProcedure.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestSnapshotTableProcedure.java @@ -20,11 +20,11 @@ import java.io.IOException; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.TableProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -195,37 +195,34 @@ public void testInvalidSnapshotsCases() throws IOException { "CREATE TABLE %s (id bigint NOT NULL, data string) USING parquet LOCATION '%s'", sourceName, location); - AssertHelpers.assertThrows( - "Should reject calls without all required args", - AnalysisException.class, - "Missing required parameters", - () -> sql("CALL %s.system.snapshot('foo')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid arg types", - AnalysisException.class, - "Wrong arg type", - () -> sql("CALL %s.system.snapshot('n', 't', map('foo', 'bar'))", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with invalid map args", - AnalysisException.class, - "cannot resolve 'map", - () -> - sql( - "CALL %s.system.snapshot('%s', 'fable', 'loc', map(2, 1, 1))", - catalogName, sourceName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.snapshot('', 'dest')", catalogName)); - - AssertHelpers.assertThrows( - "Should reject calls with empty table identifier", - IllegalArgumentException.class, - "Cannot handle an empty identifier", - () -> sql("CALL %s.system.snapshot('src', '')", catalogName)); + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.snapshot('foo')", catalogName)) + .as("Should reject calls without all required args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Missing required parameters"); + + Assertions.assertThatThrownBy( + () -> sql("CALL %s.system.snapshot('n', 't', map('foo', 'bar'))", catalogName)) + .as("Should reject calls with invalid arg types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Wrong arg type"); + + Assertions.assertThatThrownBy( + () -> + sql( + "CALL %s.system.snapshot('%s', 'fable', 'loc', map(2, 1, 1))", + catalogName, sourceName)) + .as("Should reject calls with invalid map args") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("cannot resolve 'map"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.snapshot('', 'dest')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); + + Assertions.assertThatThrownBy(() -> sql("CALL %s.system.snapshot('src', '')", catalogName)) + .as("Should reject calls with empty table identifier") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot handle an empty identifier"); } } diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestTagDDL.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestTagDDL.java index 866a965e33e6..3dadadd9c4de 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestTagDDL.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestTagDDL.java @@ -24,7 +24,6 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.TimeUnit; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.SnapshotRef; import org.apache.iceberg.Table; import org.apache.iceberg.exceptions.ValidationException; @@ -97,29 +96,29 @@ public void testCreateTagWithRetain() throws NoSuchTableException { } String tagName = "t1"; - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input", - () -> - sql( - "ALTER TABLE %s CREATE TAG %s AS OF VERSION %d RETAIN", - tableName, tagName, firstSnapshotId, maxRefAge)); - - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input", - () -> sql("ALTER TABLE %s CREATE TAG %s RETAIN %s DAYS", tableName, tagName, "abc")); - - AssertHelpers.assertThrows( - "Illegal statement", - IcebergParseException.class, - "mismatched input 'SECONDS' expecting {'DAYS', 'HOURS', 'MINUTES'}", - () -> - sql( - "ALTER TABLE %s CREATE TAG %s AS OF VERSION %d RETAIN %d SECONDS", - tableName, tagName, firstSnapshotId, maxRefAge)); + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s CREATE TAG %s AS OF VERSION %d RETAIN", + tableName, tagName, firstSnapshotId, maxRefAge)) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input"); + + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s CREATE TAG %s RETAIN %s DAYS", tableName, tagName, "abc")) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input"); + + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s CREATE TAG %s AS OF VERSION %d RETAIN %d SECONDS", + tableName, tagName, firstSnapshotId, maxRefAge)) + .as("Illegal statement") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input 'SECONDS' expecting {'DAYS', 'HOURS', 'MINUTES'}"); } @Test @@ -137,11 +136,11 @@ public void testCreateTagUseDefaultConfig() throws NoSuchTableException { long snapshotId = table.currentSnapshot().snapshotId(); String tagName = "t1"; - AssertHelpers.assertThrows( - "unknown snapshot", - ValidationException.class, - "unknown snapshot: -1", - () -> sql("ALTER TABLE %s CREATE TAG %s AS OF VERSION %d", tableName, tagName, -1)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s CREATE TAG %s AS OF VERSION %d", tableName, tagName, -1)) + .as("unknown snapshot") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("unknown snapshot: -1"); sql("ALTER TABLE %s CREATE TAG %s", tableName, tagName); table.refresh(); @@ -151,17 +150,15 @@ public void testCreateTagUseDefaultConfig() throws NoSuchTableException { Assert.assertNull( "The tag needs to have the default max ref age, which is null.", ref.maxRefAgeMs()); - AssertHelpers.assertThrows( - "Cannot create an exist tag", - IllegalArgumentException.class, - "already exists", - () -> sql("ALTER TABLE %s CREATE TAG %s", tableName, tagName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s CREATE TAG %s", tableName, tagName)) + .as("Cannot create an exist tag") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("already exists"); - AssertHelpers.assertThrows( - "Non-conforming tag name", - IcebergParseException.class, - "mismatched input '123'", - () -> sql("ALTER TABLE %s CREATE TAG %s", tableName, "123")); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s CREATE TAG %s", tableName, "123")) + .as("Non-conforming tag name") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input '123'"); table.manageSnapshots().removeTag(tagName).commit(); List records = @@ -207,11 +204,11 @@ public void testReplaceTagFailsForBranch() throws NoSuchTableException { insertRows(); long second = table.currentSnapshot().snapshotId(); - AssertHelpers.assertThrows( - "Cannot perform replace tag on branches", - IllegalArgumentException.class, - "Ref branch1 is a branch not a tag", - () -> sql("ALTER TABLE %s REPLACE Tag %s", tableName, branchName, second)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s REPLACE Tag %s", tableName, branchName, second)) + .as("Cannot perform replace tag on branches") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ref branch1 is a branch not a tag"); } @Test @@ -244,14 +241,14 @@ public void testReplaceTag() throws NoSuchTableException { public void testReplaceTagDoesNotExist() throws NoSuchTableException { Table table = insertRows(); - AssertHelpers.assertThrows( - "Cannot perform replace tag on tag which does not exist", - IllegalArgumentException.class, - "Tag does not exist", - () -> - sql( - "ALTER TABLE %s REPLACE Tag %s AS OF VERSION %d", - tableName, "someTag", table.currentSnapshot().snapshotId())); + Assertions.assertThatThrownBy( + () -> + sql( + "ALTER TABLE %s REPLACE Tag %s AS OF VERSION %d", + tableName, "someTag", table.currentSnapshot().snapshotId())) + .as("Cannot perform replace tag on tag which does not exist") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Tag does not exist"); } @Test @@ -316,20 +313,19 @@ public void testDropTag() throws NoSuchTableException { @Test public void testDropTagNonConformingName() { - AssertHelpers.assertThrows( - "Non-conforming tag name", - IcebergParseException.class, - "mismatched input '123'", - () -> sql("ALTER TABLE %s DROP TAG %s", tableName, "123")); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s DROP TAG %s", tableName, "123")) + .as("Non-conforming tag name") + .isInstanceOf(IcebergParseException.class) + .hasMessageContaining("mismatched input '123'"); } @Test public void testDropTagDoesNotExist() { - AssertHelpers.assertThrows( - "Cannot perform drop tag on tag which does not exist", - IllegalArgumentException.class, - "Tag does not exist: nonExistingTag", - () -> sql("ALTER TABLE %s DROP TAG %s", tableName, "nonExistingTag")); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s DROP TAG %s", tableName, "nonExistingTag")) + .as("Cannot perform drop tag on tag which does not exist") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Tag does not exist: nonExistingTag"); } @Test @@ -338,11 +334,10 @@ public void testDropTagFailesForBranch() throws NoSuchTableException { Table table = insertRows(); table.manageSnapshots().createBranch(branchName, table.currentSnapshot().snapshotId()).commit(); - AssertHelpers.assertThrows( - "Cannot perform drop tag on branch", - IllegalArgumentException.class, - "Ref b1 is a branch not a tag", - () -> sql("ALTER TABLE %s DROP TAG %s", tableName, branchName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s DROP TAG %s", tableName, branchName)) + .as("Cannot perform drop tag on branch") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Ref b1 is a branch not a tag"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestUpdate.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestUpdate.java index 8093e6fc0984..76c7a171f603 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestUpdate.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestUpdate.java @@ -42,7 +42,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.apache.iceberg.AppendFiles; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.RowLevelOperationMode; import org.apache.iceberg.Snapshot; @@ -1149,17 +1148,15 @@ public void testUpdateWithInvalidUpdates() { "id INT, a ARRAY>, m MAP", "{ \"id\": 0, \"a\": null, \"m\": null }"); - AssertHelpers.assertThrows( - "Should complain about updating an array column", - AnalysisException.class, - "Updating nested fields is only supported for structs", - () -> sql("UPDATE %s SET a.c1 = 1", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about updating a map column", - AnalysisException.class, - "Updating nested fields is only supported for structs", - () -> sql("UPDATE %s SET m.key = 'new_key'", commitTarget())); + Assertions.assertThatThrownBy(() -> sql("UPDATE %s SET a.c1 = 1", commitTarget())) + .as("Should complain about updating an array column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updating nested fields is only supported for structs"); + + Assertions.assertThatThrownBy(() -> sql("UPDATE %s SET m.key = 'new_key'", commitTarget())) + .as("Should complain about updating a map column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updating nested fields is only supported for structs"); } @Test @@ -1167,27 +1164,27 @@ public void testUpdateWithConflictingAssignments() { createAndInitTable( "id INT, c STRUCT>", "{ \"id\": 0, \"s\": null }"); - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a top-level column", - AnalysisException.class, - "Updates are in conflict", - () -> sql("UPDATE %s t SET t.id = 1, t.c.n1 = 2, t.id = 2", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a nested column", - AnalysisException.class, - "Updates are in conflict for these columns", - () -> sql("UPDATE %s t SET t.c.n1 = 1, t.id = 2, t.c.n1 = 2", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about conflicting updates to a nested column", - AnalysisException.class, - "Updates are in conflict", - () -> { - sql( - "UPDATE %s SET c.n1 = 1, c = named_struct('n1', 1, 'n2', named_struct('dn1', 1, 'dn2', 2))", - commitTarget()); - }); + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s t SET t.id = 1, t.c.n1 = 2, t.id = 2", commitTarget())) + .as("Should complain about conflicting updates to a top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict"); + + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s t SET t.c.n1 = 1, t.id = 2, t.c.n1 = 2", commitTarget())) + .as("Should complain about conflicting updates to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict for these columns"); + + Assertions.assertThatThrownBy( + () -> { + sql( + "UPDATE %s SET c.n1 = 1, c = named_struct('n1', 1, 'n2', named_struct('dn1', 1, 'dn2', 2))", + commitTarget()); + }) + .as("Should complain about conflicting updates to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Updates are in conflict"); } @Test @@ -1200,38 +1197,37 @@ public void testUpdateWithInvalidAssignments() { withSQLConf( ImmutableMap.of("spark.sql.storeAssignmentPolicy", policy), () -> { - AssertHelpers.assertThrows( - "Should complain about writing nulls to a top-level column", - AnalysisException.class, - "Cannot write nullable values to non-null column", - () -> sql("UPDATE %s t SET t.id = NULL", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about writing nulls to a nested column", - AnalysisException.class, - "Cannot write nullable values to non-null column", - () -> sql("UPDATE %s t SET t.s.n1 = NULL", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about writing missing fields in structs", - AnalysisException.class, - "missing fields", - () -> sql("UPDATE %s t SET t.s = named_struct('n1', 1)", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about writing invalid data types", - AnalysisException.class, - "Cannot safely cast", - () -> sql("UPDATE %s t SET t.s.n1 = 'str'", commitTarget())); - - AssertHelpers.assertThrows( - "Should complain about writing incompatible structs", - AnalysisException.class, - "field name does not match", - () -> - sql( - "UPDATE %s t SET t.s.n2 = named_struct('dn2', 1, 'dn1', 2)", - commitTarget())); + Assertions.assertThatThrownBy(() -> sql("UPDATE %s t SET t.id = NULL", commitTarget())) + .as("Should complain about writing nulls to a top-level column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot write nullable values to non-null column"); + + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s t SET t.s.n1 = NULL", commitTarget())) + .as("Should complain about writing nulls to a nested column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot write nullable values to non-null column"); + + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s t SET t.s = named_struct('n1', 1)", commitTarget())) + .as("Should complain about writing missing fields in structs") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("missing fields"); + + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s t SET t.s.n1 = 'str'", commitTarget())) + .as("Should complain about writing invalid data types") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot safely cast"); + + Assertions.assertThatThrownBy( + () -> + sql( + "UPDATE %s t SET t.s.n2 = named_struct('dn2', 1, 'dn1', 2)", + commitTarget())) + .as("Should complain about writing incompatible structs") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("field name does not match"); }); } } @@ -1240,22 +1236,21 @@ public void testUpdateWithInvalidAssignments() { public void testUpdateWithNonDeterministicCondition() { createAndInitTable("id INT, dep STRING", "{ \"id\": 1, \"dep\": \"hr\" }"); - AssertHelpers.assertThrows( - "Should complain about non-deterministic expressions", - AnalysisException.class, - "nondeterministic expressions are only allowed", - () -> sql("UPDATE %s SET id = -1 WHERE id = 1 AND rand() > 0.5", commitTarget())); + Assertions.assertThatThrownBy( + () -> sql("UPDATE %s SET id = -1 WHERE id = 1 AND rand() > 0.5", commitTarget())) + .as("Should complain about non-deterministic expressions") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("nondeterministic expressions are only allowed"); } @Test public void testUpdateOnNonIcebergTableNotSupported() { createOrReplaceView("testtable", "{ \"c1\": -100, \"c2\": -200 }"); - AssertHelpers.assertThrows( - "UPDATE is not supported for non iceberg table", - UnsupportedOperationException.class, - "not supported temporarily", - () -> sql("UPDATE %s SET c1 = -1 WHERE c2 = 1", "testtable")); + Assertions.assertThatThrownBy(() -> sql("UPDATE %s SET c1 = -1 WHERE c2 = 1", "testtable")) + .as("UPDATE is not supported for non iceberg table") + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("not supported temporarily"); } @Test diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestWriteAborts.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestWriteAborts.java index 24a9b3f232e0..2966e95fa3de 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestWriteAborts.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestWriteAborts.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.TableProperties; import org.apache.iceberg.hadoop.HadoopFileIO; @@ -39,6 +38,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Rule; import org.junit.Test; @@ -105,23 +105,23 @@ public void testBatchAppend() throws Exception { new SimpleRecord(4, "b")); Dataset inputDF = spark.createDataFrame(records, SimpleRecord.class); - AssertHelpers.assertThrows( - "Write must fail", - SparkException.class, - "Writing job aborted", - () -> { - try { - // incoming records are not ordered by partitions so the job must fail - inputDF - .coalesce(1) - .sortWithinPartitions("id") - .writeTo(tableName) - .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") - .append(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + // incoming records are not ordered by partitions so the job must fail + inputDF + .coalesce(1) + .sortWithinPartitions("id") + .writeTo(tableName) + .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") + .append(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Write must fail") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted"); assertEquals("Should be no records", sql("SELECT * FROM %s", tableName), ImmutableList.of()); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/TestFunctionCatalog.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/TestFunctionCatalog.java index 9dcd59cb4f11..d224e175d3b0 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/TestFunctionCatalog.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/TestFunctionCatalog.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg.spark; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.IcebergBuild; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.spark.functions.IcebergVersionFunction; @@ -68,11 +67,10 @@ public void testListFunctionsViaCatalog() throws NoSuchNamespaceException { new Identifier[0], asFunctionCatalog.listFunctions(DEFAULT_NAMESPACE)); - AssertHelpers.assertThrows( - "Listing functions in a namespace that does not exist should throw", - NoSuchNamespaceException.class, - "Namespace 'db' not found", - () -> asFunctionCatalog.listFunctions(DB_NAMESPACE)); + Assertions.assertThatThrownBy(() -> asFunctionCatalog.listFunctions(DB_NAMESPACE)) + .as("Listing functions in a namespace that does not exist should throw") + .isInstanceOf(NoSuchNamespaceException.class) + .hasMessageContaining("Namespace 'db' not found"); } @Test @@ -87,24 +85,24 @@ public void testLoadFunctions() throws NoSuchFunctionException { .isExactlyInstanceOf(IcebergVersionFunction.class); } - AssertHelpers.assertThrows( - "Cannot load a function if it's not used with the system namespace or the empty namespace", - NoSuchFunctionException.class, - "Undefined function: default.iceberg_version", - () -> asFunctionCatalog.loadFunction(Identifier.of(DEFAULT_NAMESPACE, "iceberg_version"))); + Assertions.assertThatThrownBy( + () -> + asFunctionCatalog.loadFunction(Identifier.of(DEFAULT_NAMESPACE, "iceberg_version"))) + .as( + "Cannot load a function if it's not used with the system namespace or the empty namespace") + .isInstanceOf(NoSuchFunctionException.class) + .hasMessageContaining("Undefined function: default.iceberg_version"); Identifier undefinedFunction = Identifier.of(SYSTEM_NAMESPACE, "undefined_function"); - AssertHelpers.assertThrows( - "Cannot load a function that does not exist", - NoSuchFunctionException.class, - "Undefined function: system.undefined_function", - () -> asFunctionCatalog.loadFunction(undefinedFunction)); - - AssertHelpers.assertThrows( - "Using an undefined function from SQL should fail analysis", - AnalysisException.class, - "Undefined function", - () -> sql("SELECT undefined_function(1, 2)")); + Assertions.assertThatThrownBy(() -> asFunctionCatalog.loadFunction(undefinedFunction)) + .as("Cannot load a function that does not exist") + .isInstanceOf(NoSuchFunctionException.class) + .hasMessageContaining("Undefined function: system.undefined_function"); + + Assertions.assertThatThrownBy(() -> sql("SELECT undefined_function(1, 2)")) + .as("Using an undefined function from SQL should fail analysis") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Undefined function"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java index 0e333f216c26..5b1cf8281784 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestDeleteReachableFilesAction.java @@ -27,7 +27,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.StreamSupport; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.DeleteFile; @@ -51,6 +50,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Sets; import org.apache.iceberg.spark.SparkTestBase; import org.apache.iceberg.types.Types; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -362,22 +362,22 @@ public void testIgnoreMetadataFilesNotFound() { public void testEmptyIOThrowsException() { DeleteReachableFiles baseRemoveFilesSparkAction = sparkActions().deleteReachableFiles(metadataLocation(table)).io(null); - AssertHelpers.assertThrows( - "FileIO can't be null in DeleteReachableFiles action", - IllegalArgumentException.class, - "File IO cannot be null", - baseRemoveFilesSparkAction::execute); + Assertions.assertThatThrownBy(baseRemoveFilesSparkAction::execute) + .as("FileIO can't be null in DeleteReachableFiles action") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("File IO cannot be null"); } @Test public void testRemoveFilesActionWhenGarbageCollectionDisabled() { table.updateProperties().set(TableProperties.GC_ENABLED, "false").commit(); - AssertHelpers.assertThrows( - "Should complain about removing files when GC is disabled", - ValidationException.class, - "Cannot delete files: GC is disabled (deleting files may corrupt other tables)", - () -> sparkActions().deleteReachableFiles(metadataLocation(table)).execute()); + Assertions.assertThatThrownBy( + () -> sparkActions().deleteReachableFiles(metadataLocation(table)).execute()) + .as("Should complain about removing files when GC is disabled") + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Cannot delete files: GC is disabled (deleting files may corrupt other tables)"); } private String metadataLocation(Table tbl) { diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java index c8bef4a2a973..a3580bd2d980 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestExpireSnapshotsAction.java @@ -29,7 +29,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseTable; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; @@ -57,6 +56,7 @@ import org.apache.iceberg.spark.data.TestHelpers; import org.apache.iceberg.types.Types; import org.apache.spark.sql.Dataset; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -447,11 +447,10 @@ public void testExpireSnapshotsWithDisabledGarbageCollection() { table.newAppend().appendFile(FILE_A).commit(); - AssertHelpers.assertThrows( - "Should complain about expiring snapshots", - ValidationException.class, - "Cannot expire snapshots: GC is disabled", - () -> SparkActions.get().expireSnapshots(table)); + Assertions.assertThatThrownBy(() -> SparkActions.get().expireSnapshots(table)) + .as("Should complain about expiring snapshots") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot expire snapshots: GC is disabled"); } @Test @@ -529,11 +528,13 @@ public void testRetainLastMultipleCalls() { @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", - () -> SparkActions.get().expireSnapshots(table).retainLast(0).execute()); + Assertions.assertThatThrownBy( + () -> SparkActions.get().expireSnapshots(table).retainLast(0).execute()) + .as( + "Should fail retain 0 snapshots " + + "because number of snapshots to retain cannot be zero") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Number of snapshots to retain must be at least 1, cannot be: 0"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java index c94aec6f5c19..10af2454fa4a 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRemoveOrphanFilesAction.java @@ -40,7 +40,6 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Files; import org.apache.iceberg.GenericBlobMetadata; import org.apache.iceberg.GenericStatisticsFile; @@ -755,11 +754,10 @@ public void testGarbageCollectionDisabled() { table.updateProperties().set(TableProperties.GC_ENABLED, "false").commit(); - AssertHelpers.assertThrows( - "Should complain about removing orphan files", - ValidationException.class, - "Cannot delete orphan files: GC is disabled", - () -> SparkActions.get().deleteOrphanFiles(table).execute()); + Assertions.assertThatThrownBy(() -> SparkActions.get().deleteOrphanFiles(table).execute()) + .as("Should complain about removing orphan files") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot delete orphan files: GC is disabled"); } @Test @@ -991,18 +989,18 @@ public void testPathsWithActualFileHavingNoAuthority() { public void testPathsWithEqualSchemes() { List validFiles = Lists.newArrayList("scheme1://bucket1/dir1/dir2/file1"); List actualFiles = Lists.newArrayList("scheme2://bucket1/dir1/dir2/file1"); - AssertHelpers.assertThrows( - "Test remove orphan files with equal schemes", - ValidationException.class, - "Conflicting authorities/schemes: [(scheme1, scheme2)]", - () -> - executeTest( - validFiles, - actualFiles, - Lists.newArrayList(), - ImmutableMap.of(), - ImmutableMap.of(), - DeleteOrphanFiles.PrefixMismatchMode.ERROR)); + Assertions.assertThatThrownBy( + () -> + executeTest( + validFiles, + actualFiles, + Lists.newArrayList(), + ImmutableMap.of(), + ImmutableMap.of(), + DeleteOrphanFiles.PrefixMismatchMode.ERROR)) + .as("Test remove orphan files with equal schemes") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Conflicting authorities/schemes: [(scheme1, scheme2)]"); Map equalSchemes = Maps.newHashMap(); equalSchemes.put("scheme1", "scheme"); @@ -1020,18 +1018,18 @@ public void testPathsWithEqualSchemes() { public void testPathsWithEqualAuthorities() { List validFiles = Lists.newArrayList("hdfs://servicename1/dir1/dir2/file1"); List actualFiles = Lists.newArrayList("hdfs://servicename2/dir1/dir2/file1"); - AssertHelpers.assertThrows( - "Test remove orphan files with equal authorities", - ValidationException.class, - "Conflicting authorities/schemes: [(servicename1, servicename2)]", - () -> - executeTest( - validFiles, - actualFiles, - Lists.newArrayList(), - ImmutableMap.of(), - ImmutableMap.of(), - DeleteOrphanFiles.PrefixMismatchMode.ERROR)); + Assertions.assertThatThrownBy( + () -> + executeTest( + validFiles, + actualFiles, + Lists.newArrayList(), + ImmutableMap.of(), + ImmutableMap.of(), + DeleteOrphanFiles.PrefixMismatchMode.ERROR)) + .as("Test remove orphan files with equal authorities") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Conflicting authorities/schemes: [(servicename1, servicename2)]"); Map equalAuthorities = Maps.newHashMap(); equalAuthorities.put("servicename1", "servicename"); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java index cfa1c9da951e..4159691a64f6 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java @@ -47,7 +47,6 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.ContentFile; import org.apache.iceberg.DataFile; import org.apache.iceberg.DeleteFile; @@ -643,10 +642,9 @@ public void testSingleCommitWithRewriteFailure() { .when(spyRewrite) .rewriteFiles(any(), argThat(failGroup)); - AssertHelpers.assertThrows( - "Should fail entire rewrite if part fails", - RuntimeException.class, - () -> spyRewrite.execute()); + Assertions.assertThatThrownBy(() -> spyRewrite.execute()) + .as("Should fail entire rewrite if part fails") + .isInstanceOf(RuntimeException.class); table.refresh(); @@ -678,10 +676,9 @@ public void testSingleCommitWithCommitFailure() { doReturn(util).when(spyRewrite).commitManager(table.currentSnapshot().snapshotId()); - AssertHelpers.assertThrows( - "Should fail entire rewrite if commit fails", - RuntimeException.class, - () -> spyRewrite.execute()); + Assertions.assertThatThrownBy(() -> spyRewrite.execute()) + .as("Should fail entire rewrite if commit fails") + .isInstanceOf(RuntimeException.class); table.refresh(); @@ -714,10 +711,9 @@ public void testParallelSingleCommitWithRewriteFailure() { .when(spyRewrite) .rewriteFiles(any(), argThat(failGroup)); - AssertHelpers.assertThrows( - "Should fail entire rewrite if part fails", - RuntimeException.class, - () -> spyRewrite.execute()); + Assertions.assertThatThrownBy(() -> spyRewrite.execute()) + .as("Should fail entire rewrite if part fails") + .isInstanceOf(RuntimeException.class); table.refresh(); @@ -859,32 +855,31 @@ public void testParallelPartialProgressWithCommitFailure() { public void testInvalidOptions() { Table table = createTable(20); - AssertHelpers.assertThrows( - "No negative values for partial progress max commits", - IllegalArgumentException.class, - () -> - basicRewrite(table) - .option(RewriteDataFiles.PARTIAL_PROGRESS_ENABLED, "true") - .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_COMMITS, "-5") - .execute()); - - AssertHelpers.assertThrows( - "No negative values for max concurrent groups", - IllegalArgumentException.class, - () -> - basicRewrite(table) - .option(RewriteDataFiles.MAX_CONCURRENT_FILE_GROUP_REWRITES, "-5") - .execute()); - - AssertHelpers.assertThrows( - "No unknown options allowed", - IllegalArgumentException.class, - () -> basicRewrite(table).option("foobarity", "-5").execute()); - - AssertHelpers.assertThrows( - "Cannot set rewrite-job-order to foo", - IllegalArgumentException.class, - () -> basicRewrite(table).option(RewriteDataFiles.REWRITE_JOB_ORDER, "foo").execute()); + Assertions.assertThatThrownBy( + () -> + basicRewrite(table) + .option(RewriteDataFiles.PARTIAL_PROGRESS_ENABLED, "true") + .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_COMMITS, "-5") + .execute()) + .as("No negative values for partial progress max commits") + .isInstanceOf(IllegalArgumentException.class); + + Assertions.assertThatThrownBy( + () -> + basicRewrite(table) + .option(RewriteDataFiles.MAX_CONCURRENT_FILE_GROUP_REWRITES, "-5") + .execute()) + .as("No negative values for max concurrent groups") + .isInstanceOf(IllegalArgumentException.class); + + Assertions.assertThatThrownBy(() -> basicRewrite(table).option("foobarity", "-5").execute()) + .as("No unknown options allowed") + .isInstanceOf(IllegalArgumentException.class); + + Assertions.assertThatThrownBy( + () -> basicRewrite(table).option(RewriteDataFiles.REWRITE_JOB_ORDER, "foo").execute()) + .as("Cannot set rewrite-job-order to foo") + .isInstanceOf(IllegalArgumentException.class); } @Test @@ -1120,10 +1115,9 @@ public void testCommitStateUnknownException() { doReturn(util).when(spyAction).commitManager(table.currentSnapshot().snapshotId()); - AssertHelpers.assertThrows( - "Should propagate CommitStateUnknown Exception", - CommitStateUnknownException.class, - () -> spyAction.execute()); + Assertions.assertThatThrownBy(() -> spyAction.execute()) + .as("Should propagate CommitStateUnknown Exception") + .isInstanceOf(CommitStateUnknownException.class); List postRewriteData = currentData(); assertEquals("We shouldn't have changed the data", originalData, postRewriteData); @@ -1239,23 +1233,20 @@ public void testInvalidAPIUsage() { SortOrder sortOrder = SortOrder.builderFor(table.schema()).asc("c2").build(); - AssertHelpers.assertThrows( - "Should be unable to set Strategy more than once", - IllegalArgumentException.class, - "Must use only one rewriter type", - () -> actions().rewriteDataFiles(table).binPack().sort()); - - AssertHelpers.assertThrows( - "Should be unable to set Strategy more than once", - IllegalArgumentException.class, - "Must use only one rewriter type", - () -> actions().rewriteDataFiles(table).sort(sortOrder).binPack()); - - AssertHelpers.assertThrows( - "Should be unable to set Strategy more than once", - IllegalArgumentException.class, - "Must use only one rewriter type", - () -> actions().rewriteDataFiles(table).sort(sortOrder).binPack()); + Assertions.assertThatThrownBy(() -> actions().rewriteDataFiles(table).binPack().sort()) + .as("Should be unable to set Strategy more than once") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Must use only one rewriter type"); + + Assertions.assertThatThrownBy(() -> actions().rewriteDataFiles(table).sort(sortOrder).binPack()) + .as("Should be unable to set Strategy more than once") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Must use only one rewriter type"); + + Assertions.assertThatThrownBy(() -> actions().rewriteDataFiles(table).sort(sortOrder).binPack()) + .as("Should be unable to set Strategy more than once") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Must use only one rewriter type"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java index a5dd0054da25..339bb1db328e 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteManifestsAction.java @@ -35,7 +35,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.ManifestFile; import org.apache.iceberg.PartitionSpec; @@ -59,6 +58,7 @@ import org.apache.spark.sql.Encoders; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.TableIdentifier; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -231,11 +231,12 @@ public void testRewriteManifestsWithCommitStateUnknownException() { Table spyTable = spy(table); when(spyTable.rewriteManifests()).thenReturn(spyNewRewriteManifests); - AssertHelpers.assertThrowsCause( - "Should throw a Commit State Unknown Exception", - RuntimeException.class, - "Datacenter on Fire", - () -> actions.rewriteManifests(spyTable).rewriteIf(manifest -> true).execute()); + Assertions.assertThatThrownBy( + () -> actions.rewriteManifests(spyTable).rewriteIf(manifest -> true).execute()) + .as("Should throw a Commit State Unknown Exception") + .cause() + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Datacenter on Fire"); table.refresh(); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/data/parquet/vectorized/TestParquetVectorizedReads.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/data/parquet/vectorized/TestParquetVectorizedReads.java index 53099eefa40c..866bbd240aa5 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/data/parquet/vectorized/TestParquetVectorizedReads.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/data/parquet/vectorized/TestParquetVectorizedReads.java @@ -27,7 +27,6 @@ import java.security.SecureRandom; import java.util.Iterator; import org.apache.avro.generic.GenericData; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Files; import org.apache.iceberg.Schema; import org.apache.iceberg.io.CloseableIterable; @@ -49,6 +48,7 @@ import org.apache.parquet.schema.MessageType; import org.apache.parquet.schema.Type; import org.apache.spark.sql.vectorized.ColumnarBatch; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Assume; import org.junit.Ignore; @@ -255,18 +255,18 @@ public void testMixedTypes() {} @Test @Override public void testNestedStruct() { - AssertHelpers.assertThrows( - "Vectorized reads are not supported yet for struct fields", - UnsupportedOperationException.class, - "Vectorized reads are not supported yet for struct fields", - () -> - VectorizedSparkParquetReaders.buildReader( - TypeUtil.assignIncreasingFreshIds( - new Schema(required(1, "struct", SUPPORTED_PRIMITIVES))), - new MessageType( - "struct", new GroupType(Type.Repetition.OPTIONAL, "struct").withId(1)), - Maps.newHashMap(), - null)); + Assertions.assertThatThrownBy( + () -> + VectorizedSparkParquetReaders.buildReader( + TypeUtil.assignIncreasingFreshIds( + new Schema(required(1, "struct", SUPPORTED_PRIMITIVES))), + new MessageType( + "struct", new GroupType(Type.Repetition.OPTIONAL, "struct").withId(1)), + Maps.newHashMap(), + null)) + .as("Vectorized reads are not supported yet for struct fields") + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("Vectorized reads are not supported yet for struct fields"); } @Test @@ -390,13 +390,10 @@ public void testUnsupportedReadsForParquetV2() throws Exception { try (FileAppender writer = parquetV2Writer(schema, dataFile)) { writer.addAll(data); } - AssertHelpers.assertThrows( - "Vectorized reads not supported", - UnsupportedOperationException.class, - "Cannot support vectorized reads for column", - () -> { - assertRecordsMatch(schema, 30000, data, dataFile, true, BATCH_SIZE); - return null; - }); + Assertions.assertThatThrownBy( + () -> assertRecordsMatch(schema, 30000, data, dataFile, true, BATCH_SIZE)) + .as("Vectorized reads not supported") + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("Cannot support vectorized reads for column"); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java index a9b4f0d3ad2f..f57c87e8b809 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataFrameWriterV2.java @@ -19,7 +19,6 @@ package org.apache.iceberg.spark.source; import java.util.List; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.TableProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.spark.Spark3Util; @@ -33,6 +32,7 @@ import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.catalyst.parser.ParseException; import org.apache.spark.sql.internal.SQLConf; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -76,18 +76,18 @@ public void testMergeSchemaFailsWithoutWriterOption() throws Exception { // this has a different error message than the case without accept-any-schema because it uses // Iceberg checks - AssertHelpers.assertThrows( - "Should fail when merge-schema is not enabled on the writer", - IllegalArgumentException.class, - "Field new_col not found in source schema", - () -> { - try { - threeColDF.writeTo(tableName).append(); - } catch (NoSuchTableException e) { - // needed because append has checked exceptions - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + threeColDF.writeTo(tableName).append(); + } catch (NoSuchTableException e) { + // needed because append has checked exceptions + throw new RuntimeException(e); + } + }) + .as("Should fail when merge-schema is not enabled on the writer") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Field new_col not found in source schema"); } @Test @@ -111,18 +111,18 @@ public void testMergeSchemaWithoutAcceptAnySchema() throws Exception { "{ \"id\": 3, \"data\": \"c\", \"new_col\": 12.06 }", "{ \"id\": 4, \"data\": \"d\", \"new_col\": 14.41 }"); - AssertHelpers.assertThrows( - "Should fail when accept-any-schema is not enabled on the table", - AnalysisException.class, - "too many data columns", - () -> { - try { - threeColDF.writeTo(tableName).option("merge-schema", "true").append(); - } catch (NoSuchTableException e) { - // needed because append has checked exceptions - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + threeColDF.writeTo(tableName).option("merge-schema", "true").append(); + } catch (NoSuchTableException e) { + // needed because append has checked exceptions + throw new RuntimeException(e); + } + }) + .as("Should fail when accept-any-schema is not enabled on the table") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("too many data columns"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java index 7e254960e759..2c3a7aba59d1 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestDataSourceOptions.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.FileFormat; import org.apache.iceberg.FileScanTask; @@ -246,50 +245,52 @@ public void testIncrementalScanOptions() throws IOException { List snapshotIds = SnapshotUtil.currentAncestorIds(table); // start-snapshot-id and snapshot-id are both configured. - AssertHelpers.assertThrows( - "Check both start-snapshot-id and snapshot-id are configured", - IllegalArgumentException.class, - "Cannot set start-snapshot-id and end-snapshot-id for incremental scans", - () -> { - spark - .read() - .format("iceberg") - .option("snapshot-id", snapshotIds.get(3).toString()) - .option("start-snapshot-id", snapshotIds.get(3).toString()) - .load(tableLocation) - .explain(); - }); + Assertions.assertThatThrownBy( + () -> { + spark + .read() + .format("iceberg") + .option("snapshot-id", snapshotIds.get(3).toString()) + .option("start-snapshot-id", snapshotIds.get(3).toString()) + .load(tableLocation) + .explain(); + }) + .as("Check both start-snapshot-id and snapshot-id are configured") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Cannot set start-snapshot-id and end-snapshot-id for incremental scans"); // end-snapshot-id and as-of-timestamp are both configured. - AssertHelpers.assertThrows( - "Check both start-snapshot-id and snapshot-id are configured", - IllegalArgumentException.class, - "Cannot set start-snapshot-id and end-snapshot-id for incremental scans", - () -> { - spark - .read() - .format("iceberg") - .option( - SparkReadOptions.AS_OF_TIMESTAMP, - Long.toString(table.snapshot(snapshotIds.get(3)).timestampMillis())) - .option("end-snapshot-id", snapshotIds.get(2).toString()) - .load(tableLocation) - .explain(); - }); + Assertions.assertThatThrownBy( + () -> { + spark + .read() + .format("iceberg") + .option( + SparkReadOptions.AS_OF_TIMESTAMP, + Long.toString(table.snapshot(snapshotIds.get(3)).timestampMillis())) + .option("end-snapshot-id", snapshotIds.get(2).toString()) + .load(tableLocation) + .explain(); + }) + .as("Check both start-snapshot-id and snapshot-id are configured") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Cannot set start-snapshot-id and end-snapshot-id for incremental scans"); // only end-snapshot-id is configured. - AssertHelpers.assertThrows( - "Check both start-snapshot-id and snapshot-id are configured", - IllegalArgumentException.class, - "Cannot set only end-snapshot-id for incremental scans", - () -> { - spark - .read() - .format("iceberg") - .option("end-snapshot-id", snapshotIds.get(2).toString()) - .load(tableLocation) - .explain(); - }); + Assertions.assertThatThrownBy( + () -> { + spark + .read() + .format("iceberg") + .option("end-snapshot-id", snapshotIds.get(2).toString()) + .load(tableLocation) + .explain(); + }) + .as("Check both start-snapshot-id and snapshot-id are configured") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot set only end-snapshot-id for incremental scans"); // test (1st snapshot, current snapshot] incremental scan. List result = diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java index 6ab9e57949e3..8630c50151f3 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestForwardCompatibility.java @@ -28,7 +28,6 @@ import java.util.concurrent.TimeoutException; import org.apache.avro.generic.GenericData; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.FileFormat; @@ -55,6 +54,7 @@ import org.apache.spark.sql.execution.streaming.MemoryStream; import org.apache.spark.sql.streaming.StreamingQuery; import org.apache.spark.sql.streaming.StreamingQueryException; +import org.assertj.core.api.Assertions; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -120,16 +120,16 @@ public void testSparkWriteFailsUnknownTransform() throws IOException { Dataset df = spark.createDataFrame(expected, SimpleRecord.class); - AssertHelpers.assertThrows( - "Should reject write with unsupported transform", - UnsupportedOperationException.class, - "Cannot write using unsupported transforms: zero", - () -> - df.select("id", "data") - .write() - .format("iceberg") - .mode("append") - .save(location.toString())); + Assertions.assertThatThrownBy( + () -> + df.select("id", "data") + .write() + .format("iceberg") + .mode("append") + .save(location.toString())) + .as("Should reject write with unsupported transform") + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("Cannot write using unsupported transforms: zero"); } @Test @@ -159,11 +159,10 @@ public void testSparkStreamingWriteFailsUnknownTransform() throws IOException, T List batch1 = Lists.newArrayList(1, 2); send(batch1, inputStream); - AssertHelpers.assertThrows( - "Should reject streaming write with unsupported transform", - StreamingQueryException.class, - "Cannot write using unsupported transforms: zero", - query::processAllAvailable); + Assertions.assertThatThrownBy(query::processAllAvailable) + .as("Should reject streaming write with unsupported transform") + .isInstanceOf(StreamingQueryException.class) + .hasMessageContaining("Cannot write using unsupported transforms: zero"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java index 111da882fe8a..079ca5f77fa4 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java @@ -38,7 +38,6 @@ import java.util.stream.StreamSupport; import org.apache.avro.generic.GenericData; import org.apache.avro.generic.GenericRecordBuilder; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DeleteFile; import org.apache.iceberg.Files; @@ -1119,17 +1118,17 @@ public void testPruneManifestsTable() { if (!spark.version().startsWith("2")) { // Spark 2 isn't able to actually push down nested struct projections so this will not break - AssertHelpers.assertThrows( - "Can't prune struct inside list", - SparkException.class, - "Cannot project a partial list element struct", - () -> - spark - .read() - .format("iceberg") - .load(loadLocation(tableIdentifier, "manifests")) - .select("partition_spec_id", "path", "partition_summaries.contains_null") - .collectAsList()); + Assertions.assertThatThrownBy( + () -> + spark + .read() + .format("iceberg") + .load(loadLocation(tableIdentifier, "manifests")) + .select("partition_spec_id", "path", "partition_summaries.contains_null") + .collectAsList()) + .as("Can't prune struct inside list") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Cannot project a partial list element struct"); } Dataset actualDf = diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java index 0baaef1374d4..9c5d48e1debb 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTablesWithPartitionEvolution.java @@ -33,7 +33,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.FileFormat; import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.MetadataTableType; @@ -54,6 +53,7 @@ import org.apache.spark.sql.catalyst.parser.ParseException; import org.apache.spark.sql.types.DataType; import org.apache.spark.sql.types.StructType; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -641,11 +641,10 @@ public void testMetadataTablesWithUnknownTransforms() { sql("REFRESH TABLE %s", tableName); for (MetadataTableType tableType : Arrays.asList(FILES, ALL_DATA_FILES, ENTRIES, ALL_ENTRIES)) { - AssertHelpers.assertThrows( - "Should complain about the partition type", - ValidationException.class, - "Cannot build table partition type, unknown transforms", - () -> loadMetadataTable(tableType)); + Assertions.assertThatThrownBy(() -> loadMetadataTable(tableType)) + .as("Should complain about the partition type") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot build table partition type, unknown transforms"); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestRequiredDistributionAndOrdering.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestRequiredDistributionAndOrdering.java index 8e54a23f815a..e8e402ac44cc 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestRequiredDistributionAndOrdering.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestRequiredDistributionAndOrdering.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.TableProperties; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; @@ -30,6 +29,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Test; @@ -163,20 +163,20 @@ public void testDisabledDistributionAndOrdering() { Dataset inputDF = ds.coalesce(1).sortWithinPartitions("c1"); // should fail if ordering is disabled - AssertHelpers.assertThrows( - "Should reject writes without ordering", - SparkException.class, - "Writing job aborted", - () -> { - try { - inputDF - .writeTo(tableName) - .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") - .append(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + inputDF + .writeTo(tableName) + .option(SparkWriteOptions.USE_TABLE_DISTRIBUTION_AND_ORDERING, "false") + .append(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Should reject writes without ordering") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted"); } @Test @@ -233,17 +233,17 @@ public void testNoSortBucketTransformsWithoutExtensions() throws NoSuchTableExce Dataset inputDF = ds.coalesce(1).sortWithinPartitions("c1"); // should fail by default as extensions are disabled - AssertHelpers.assertThrows( - "Should reject writes without ordering", - SparkException.class, - "Writing job aborted", - () -> { - try { - inputDF.writeTo(tableName).append(); - } catch (NoSuchTableException e) { - throw new RuntimeException(e); - } - }); + Assertions.assertThatThrownBy( + () -> { + try { + inputDF.writeTo(tableName).append(); + } catch (NoSuchTableException e) { + throw new RuntimeException(e); + } + }) + .as("Should reject writes without ordering") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted"); inputDF.writeTo(tableName).option(SparkWriteOptions.FANOUT_ENABLED, "true").append(); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkDataWrite.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkDataWrite.java index 551dc961e309..3d759759f608 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkDataWrite.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkDataWrite.java @@ -30,7 +30,6 @@ import java.util.Map; import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.AppendFiles; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.FileFormat; import org.apache.iceberg.ManifestFile; @@ -691,20 +690,21 @@ public void testCommitUnknownException() throws IOException { ManualSource.setTable(manualTableName, sparkTable); // Although an exception is thrown here, write and commit have succeeded - AssertHelpers.assertThrowsWithCause( - "Should throw a Commit State Unknown Exception", - SparkException.class, - "Writing job aborted", - CommitStateUnknownException.class, - "Datacenter on Fire", - () -> - df2.select("id", "data") - .sort("data") - .write() - .format("org.apache.iceberg.spark.source.ManualSource") - .option(ManualSource.TABLE_NAME, manualTableName) - .mode(SaveMode.Append) - .save(targetLocation)); + Assertions.assertThatThrownBy( + () -> + df2.select("id", "data") + .sort("data") + .write() + .format("org.apache.iceberg.spark.source.ManualSource") + .option(ManualSource.TABLE_NAME, manualTableName) + .mode(SaveMode.Append) + .save(targetLocation)) + .as("Should throw a Commit State Unknown Exception") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Writing job aborted") + .cause() + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageContaining("Datacenter on Fire"); // Since write and commit succeeded, the rows should be readable Dataset result = spark.read().format("iceberg").load(targetLocation); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkMetadataColumns.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkMetadataColumns.java index 5c7929112fb9..2b0033fdcbb5 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkMetadataColumns.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestSparkMetadataColumns.java @@ -30,7 +30,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.FileFormat; import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.MetadataColumns; @@ -53,6 +52,7 @@ import org.apache.spark.sql.Encoders; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -224,11 +224,10 @@ public void testPartitionMetadataColumnWithUnknownTransforms() { TableMetadata base = ops.current(); ops.commit(base, base.updatePartitionSpec(UNKNOWN_SPEC)); - AssertHelpers.assertThrows( - "Should fail to query the partition metadata column", - ValidationException.class, - "Cannot build table partition type, unknown transforms", - () -> sql("SELECT _partition FROM %s", TABLE_NAME)); + Assertions.assertThatThrownBy(() -> sql("SELECT _partition FROM %s", TABLE_NAME)) + .as("Should fail to query the partition metadata column") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot build table partition type, unknown transforms"); } @Test @@ -246,11 +245,10 @@ public void testConflictingColumns() { ImmutableList.of(row(1L, "a1")), sql("SELECT id, category FROM %s", TABLE_NAME)); - AssertHelpers.assertThrows( - "Should fail to query conflicting columns", - ValidationException.class, - "column names conflict", - () -> sql("SELECT * FROM %s", TABLE_NAME)); + Assertions.assertThatThrownBy(() -> sql("SELECT * FROM %s", TABLE_NAME)) + .as("Should fail to query conflicting columns") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("column names conflict"); table.refresh(); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java index de94a7c8bf8b..7a4c25607850 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java @@ -29,7 +29,6 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.IntStream; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseTable; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; @@ -495,11 +494,12 @@ public void testReadStreamWithSnapshotTypeOverwriteErrorsOut() throws Exception StreamingQuery query = startStream(); - AssertHelpers.assertThrowsCause( - "Streaming should fail with IllegalStateException, as the snapshot is not of type APPEND", - IllegalStateException.class, - "Cannot process overwrite snapshot", - () -> query.processAllAvailable()); + Assertions.assertThatThrownBy(() -> query.processAllAvailable()) + .as( + "Streaming should fail with IllegalStateException, as the snapshot is not of type APPEND") + .cause() + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot process overwrite snapshot"); } @Test @@ -536,11 +536,12 @@ public void testReadStreamWithSnapshotTypeDeleteErrorsOut() throws Exception { StreamingQuery query = startStream(); - AssertHelpers.assertThrowsCause( - "Streaming should fail with IllegalStateException, as the snapshot is not of type APPEND", - IllegalStateException.class, - "Cannot process delete snapshot", - () -> query.processAllAvailable()); + Assertions.assertThatThrownBy(() -> query.processAllAvailable()) + .as( + "Streaming should fail with IllegalStateException, as the snapshot is not of type APPEND") + .cause() + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot process delete snapshot"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestTimestampWithoutZone.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestTimestampWithoutZone.java index 053f6dbaea46..d85e1954950d 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestTimestampWithoutZone.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestTimestampWithoutZone.java @@ -28,7 +28,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.FileFormat; @@ -53,6 +52,7 @@ import org.apache.spark.sql.Row; import org.apache.spark.sql.SaveMode; import org.apache.spark.sql.SparkSession; +import org.assertj.core.api.Assertions; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; @@ -159,21 +159,22 @@ public void testUnpartitionedTimestampWithoutZoneProjection() { @Test public void testUnpartitionedTimestampWithoutZoneError() { - AssertHelpers.assertThrows( - String.format( - "Read operation performed on a timestamp without timezone field while " - + "'%s' set to false should throw exception", - SparkReadOptions.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE), - IllegalArgumentException.class, - SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR, - () -> - spark - .read() - .format("iceberg") - .option(SparkReadOptions.VECTORIZATION_ENABLED, String.valueOf(vectorized)) - .option(SparkReadOptions.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE, "false") - .load(unpartitioned.toString()) - .collectAsList()); + Assertions.assertThatThrownBy( + () -> + spark + .read() + .format("iceberg") + .option(SparkReadOptions.VECTORIZATION_ENABLED, String.valueOf(vectorized)) + .option(SparkReadOptions.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE, "false") + .load(unpartitioned.toString()) + .collectAsList()) + .as( + String.format( + "Read operation performed on a timestamp without timezone field while " + + "'%s' set to false should throw exception", + SparkReadOptions.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR); } @Test @@ -217,11 +218,10 @@ public void testUnpartitionedTimestampWithoutZoneWriteError() { .mode(SaveMode.Append) .save(unpartitioned.toString()); - AssertHelpers.assertThrows( - errorMessage, - IllegalArgumentException.class, - SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR, - writeOperation); + Assertions.assertThatThrownBy(writeOperation::run) + .as(errorMessage) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java index 9bf00f1b1365..291b8d27539c 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import org.apache.hadoop.conf.Configuration; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.FileScanTask; import org.apache.iceberg.PartitionSpec; @@ -49,6 +48,7 @@ import org.apache.spark.sql.SaveMode; import org.apache.spark.sql.SparkSession; import org.apache.spark.sql.catalyst.InternalRow; +import org.assertj.core.api.Assertions; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -231,11 +231,10 @@ public void testBadCustomMetricCollectionForParquet() throws IOException { properties.put(TableProperties.DEFAULT_WRITE_METRICS_MODE, "counts"); properties.put("write.metadata.metrics.column.ids", "full"); - AssertHelpers.assertThrows( - "Creating a table with invalid metrics should fail", - ValidationException.class, - null, - () -> tables.create(SIMPLE_SCHEMA, spec, properties, tableLocation)); + Assertions.assertThatThrownBy( + () -> tables.create(SIMPLE_SCHEMA, spec, properties, tableLocation)) + .as("Creating a table with invalid metrics should fail") + .isInstanceOf(ValidationException.class); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java index e347cde7ba32..ec6b522fec21 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java @@ -19,7 +19,6 @@ package org.apache.iceberg.spark.sql; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.hadoop.HadoopCatalog; @@ -28,6 +27,7 @@ import org.apache.iceberg.types.Types.NestedField; import org.apache.spark.SparkException; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -55,11 +55,10 @@ public void removeTable() { @Test public void testAddColumnNotNull() { - AssertHelpers.assertThrows( - "Should reject adding NOT NULL column", - SparkException.class, - "Incompatible change: cannot add required column", - () -> sql("ALTER TABLE %s ADD COLUMN c3 INT NOT NULL", tableName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s ADD COLUMN c3 INT NOT NULL", tableName)) + .as("Should reject adding NOT NULL column") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Incompatible change: cannot add required column"); } @Test @@ -156,11 +155,10 @@ public void testAddColumnWithMap() { validationCatalog.loadTable(tableIdent).schema().asStruct()); // should not allow changing map key column - AssertHelpers.assertThrows( - "Should reject changing key of the map column", - SparkException.class, - "Unsupported table change: Cannot add fields to map keys:", - () -> sql("ALTER TABLE %s ADD COLUMN data2.key.y int", tableName)); + Assertions.assertThatThrownBy(() -> sql("ALTER TABLE %s ADD COLUMN data2.key.y int", tableName)) + .as("Should reject changing key of the map column") + .isInstanceOf(SparkException.class) + .hasMessageContaining("Unsupported table change: Cannot add fields to map keys:"); } @Test @@ -253,11 +251,11 @@ public void testAlterColumnSetNotNull() { expectedSchema, validationCatalog.loadTable(tableIdent).schema().asStruct()); - AssertHelpers.assertThrows( - "Should reject adding NOT NULL constraint to an optional column", - AnalysisException.class, - "Cannot change nullable column to non-nullable: data", - () -> sql("ALTER TABLE %s ALTER COLUMN data SET NOT NULL", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s ALTER COLUMN data SET NOT NULL", tableName)) + .as("Should reject adding NOT NULL constraint to an optional column") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Cannot change nullable column to non-nullable: data"); } @Test @@ -323,9 +321,9 @@ public void testSetTableProperties() { "Should not have the removed table property", validationCatalog.loadTable(tableIdent).properties().get("prop")); - AssertHelpers.assertThrows( - "Cannot specify the 'sort-order' because it's a reserved table property", - UnsupportedOperationException.class, - () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('sort-order'='value')", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('sort-order'='value')", tableName)) + .as("Cannot specify the 'sort-order' because it's a reserved table property") + .isInstanceOf(UnsupportedOperationException.class); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java index 3a62361590d1..f4bef9cec155 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestCreateTable.java @@ -21,7 +21,6 @@ import java.io.File; import java.util.Map; import java.util.UUID; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseTable; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Schema; @@ -168,14 +167,14 @@ public void testCreateTableUsingParquet() { "parquet", table.properties().get(TableProperties.DEFAULT_FILE_FORMAT)); - AssertHelpers.assertThrows( - "Should reject unsupported format names", - IllegalArgumentException.class, - "Unsupported format in USING: crocodile", - () -> - sql( - "CREATE TABLE %s.default.fail (id BIGINT NOT NULL, data STRING) USING crocodile", - catalogName)); + Assertions.assertThatThrownBy( + () -> + sql( + "CREATE TABLE %s.default.fail (id BIGINT NOT NULL, data STRING) USING crocodile", + catalogName)) + .as("Should reject unsupported format names") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Unsupported format in USING: crocodile"); } @Test @@ -382,10 +381,11 @@ public void testDowngradeTableToFormatV1ThroughTablePropertyFails() { TableOperations ops = ((BaseTable) table).operations(); Assert.assertEquals("should create table using format v2", 2, ops.refresh().formatVersion()); - AssertHelpers.assertThrowsCause( - "should fail to downgrade to v1", - IllegalArgumentException.class, - "Cannot downgrade v2 table to v1", - () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('format-version'='1')", tableName)); + Assertions.assertThatThrownBy( + () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('format-version'='1')", tableName)) + .as("should fail to downgrade to v1") + .cause() + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot downgrade v2 table to v1"); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDeleteFrom.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDeleteFrom.java index cae1901aa713..55031188453a 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDeleteFrom.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDeleteFrom.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; @@ -30,6 +29,7 @@ import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -86,11 +86,11 @@ public void testDeleteFromTableAtSnapshot() throws NoSuchTableException { long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId(); String prefix = "snapshot_id_"; - AssertHelpers.assertThrows( - "Should not be able to delete from a table at a specific snapshot", - IllegalArgumentException.class, - "Cannot delete from table at a specific snapshot", - () -> sql("DELETE FROM %s.%s WHERE id < 4", tableName, prefix + snapshotId)); + Assertions.assertThatThrownBy( + () -> sql("DELETE FROM %s.%s WHERE id < 4", tableName, prefix + snapshotId)) + .as("Should not be able to delete from a table at a specific snapshot") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot delete from table at a specific snapshot"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDropTable.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDropTable.java index 2189bd0dae75..c5c39e5d782f 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDropTable.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestDropTable.java @@ -24,12 +24,12 @@ import java.util.stream.Collectors; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.MetadataTableType; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.Streams; import org.apache.iceberg.spark.SparkCatalogTestBase; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -116,11 +116,11 @@ public void testPurgeTableGCDisabled() throws IOException { "There totally should have 2 files for manifests and files", 2, manifestAndFiles.size()); Assert.assertTrue("All files should be existed", checkFilesExist(manifestAndFiles, true)); - AssertHelpers.assertThrows( - "Purge table is not allowed when GC is disabled", - ValidationException.class, - "Cannot purge table: GC is disabled (deleting files may corrupt other tables", - () -> sql("DROP TABLE %s PURGE", tableName)); + Assertions.assertThatThrownBy(() -> sql("DROP TABLE %s PURGE", tableName)) + .as("Purge table is not allowed when GC is disabled") + .isInstanceOf(ValidationException.class) + .hasMessageContaining( + "Cannot purge table: GC is disabled (deleting files may corrupt other tables"); Assert.assertTrue("Table should not been dropped", validationCatalog.tableExists(tableIdent)); Assert.assertTrue("All files should not be deleted", checkFilesExist(manifestAndFiles, true)); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestNamespaceSQL.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestNamespaceSQL.java index 6c29ea4442ef..77b564cd001b 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestNamespaceSQL.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestNamespaceSQL.java @@ -23,13 +23,13 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; import org.apache.iceberg.spark.SparkCatalogTestBase; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -103,11 +103,10 @@ public void testDropNonEmptyNamespace() { Assert.assertTrue( "Table should exist", validationCatalog.tableExists(TableIdentifier.of(NS, "table"))); - AssertHelpers.assertThrows( - "Should fail if trying to delete a non-empty namespace", - NamespaceNotEmptyException.class, - "Namespace db is not empty.", - () -> sql("DROP NAMESPACE %s", fullNamespace)); + Assertions.assertThatThrownBy(() -> sql("DROP NAMESPACE %s", fullNamespace)) + .as("Should fail if trying to delete a non-empty namespace") + .isInstanceOf(NamespaceNotEmptyException.class) + .hasMessageContaining("Namespace db is not empty."); sql("DROP TABLE %s.table", fullNamespace); } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java index 2265dec4763c..ea97949d1327 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.events.Listeners; import org.apache.iceberg.events.ScanEvent; @@ -396,48 +395,48 @@ public void testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() { sql("INSERT INTO %s VALUES (4, 'd', 4.0), (5, 'e', 5.0)", tableName); // using snapshot in table identifier and VERSION AS OF - AssertHelpers.assertThrows( - "Cannot do time-travel based on both table identifier and AS OF", - IllegalArgumentException.class, - "Cannot do time-travel based on both table identifier and AS OF", - () -> { - sql( - "SELECT * FROM %s.%s VERSION AS OF %s", - tableName, snapshotPrefix + snapshotId, snapshotId); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "SELECT * FROM %s.%s VERSION AS OF %s", + tableName, snapshotPrefix + snapshotId, snapshotId); + }) + .as("Cannot do time-travel based on both table identifier and AS OF") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot do time-travel based on both table identifier and AS OF"); // using snapshot in table identifier and TIMESTAMP AS OF - AssertHelpers.assertThrows( - "Cannot do time-travel based on both table identifier and AS OF", - IllegalArgumentException.class, - "Cannot do time-travel based on both table identifier and AS OF", - () -> { - sql( - "SELECT * FROM %s.%s VERSION AS OF %s", - tableName, timestampPrefix + timestamp, snapshotId); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "SELECT * FROM %s.%s VERSION AS OF %s", + tableName, timestampPrefix + timestamp, snapshotId); + }) + .as("Cannot do time-travel based on both table identifier and AS OF") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot do time-travel based on both table identifier and AS OF"); // using timestamp in table identifier and VERSION AS OF - AssertHelpers.assertThrows( - "Cannot do time-travel based on both table identifier and AS OF", - IllegalArgumentException.class, - "Cannot do time-travel based on both table identifier and AS OF", - () -> { - sql( - "SELECT * FROM %s.%s TIMESTAMP AS OF %s", - tableName, snapshotPrefix + snapshotId, timestamp); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "SELECT * FROM %s.%s TIMESTAMP AS OF %s", + tableName, snapshotPrefix + snapshotId, timestamp); + }) + .as("Cannot do time-travel based on both table identifier and AS OF") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot do time-travel based on both table identifier and AS OF"); // using timestamp in table identifier and TIMESTAMP AS OF - AssertHelpers.assertThrows( - "Cannot do time-travel based on both table identifier and AS OF", - IllegalArgumentException.class, - "Cannot do time-travel based on both table identifier and AS OF", - () -> { - sql( - "SELECT * FROM %s.%s TIMESTAMP AS OF %s", - tableName, timestampPrefix + timestamp, timestamp); - }); + Assertions.assertThatThrownBy( + () -> { + sql( + "SELECT * FROM %s.%s TIMESTAMP AS OF %s", + tableName, timestampPrefix + timestamp, timestamp); + }) + .as("Cannot do time-travel based on both table identifier and AS OF") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot do time-travel based on both table identifier and AS OF"); } @Test @@ -472,21 +471,22 @@ public void testSpecifySnapshotAndTimestamp() { // create a second snapshot sql("INSERT INTO %s VALUES (4, 'd', 4.0), (5, 'e', 5.0)", tableName); - AssertHelpers.assertThrows( - "Should not be able to specify both snapshot id and timestamp", - IllegalArgumentException.class, - String.format( - "Can specify only one of snapshot-id (%s), as-of-timestamp (%s)", - snapshotId, timestamp), - () -> { - spark - .read() - .format("iceberg") - .option(SparkReadOptions.SNAPSHOT_ID, snapshotId) - .option(SparkReadOptions.AS_OF_TIMESTAMP, timestamp) - .load(tableName) - .collectAsList(); - }); + Assertions.assertThatThrownBy( + () -> { + spark + .read() + .format("iceberg") + .option(SparkReadOptions.SNAPSHOT_ID, snapshotId) + .option(SparkReadOptions.AS_OF_TIMESTAMP, timestamp) + .load(tableName) + .collectAsList(); + }) + .as("Should not be able to specify both snapshot id and timestamp") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + String.format( + "Can specify only one of snapshot-id (%s), as-of-timestamp (%s)", + snapshotId, timestamp)); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkBucketFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkBucketFunction.java index c9c8c02b417c..2cf2865fe8bb 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkBucketFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkBucketFunction.java @@ -21,7 +21,6 @@ import java.math.BigDecimal; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.expressions.Literal; import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; @@ -191,101 +190,107 @@ public void testNumBucketsAcceptsShortAndByte() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'bucket' cannot process input: (): Wrong number of inputs (expected numBuckets and value)", - () -> scalarSql("SELECT system.bucket()")); - - AssertHelpers.assertThrows( - "Function resolution should not work with only one argument", - AnalysisException.class, - "Function 'bucket' cannot process input: (int): Wrong number of inputs (expected numBuckets and value)", - () -> scalarSql("SELECT system.bucket(1)")); - - AssertHelpers.assertThrows( - "Function resolution should not work with more than two arguments", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, bigint, int): Wrong number of inputs (expected numBuckets and value)", - () -> scalarSql("SELECT system.bucket(1, 1L, 1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (): Wrong number of inputs (expected numBuckets and value)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(1)")) + .as("Function resolution should not work with only one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int): Wrong number of inputs (expected numBuckets and value)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(1, 1L, 1)")) + .as("Function resolution should not work with more than two arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int, bigint, int): Wrong number of inputs (expected numBuckets and value)"); } @Test public void testInvalidTypesCannotBeUsedForNumberOfBuckets() { - AssertHelpers.assertThrows( - "Decimal type should not be coercible to the number of buckets", - AnalysisException.class, - "Function 'bucket' cannot process input: (decimal(9,2), int): Expected number of buckets to be tinyint, shortint or int", - () -> scalarSql("SELECT system.bucket(CAST('12.34' as DECIMAL(9, 2)), 10)")); - - AssertHelpers.assertThrows( - "Long type should not be coercible to the number of buckets", - AnalysisException.class, - "Function 'bucket' cannot process input: (bigint, int): Expected number of buckets to be tinyint, shortint or int", - () -> scalarSql("SELECT system.bucket(12L, 10)")); - - AssertHelpers.assertThrows( - "String type should not be coercible to the number of buckets", - AnalysisException.class, - "Function 'bucket' cannot process input: (string, int): Expected number of buckets to be tinyint, shortint or int", - () -> scalarSql("SELECT system.bucket('5', 10)")); - - AssertHelpers.assertThrows( - "Interval year to month type should not be coercible to the number of buckets", - AnalysisException.class, - "Function 'bucket' cannot process input: (interval year to month, int): Expected number of buckets to be tinyint, shortint or int", - () -> scalarSql("SELECT system.bucket(INTERVAL '100-00' YEAR TO MONTH, 10)")); - - AssertHelpers.assertThrows( - "Interval day-time type should not be coercible to the number of buckets", - AnalysisException.class, - "Function 'bucket' cannot process input: (interval day to second, int): Expected number of buckets to be tinyint, shortint or int", - () -> scalarSql("SELECT system.bucket(CAST('11 23:4:0' AS INTERVAL DAY TO SECOND), 10)")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.bucket(CAST('12.34' as DECIMAL(9, 2)), 10)")) + .as("Decimal type should not be coercible to the number of buckets") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (decimal(9,2), int): Expected number of buckets to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(12L, 10)")) + .as("Long type should not be coercible to the number of buckets") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (bigint, int): Expected number of buckets to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket('5', 10)")) + .as("String type should not be coercible to the number of buckets") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (string, int): Expected number of buckets to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.bucket(INTERVAL '100-00' YEAR TO MONTH, 10)")) + .as("Interval year to month type should not be coercible to the number of buckets") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (interval year to month, int): Expected number of buckets to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy( + () -> + scalarSql("SELECT system.bucket(CAST('11 23:4:0' AS INTERVAL DAY TO SECOND), 10)")) + .as("Interval day-time type should not be coercible to the number of buckets") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (interval day to second, int): Expected number of buckets to be tinyint, shortint or int"); } @Test public void testInvalidTypesForBucketColumn() { - AssertHelpers.assertThrows( - "Double type should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, float): Expected column to be date, tinyint, smallint, int, bigint, decimal, timestamp, string, or binary", - () -> scalarSql("SELECT system.bucket(10, cast(12.3456 as float))")); - - AssertHelpers.assertThrows( - "Double type should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, double): Expected column to be date, tinyint, smallint, int, bigint, decimal, timestamp, string, or binary", - () -> scalarSql("SELECT system.bucket(10, cast(12.3456 as double))")); - - AssertHelpers.assertThrows( - "Boolean type should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, boolean)", - () -> scalarSql("SELECT system.bucket(10, true)")); - - AssertHelpers.assertThrows( - "Map types should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, map)", - () -> scalarSql("SELECT system.bucket(10, map(1, 1))")); - - AssertHelpers.assertThrows( - "Array types should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, array)", - () -> scalarSql("SELECT system.bucket(10, array(1L))")); - - AssertHelpers.assertThrows( - "Interval year-to-month type should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, interval year to month)", - () -> scalarSql("SELECT system.bucket(10, INTERVAL '100-00' YEAR TO MONTH)")); - - AssertHelpers.assertThrows( - "Interval day-time type should not be bucketable", - AnalysisException.class, - "Function 'bucket' cannot process input: (int, interval day to second)", - () -> scalarSql("SELECT system.bucket(10, CAST('11 23:4:0' AS INTERVAL DAY TO SECOND))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.bucket(10, cast(12.3456 as float))")) + .as("Double type should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int, float): Expected column to be date, tinyint, smallint, int, bigint, decimal, timestamp, string, or binary"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.bucket(10, cast(12.3456 as double))")) + .as("Double type should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int, double): Expected column to be date, tinyint, smallint, int, bigint, decimal, timestamp, string, or binary"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(10, true)")) + .as("Boolean type should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'bucket' cannot process input: (int, boolean)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(10, map(1, 1))")) + .as("Map types should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'bucket' cannot process input: (int, map)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.bucket(10, array(1L))")) + .as("Array types should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'bucket' cannot process input: (int, array)"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.bucket(10, INTERVAL '100-00' YEAR TO MONTH)")) + .as("Interval year-to-month type should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int, interval year to month)"); + + Assertions.assertThatThrownBy( + () -> + scalarSql("SELECT system.bucket(10, CAST('11 23:4:0' AS INTERVAL DAY TO SECOND))")) + .as("Interval day-time type should not be bucketable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'bucket' cannot process input: (int, interval day to second)"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkDaysFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkDaysFunction.java index ccba28735e33..e139d3d000ef 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkDaysFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkDaysFunction.java @@ -19,9 +19,9 @@ package org.apache.iceberg.spark.sql; import java.sql.Date; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -69,31 +69,31 @@ public void testTimestamps() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'days' cannot process input: (): Wrong number of inputs", - () -> scalarSql("SELECT system.days()")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.days()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'days' cannot process input: (): Wrong number of inputs"); - AssertHelpers.assertThrows( - "Function resolution should not work with more than one argument", - AnalysisException.class, - "Function 'days' cannot process input: (date, date): Wrong number of inputs", - () -> scalarSql("SELECT system.days(date('1969-12-31'), date('1969-12-31'))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.days(date('1969-12-31'), date('1969-12-31'))")) + .as("Function resolution should not work with more than one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'days' cannot process input: (date, date): Wrong number of inputs"); } @Test public void testInvalidInputTypes() { - AssertHelpers.assertThrows( - "Int type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'days' cannot process input: (int): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.days(1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.days(1)")) + .as("Int type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'days' cannot process input: (int): Expected value to be date or timestamp"); - AssertHelpers.assertThrows( - "Long type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'days' cannot process input: (bigint): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.days(1L)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.days(1L)")) + .as("Long type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'days' cannot process input: (bigint): Expected value to be date or timestamp"); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkHoursFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkHoursFunction.java index fc0d781318c9..312299ffee09 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkHoursFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkHoursFunction.java @@ -18,9 +18,9 @@ */ package org.apache.iceberg.spark.sql; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.spark.sql.AnalysisException; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -51,31 +51,31 @@ public void testTimestamps() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'hours' cannot process input: (): Wrong number of inputs", - () -> scalarSql("SELECT system.hours()")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.hours()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'hours' cannot process input: (): Wrong number of inputs"); - AssertHelpers.assertThrows( - "Function resolution should not work with more than one argument", - AnalysisException.class, - "Function 'hours' cannot process input: (date, date): Wrong number of inputs", - () -> scalarSql("SELECT system.hours(date('1969-12-31'), date('1969-12-31'))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.hours(date('1969-12-31'), date('1969-12-31'))")) + .as("Function resolution should not work with more than one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'hours' cannot process input: (date, date): Wrong number of inputs"); } @Test public void testInvalidInputTypes() { - AssertHelpers.assertThrows( - "Int type should not be coercible to timestamp", - AnalysisException.class, - "Function 'hours' cannot process input: (int): Expected value to be timestamp", - () -> scalarSql("SELECT system.hours(1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.hours(1)")) + .as("Int type should not be coercible to timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'hours' cannot process input: (int): Expected value to be timestamp"); - AssertHelpers.assertThrows( - "Long type should not be coercible to timestamp", - AnalysisException.class, - "Function 'hours' cannot process input: (bigint): Expected value to be timestamp", - () -> scalarSql("SELECT system.hours(1L)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.hours(1L)")) + .as("Long type should not be coercible to timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'hours' cannot process input: (bigint): Expected value to be timestamp"); } } diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkMonthsFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkMonthsFunction.java index b88bf00256b0..c95823b889c3 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkMonthsFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkMonthsFunction.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg.spark.sql; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.iceberg.spark.functions.MonthsFunction; import org.apache.spark.sql.AnalysisException; @@ -68,32 +67,32 @@ public void testTimestamps() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'months' cannot process input: (): Wrong number of inputs", - () -> scalarSql("SELECT system.months()")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.months()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'months' cannot process input: (): Wrong number of inputs"); - AssertHelpers.assertThrows( - "Function resolution should not work with more than one argument", - AnalysisException.class, - "Function 'months' cannot process input: (date, date): Wrong number of inputs", - () -> scalarSql("SELECT system.months(date('1969-12-31'), date('1969-12-31'))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.months(date('1969-12-31'), date('1969-12-31'))")) + .as("Function resolution should not work with more than one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'months' cannot process input: (date, date): Wrong number of inputs"); } @Test public void testInvalidInputTypes() { - AssertHelpers.assertThrows( - "Int type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'months' cannot process input: (int): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.months(1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.months(1)")) + .as("Int type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'months' cannot process input: (int): Expected value to be date or timestamp"); - AssertHelpers.assertThrows( - "Long type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'months' cannot process input: (bigint): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.months(1L)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.months(1L)")) + .as("Long type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'months' cannot process input: (bigint): Expected value to be date or timestamp"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkTruncateFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkTruncateFunction.java index f21544fcdf7a..aaf4f569b655 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkTruncateFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkTruncateFunction.java @@ -20,7 +20,6 @@ import java.math.BigDecimal; import java.nio.charset.StandardCharsets; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.spark.sql.AnalysisException; @@ -298,95 +297,106 @@ public void testWidthAcceptsShortAndByte() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'truncate' cannot process input: (): Wrong number of inputs (expected width and value)", - () -> scalarSql("SELECT system.truncate()")); - - AssertHelpers.assertThrows( - "Function resolution should not work with only one argument", - AnalysisException.class, - "Function 'truncate' cannot process input: (int): Wrong number of inputs (expected width and value)", - () -> scalarSql("SELECT system.truncate(1)")); - - AssertHelpers.assertThrows( - "Function resolution should not work with more than two arguments", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, bigint, int): Wrong number of inputs (expected width and value)", - () -> scalarSql("SELECT system.truncate(1, 1L, 1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (): Wrong number of inputs (expected width and value)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate(1)")) + .as("Function resolution should not work with only one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int): Wrong number of inputs (expected width and value)"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate(1, 1L, 1)")) + .as("Function resolution should not work with more than two arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, bigint, int): Wrong number of inputs (expected width and value)"); } @Test public void testInvalidTypesCannotBeUsedForWidth() { - AssertHelpers.assertThrows( - "Decimal type should not be coercible to the width field", - AnalysisException.class, - "Function 'truncate' cannot process input: (decimal(9,2), int): Expected truncation width to be tinyint, shortint or int", - () -> scalarSql("SELECT system.truncate(CAST('12.34' as DECIMAL(9, 2)), 10)")); - - AssertHelpers.assertThrows( - "String type should not be coercible to the width field", - AnalysisException.class, - "Function 'truncate' cannot process input: (string, int): Expected truncation width to be tinyint, shortint or int", - () -> scalarSql("SELECT system.truncate('5', 10)")); - - AssertHelpers.assertThrows( - "Interval year to month type should not be coercible to the width field", - AnalysisException.class, - "Function 'truncate' cannot process input: (interval year to month, int): Expected truncation width to be tinyint, shortint or int", - () -> scalarSql("SELECT system.truncate(INTERVAL '100-00' YEAR TO MONTH, 10)")); - - AssertHelpers.assertThrows( - "Interval day-time type should not be coercible to the width field", - AnalysisException.class, - "Function 'truncate' cannot process input: (interval day to second, int): Expected truncation width to be tinyint, shortint or int", - () -> scalarSql("SELECT system.truncate(CAST('11 23:4:0' AS INTERVAL DAY TO SECOND), 10)")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.truncate(CAST('12.34' as DECIMAL(9, 2)), 10)")) + .as("Decimal type should not be coercible to the width field") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (decimal(9,2), int): Expected truncation width to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate('5', 10)")) + .as("String type should not be coercible to the width field") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (string, int): Expected truncation width to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.truncate(INTERVAL '100-00' YEAR TO MONTH, 10)")) + .as("Interval year to month type should not be coercible to the width field") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (interval year to month, int): Expected truncation width to be tinyint, shortint or int"); + + Assertions.assertThatThrownBy( + () -> + scalarSql( + "SELECT system.truncate(CAST('11 23:4:0' AS INTERVAL DAY TO SECOND), 10)")) + .as("Interval day-time type should not be coercible to the width field") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (interval day to second, int): Expected truncation width to be tinyint, shortint or int"); } @Test public void testInvalidTypesForTruncationColumn() { - AssertHelpers.assertThrows( - "FLoat type should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, float): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, cast(12.3456 as float))")); - - AssertHelpers.assertThrows( - "Double type should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, double): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, cast(12.3456 as double))")); - - AssertHelpers.assertThrows( - "Boolean type should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, boolean): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, true)")); - - AssertHelpers.assertThrows( - "Map types should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, map): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, map(1, 1))")); - - AssertHelpers.assertThrows( - "Array types should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, array): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, array(1L))")); - - AssertHelpers.assertThrows( - "Interval year-to-month type should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, interval year to month): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, INTERVAL '100-00' YEAR TO MONTH)")); - - AssertHelpers.assertThrows( - "Interval day-time type should not be truncatable", - AnalysisException.class, - "Function 'truncate' cannot process input: (int, interval day to second): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary", - () -> scalarSql("SELECT system.truncate(10, CAST('11 23:4:0' AS INTERVAL DAY TO SECOND))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.truncate(10, cast(12.3456 as float))")) + .as("FLoat type should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, float): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.truncate(10, cast(12.3456 as double))")) + .as("Double type should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, double): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate(10, true)")) + .as("Boolean type should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, boolean): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate(10, map(1, 1))")) + .as("Map types should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, map): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.truncate(10, array(1L))")) + .as("Array types should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, array): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.truncate(10, INTERVAL '100-00' YEAR TO MONTH)")) + .as("Interval year-to-month type should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, interval year to month): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); + + Assertions.assertThatThrownBy( + () -> + scalarSql( + "SELECT system.truncate(10, CAST('11 23:4:0' AS INTERVAL DAY TO SECOND))")) + .as("Interval day-time type should not be truncatable") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'truncate' cannot process input: (int, interval day to second): Expected truncation col to be tinyint, shortint, int, bigint, decimal, string, or binary"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkYearsFunction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkYearsFunction.java index d4676716a612..600b6ac0c1c5 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkYearsFunction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestSparkYearsFunction.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg.spark.sql; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.iceberg.spark.functions.YearsFunction; import org.apache.spark.sql.AnalysisException; @@ -70,32 +69,32 @@ public void testTimestamps() { @Test public void testWrongNumberOfArguments() { - AssertHelpers.assertThrows( - "Function resolution should not work with zero arguments", - AnalysisException.class, - "Function 'years' cannot process input: (): Wrong number of inputs", - () -> scalarSql("SELECT system.years()")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.years()")) + .as("Function resolution should not work with zero arguments") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining("Function 'years' cannot process input: (): Wrong number of inputs"); - AssertHelpers.assertThrows( - "Function resolution should not work with more than one argument", - AnalysisException.class, - "Function 'years' cannot process input: (date, date): Wrong number of inputs", - () -> scalarSql("SELECT system.years(date('1969-12-31'), date('1969-12-31'))")); + Assertions.assertThatThrownBy( + () -> scalarSql("SELECT system.years(date('1969-12-31'), date('1969-12-31'))")) + .as("Function resolution should not work with more than one argument") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'years' cannot process input: (date, date): Wrong number of inputs"); } @Test public void testInvalidInputTypes() { - AssertHelpers.assertThrows( - "Int type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'years' cannot process input: (int): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.years(1)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.years(1)")) + .as("Int type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'years' cannot process input: (int): Expected value to be date or timestamp"); - AssertHelpers.assertThrows( - "Long type should not be coercible to date/timestamp", - AnalysisException.class, - "Function 'years' cannot process input: (bigint): Expected value to be date or timestamp", - () -> scalarSql("SELECT system.years(1L)")); + Assertions.assertThatThrownBy(() -> scalarSql("SELECT system.years(1L)")) + .as("Long type should not be coercible to date/timestamp") + .isInstanceOf(AnalysisException.class) + .hasMessageContaining( + "Function 'years' cannot process input: (bigint): Expected value to be date or timestamp"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestTimestampWithoutZone.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestTimestampWithoutZone.java index 51b8d255a99b..5a8c5736a6b4 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestTimestampWithoutZone.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestTimestampWithoutZone.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Schema; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.TableIdentifier; @@ -37,6 +36,7 @@ import org.apache.iceberg.types.Type; import org.apache.iceberg.types.Types; import org.apache.spark.sql.util.CaseInsensitiveStringMap; +import org.assertj.core.api.Assertions; import org.joda.time.DateTime; import org.junit.After; import org.junit.Assert; @@ -95,14 +95,15 @@ public void removeTables() { @Test public void testWriteTimestampWithoutZoneError() { - AssertHelpers.assertThrows( - String.format( - "Write operation performed on a timestamp without timezone field while " - + "'%s' set to false should throw exception", - SparkSQLProperties.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE), - IllegalArgumentException.class, - SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR, - () -> sql("INSERT INTO %s VALUES %s", tableName, rowToSqlValues(values))); + Assertions.assertThatThrownBy( + () -> sql("INSERT INTO %s VALUES %s", tableName, rowToSqlValues(values))) + .as( + String.format( + "Write operation performed on a timestamp without timezone field while " + + "'%s' set to false should throw exception", + SparkSQLProperties.HANDLE_TIMESTAMP_WITHOUT_TIMEZONE)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(SparkUtil.TIMESTAMP_WITHOUT_TIMEZONE_ERROR); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/UnpartitionedWritesTestBase.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/UnpartitionedWritesTestBase.java index 71089ebfd79e..813e442c6604 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/UnpartitionedWritesTestBase.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/UnpartitionedWritesTestBase.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.spark.SparkCatalogTestBase; import org.apache.iceberg.spark.source.SimpleRecord; @@ -28,6 +27,7 @@ import org.apache.spark.sql.Row; import org.apache.spark.sql.catalyst.analysis.NoSuchTableException; import org.apache.spark.sql.functions; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Assert; import org.junit.Assume; @@ -97,11 +97,12 @@ public void testInsertAppendAtSnapshot() { Assume.assumeTrue(tableName.equals(commitTarget())); long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId(); String prefix = "snapshot_id_"; - AssertHelpers.assertThrows( - "Should not be able to insert into a table at a specific snapshot", - IllegalArgumentException.class, - "Cannot write to table at a specific snapshot", - () -> sql("INSERT INTO %s.%s VALUES (4, 'd'), (5, 'e')", tableName, prefix + snapshotId)); + Assertions.assertThatThrownBy( + () -> + sql("INSERT INTO %s.%s VALUES (4, 'd'), (5, 'e')", tableName, prefix + snapshotId)) + .as("Should not be able to insert into a table at a specific snapshot") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot write to table at a specific snapshot"); } @Test @@ -109,14 +110,14 @@ public void testInsertOverwriteAtSnapshot() { Assume.assumeTrue(tableName.equals(commitTarget())); long snapshotId = validationCatalog.loadTable(tableIdent).currentSnapshot().snapshotId(); String prefix = "snapshot_id_"; - AssertHelpers.assertThrows( - "Should not be able to insert into a table at a specific snapshot", - IllegalArgumentException.class, - "Cannot write to table at a specific snapshot", - () -> - sql( - "INSERT OVERWRITE %s.%s VALUES (4, 'd'), (5, 'e')", - tableName, prefix + snapshotId)); + Assertions.assertThatThrownBy( + () -> + sql( + "INSERT OVERWRITE %s.%s VALUES (4, 'd'), (5, 'e')", + tableName, prefix + snapshotId)) + .as("Should not be able to insert into a table at a specific snapshot") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot write to table at a specific snapshot"); } @Test From 88d3c76940f1794395cd15b862f37ac517da498b Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Mon, 17 Jun 2024 14:49:02 +0200 Subject: [PATCH 2/3] Assert on exception messages where converted from AssertHelpers --- .../spark/extensions/TestChangelogTable.java | 3 +- .../actions/TestRewriteDataFilesAction.java | 29 ++++++++++++++----- .../spark/source/TestWriteMetricsConfig.java | 5 +++- .../iceberg/spark/sql/TestAlterTable.java | 4 ++- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java index c5404c24e47f..3b6ac57cc03e 100644 --- a/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java +++ b/spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestChangelogTable.java @@ -205,7 +205,8 @@ public void testTimeRangeValidation() { Assertions.assertThatThrownBy( () -> changelogRecords(snap3.timestampMillis(), snap2.timestampMillis())) .as("Should fail if start time is after end time") - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Cannot set start-timestamp to be greater than end-timestamp for changelogs"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java index 4159691a64f6..5e8865dff2da 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/actions/TestRewriteDataFilesAction.java @@ -644,7 +644,8 @@ public void testSingleCommitWithRewriteFailure() { Assertions.assertThatThrownBy(() -> spyRewrite.execute()) .as("Should fail entire rewrite if part fails") - .isInstanceOf(RuntimeException.class); + .isInstanceOf(RuntimeException.class) + .hasMessage("Rewrite Failed"); table.refresh(); @@ -678,7 +679,8 @@ public void testSingleCommitWithCommitFailure() { Assertions.assertThatThrownBy(() -> spyRewrite.execute()) .as("Should fail entire rewrite if commit fails") - .isInstanceOf(RuntimeException.class); + .isInstanceOf(RuntimeException.class) + .hasMessage("Commit Failure"); table.refresh(); @@ -713,7 +715,8 @@ public void testParallelSingleCommitWithRewriteFailure() { Assertions.assertThatThrownBy(() -> spyRewrite.execute()) .as("Should fail entire rewrite if part fails") - .isInstanceOf(RuntimeException.class); + .isInstanceOf(RuntimeException.class) + .hasMessage("Rewrite Failed"); table.refresh(); @@ -862,7 +865,10 @@ public void testInvalidOptions() { .option(RewriteDataFiles.PARTIAL_PROGRESS_MAX_COMMITS, "-5") .execute()) .as("No negative values for partial progress max commits") - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot set partial-progress.max-commits to -5, " + + "the value must be positive when partial-progress.enabled is true"); Assertions.assertThatThrownBy( () -> @@ -870,16 +876,21 @@ public void testInvalidOptions() { .option(RewriteDataFiles.MAX_CONCURRENT_FILE_GROUP_REWRITES, "-5") .execute()) .as("No negative values for max concurrent groups") - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot set max-concurrent-file-group-rewrites to -5, the value must be positive."); Assertions.assertThatThrownBy(() -> basicRewrite(table).option("foobarity", "-5").execute()) .as("No unknown options allowed") - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Cannot use options [foobarity], they are not supported by the action or the rewriter BIN-PACK"); Assertions.assertThatThrownBy( () -> basicRewrite(table).option(RewriteDataFiles.REWRITE_JOB_ORDER, "foo").execute()) .as("Cannot set rewrite-job-order to foo") - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid rewrite job order name: foo"); } @Test @@ -1117,7 +1128,9 @@ public void testCommitStateUnknownException() { Assertions.assertThatThrownBy(() -> spyAction.execute()) .as("Should propagate CommitStateUnknown Exception") - .isInstanceOf(CommitStateUnknownException.class); + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageStartingWith( + "Unknown State\n" + "Cannot determine whether the commit was successful or not"); List postRewriteData = currentData(); assertEquals("We shouldn't have changed the data", originalData, postRewriteData); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java index 291b8d27539c..fd88eb2c907e 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestWriteMetricsConfig.java @@ -234,7 +234,10 @@ public void testBadCustomMetricCollectionForParquet() throws IOException { Assertions.assertThatThrownBy( () -> tables.create(SIMPLE_SCHEMA, spec, properties, tableLocation)) .as("Creating a table with invalid metrics should fail") - .isInstanceOf(ValidationException.class); + .isInstanceOf(ValidationException.class) + .hasMessageStartingWith( + "Invalid metrics config, could not find column ids from table prop write.metadata.metrics.column.ids in " + + "schema table"); } @Test diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java index ec6b522fec21..901a7c15e78f 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestAlterTable.java @@ -324,6 +324,8 @@ public void testSetTableProperties() { Assertions.assertThatThrownBy( () -> sql("ALTER TABLE %s SET TBLPROPERTIES ('sort-order'='value')", tableName)) .as("Cannot specify the 'sort-order' because it's a reserved table property") - .isInstanceOf(UnsupportedOperationException.class); + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageStartingWith( + "Cannot specify the 'sort-order' because it's a reserved table property"); } } From c297cd97d8effecb7d4b11ca30f0f29be2ad70cd Mon Sep 17 00:00:00 2001 From: Piotr Findeisen Date: Sat, 15 Jun 2024 14:01:15 +0200 Subject: [PATCH 3/3] Remove AssertHelpers `AssertHelpers` was deprecated and all the usages were removed. --- LICENSE | 1 - .../org/apache/iceberg/AssertHelpers.java | 213 ------------------ 2 files changed, 214 deletions(-) delete mode 100644 api/src/test/java/org/apache/iceberg/AssertHelpers.java diff --git a/LICENSE b/LICENSE index 46167007c6de..efb46dab44da 100644 --- a/LICENSE +++ b/LICENSE @@ -227,7 +227,6 @@ This product includes code from Apache Parquet. * DynMethods.java * DynConstructors.java -* AssertHelpers.java * IOUtil.java readFully and tests * ByteBufferInputStream implementations and tests diff --git a/api/src/test/java/org/apache/iceberg/AssertHelpers.java b/api/src/test/java/org/apache/iceberg/AssertHelpers.java deleted file mode 100644 index a1cf1b20797e..000000000000 --- a/api/src/test/java/org/apache/iceberg/AssertHelpers.java +++ /dev/null @@ -1,213 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.iceberg; - -import java.util.concurrent.Callable; -import org.apache.avro.AvroRuntimeException; -import org.apache.avro.generic.GenericRecord; -import org.assertj.core.api.AbstractThrowableAssert; -import org.assertj.core.api.Assertions; -import org.assertj.core.api.ThrowableAssert; - -/** - * This class is deprecated. Please use {@link - * Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} directly as shown below: - * - * 
- * Assertions.assertThatThrownBy(() -> throwingCallable)
- *    .isInstanceOf(ExpectedException.class)
- *    .hasMessage(expectedErrorMsg)
- *
- * - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} directly - * as it provides a more fluent way of asserting on exceptions. - */ -@Deprecated -public class AssertHelpers { - - private AssertHelpers() {} - - /** - * A convenience method to avoid a large number of @Test(expected=...) tests - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the Runnable should throw - * @param containedInMessage A String that should be contained by the thrown exception's message - * @param callable A Callable that is expected to throw the exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrows( - String message, - Class expected, - String containedInMessage, - Callable callable) { - AbstractThrowableAssert check = - Assertions.assertThatThrownBy(callable::call).as(message).isInstanceOf(expected); - if (null != containedInMessage) { - check.hasMessageContaining(containedInMessage); - } - } - - /** - * A convenience method to avoid a large number of @Test(expected=...) tests - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the Runnable should throw - * @param containedInMessage A String that should be contained by the thrown exception's message - * @param runnable A Runnable that is expected to throw the runtime exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrows( - String message, - Class expected, - String containedInMessage, - Runnable runnable) { - AbstractThrowableAssert check = - Assertions.assertThatThrownBy(runnable::run).as(message).isInstanceOf(expected); - if (null != containedInMessage) { - check.hasMessageContaining(containedInMessage); - } - } - - /** - * A convenience method to avoid a large number of @Test(expected=...) tests - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the Runnable should throw - * @param callable A Callable that is expected to throw the exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrows( - String message, Class expected, Callable callable) { - assertThrows(message, expected, null, callable); - } - - /** - * A convenience method to avoid a large number of @Test(expected=...) tests - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the Runnable should throw - * @param runnable A Runnable that is expected to throw the runtime exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrows( - String message, Class expected, Runnable runnable) { - assertThrows(message, expected, null, runnable); - } - - /** - * A convenience method to assert the cause of thrown exception. - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the cause of the Runnable should throw - * @param containedInMessage A String that should be contained by the cause of the thrown - * exception's message - * @param runnable A Runnable that is expected to throw the runtime exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrowsCause( - String message, - Class expected, - String containedInMessage, - Runnable runnable) { - Assertions.assertThatThrownBy(runnable::run) - .as(message) - .getCause() - .isInstanceOf(expected) - .hasMessageContaining(containedInMessage); - } - - /** - * A convenience method to assert both the thrown exception and the cause of thrown exception. - * - * @param message A String message to describe this assertion - * @param expected An Exception class that the Runnable should throw - * @param expectedContainedInMessage A String that should be contained by the thrown exception's - * message, will be skipped if null. - * @param cause An Exception class that the cause of the Runnable should throw - * @param causeContainedInMessage A String that should be contained by the cause of the thrown - * exception's message, will be skipped if null. - * @param runnable A Runnable that is expected to throw the runtime exception - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrowsWithCause( - String message, - Class expected, - String expectedContainedInMessage, - Class cause, - String causeContainedInMessage, - Runnable runnable) { - AbstractThrowableAssert chain = - Assertions.assertThatThrownBy(runnable::run).as(message).isInstanceOf(expected); - - if (expectedContainedInMessage != null) { - chain = chain.hasMessageContaining(expectedContainedInMessage); - } - - chain = chain.getCause().isInstanceOf(cause); - if (causeContainedInMessage != null) { - chain.hasMessageContaining(causeContainedInMessage); - } - } - - /** - * A convenience method to check if an Avro field is empty. - * - * @param record The record to read from - * @param field The name of the field - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertEmptyAvroField(GenericRecord record, String field) { - AssertHelpers.assertThrows( - "Not a valid schema field: " + field, AvroRuntimeException.class, () -> record.get(field)); - } - - /** - * Same as {@link AssertHelpers#assertThrowsCause}, but this method compares root cause. - * - * @deprecated Use {@link Assertions#assertThatThrownBy(ThrowableAssert.ThrowingCallable)} - * directly as it provides a more fluent way of asserting on exceptions. - */ - @Deprecated - public static void assertThrowsRootCause( - String message, - Class expected, - String containedInMessage, - Runnable runnable) { - Assertions.assertThatThrownBy(runnable::run) - .as(message) - .getRootCause() - .isInstanceOf(expected) - .hasMessageContaining(containedInMessage); - } -}