This is an automated email from the ASF dual-hosted git repository.
fokko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 95be02c912 Core: Remove deprecated AssertHelpers usage in catalog
(#7596)
95be02c912 is described below
commit 95be02c91267e355c62818efb265ffea9a8ba8cb
Author: Liu Xiao <[email protected]>
AuthorDate: Mon May 15 15:25:40 2023 +0800
Core: Remove deprecated AssertHelpers usage in catalog (#7596)
* Core: Remove deprecated AssertHelpers usage in catalog
* fix test
---
.../org/apache/iceberg/catalog/CatalogTests.java | 148 ++++++++-------------
.../iceberg/catalog/TestTableIdentifierParser.java | 58 +++-----
2 files changed, 79 insertions(+), 127 deletions(-)
diff --git a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
index b66ab940a8..6e9a175183 100644
--- a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
+++ b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java
@@ -28,7 +28,6 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.iceberg.AppendFiles;
-import org.apache.iceberg.AssertHelpers;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.DataFile;
import org.apache.iceberg.DataFiles;
@@ -186,11 +185,10 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
catalog.createNamespace(NS);
Assert.assertTrue("Namespace should exist", catalog.namespaceExists(NS));
- AssertHelpers.assertThrows(
- "Should fail to create an existing database",
- AlreadyExistsException.class,
- "newdb",
- () -> catalog.createNamespace(NS));
+ Assertions.assertThatThrownBy(() -> catalog.createNamespace(NS))
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessageContaining("Namespace already exists");
+
Assert.assertTrue("Namespace should still exist",
catalog.namespaceExists(NS));
}
@@ -219,11 +217,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
Assert.assertFalse("Namespace should not exist",
catalog.namespaceExists(NS));
- AssertHelpers.assertThrows(
- "Should fail to load nonexistent namespace metadata",
- NoSuchNamespaceException.class,
- "newdb",
- () -> catalog.loadNamespaceMetadata(NS));
+ Assertions.assertThatThrownBy(() -> catalog.loadNamespaceMetadata(NS))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessage("Namespace does not exist: newdb");
catalog.createNamespace(NS);
Assert.assertTrue("Namespace should exist", catalog.namespaceExists(NS));
@@ -314,11 +310,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
C catalog = catalog();
- AssertHelpers.assertThrows(
- "setProperties should fail if the namespace does not exist",
- NoSuchNamespaceException.class,
- "does not exist",
- () -> catalog.setProperties(NS, ImmutableMap.of("test", "value")));
+ Assertions.assertThatThrownBy(() -> catalog.setProperties(NS,
ImmutableMap.of("test", "value")))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessage("Namespace does not exist: newdb");
}
@Test
@@ -348,11 +342,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
C catalog = catalog();
- AssertHelpers.assertThrows(
- "setProperties should fail if the namespace does not exist",
- NoSuchNamespaceException.class,
- "does not exist",
- () -> catalog.removeProperties(NS, ImmutableSet.of("a", "b")));
+ Assertions.assertThatThrownBy(() -> catalog.removeProperties(NS,
ImmutableSet.of("a", "b")))
+ .isInstanceOf(NoSuchNamespaceException.class)
+ .hasMessage("Namespace does not exist: newdb");
}
@Test
@@ -587,11 +579,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
catalog.buildTable(ident, SCHEMA).create();
Assert.assertTrue("Table should exist", catalog.tableExists(ident));
- AssertHelpers.assertThrows(
- "Should fail to create a table that already exists",
- AlreadyExistsException.class,
- "ns.table",
- () -> catalog.buildTable(ident, OTHER_SCHEMA).create());
+ Assertions.assertThatThrownBy(() -> catalog.buildTable(ident,
OTHER_SCHEMA).create())
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessage("Table already exists: ns.table");
Table table = catalog.loadTable(ident);
Assert.assertEquals(
@@ -711,11 +701,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
TableIdentifier ident = TableIdentifier.of("ns", "table");
Assert.assertFalse("Table should not exist", catalog.tableExists(ident));
- AssertHelpers.assertThrows(
- "Should fail to load a nonexistent table",
- NoSuchTableException.class,
- ident.toString(),
- () -> catalog.loadTable(ident));
+ Assertions.assertThatThrownBy(() -> catalog.loadTable(ident))
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessage("Table does not exist: ns.table");
}
@Test
@@ -754,11 +742,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
Assert.assertFalse(
"Destination table should not exist before rename",
catalog.tableExists(RENAMED_TABLE));
- AssertHelpers.assertThrows(
- "Should reject renaming a table that does not exist",
- NoSuchTableException.class,
- "Table does not exist",
- () -> catalog.renameTable(TABLE, RENAMED_TABLE));
+ Assertions.assertThatThrownBy(() -> catalog.renameTable(TABLE,
RENAMED_TABLE))
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessageContaining("Table does not exist");
Assert.assertFalse(
"Destination table should not exist after failed rename",
@@ -783,11 +769,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
Assert.assertTrue(
"Destination table should exist after create",
catalog.tableExists(RENAMED_TABLE));
- AssertHelpers.assertThrows(
- "Should reject renaming a table if the new name already exists",
- AlreadyExistsException.class,
- "Table already exists",
- () -> catalog.renameTable(TABLE, RENAMED_TABLE));
+ Assertions.assertThatThrownBy(() -> catalog.renameTable(TABLE,
RENAMED_TABLE))
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessageContaining("Table already exists");
Assert.assertTrue(
"Source table should still exist after failed rename",
catalog.tableExists(TABLE));
@@ -990,11 +974,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
String expectedMessage =
supportsServerSideRetry() ? "Requirement failed: UUID does not match"
: "Cannot commit";
- AssertHelpers.assertThrows(
- "Should reject changes to tables that have been dropped and recreated",
- CommitFailedException.class,
- expectedMessage,
- update::commit);
+ Assertions.assertThatThrownBy(update::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining(expectedMessage);
Table loaded = catalog.loadTable(TABLE);
Assert.assertEquals(
@@ -1052,11 +1034,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
// attempt to commit the original update
String expectedMessage =
supportsServerSideRetry() ? "Requirement failed: current schema
changed" : "Cannot commit";
- AssertHelpers.assertThrows(
- "Second schema update commit should fail because of a conflict",
- CommitFailedException.class,
- expectedMessage,
- update::commit);
+ Assertions.assertThatThrownBy(update::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining(expectedMessage);
Table loaded = catalog.loadTable(TABLE);
Assert.assertEquals(
@@ -1088,11 +1068,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
supportsServerSideRetry()
? "Requirement failed: last assigned field id changed"
: "Cannot commit";
- AssertHelpers.assertThrows(
- "Second schema update commit should fail because of a conflict",
- CommitFailedException.class,
- expectedMessage,
- update::commit);
+ Assertions.assertThatThrownBy(update::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining(expectedMessage);
Table loaded = catalog.loadTable(TABLE);
Assert.assertEquals(
@@ -1204,11 +1182,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
supportsServerSideRetry()
? "Requirement failed: default partition spec changed"
: "Cannot commit";
- AssertHelpers.assertThrows(
- "Second partition spec update commit should fail because of a
conflict",
- CommitFailedException.class,
- expectedMessage,
- update::commit);
+ Assertions.assertThatThrownBy(update::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining(expectedMessage);
Table loaded = catalog.loadTable(TABLE);
@@ -1240,11 +1216,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
supportsServerSideRetry()
? "Requirement failed: last assigned partition id changed"
: "Cannot commit";
- AssertHelpers.assertThrows(
- "Second partition spec update commit should fail because of a
conflict",
- CommitFailedException.class,
- expectedMessage,
- update::commit);
+ Assertions.assertThatThrownBy(update::commit)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageContaining(expectedMessage);
Table loaded = catalog.loadTable(TABLE);
@@ -1711,11 +1685,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
supportsServerSideRetry()
? "Requirement failed: table already exists"
: "Table already exists";
- AssertHelpers.assertThrows(
- "Should fail because table was created concurrently",
- AlreadyExistsException.class,
- expectedMessage,
- create::commitTransaction);
+ Assertions.assertThatThrownBy(create::commitTransaction)
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessageStartingWith(expectedMessage);
// validate the concurrently created table is unmodified
Table table = catalog.loadTable(TABLE);
@@ -1944,11 +1916,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
supportsServerSideRetry()
? "Requirement failed: table already exists"
: "Table already exists";
- AssertHelpers.assertThrows(
- "Should fail because table was created concurrently",
- AlreadyExistsException.class,
- expectedMessage,
- createOrReplace::commitTransaction);
+ Assertions.assertThatThrownBy(createOrReplace::commitTransaction)
+ .isInstanceOf(AlreadyExistsException.class)
+ .hasMessage(expectedMessage);
// validate the concurrently created table is unmodified
Table table = catalog.loadTable(TABLE);
@@ -2081,11 +2051,9 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
catalog.createNamespace(NS);
}
- AssertHelpers.assertThrows(
- "Should fail to create replace transaction with a missing table",
- NoSuchTableException.class,
- "Table does not exist",
- () -> catalog.buildTable(TABLE, SCHEMA).replaceTransaction());
+ Assertions.assertThatThrownBy(() -> catalog.buildTable(TABLE,
SCHEMA).replaceTransaction())
+ .isInstanceOf(NoSuchTableException.class)
+ .hasMessage("Table does not exist: newdb.table");
}
@Test
@@ -2249,11 +2217,10 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
// even though the new schema is identical, the assertion that the last
assigned id has not
// changed will fail
- AssertHelpers.assertThrows(
- "Should reject concurrent schema update",
- CommitFailedException.class,
- "last assigned field id changed",
- secondReplace::commitTransaction);
+ Assertions.assertThatThrownBy(secondReplace::commitTransaction)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageStartingWith(
+ "Commit failed: Requirement failed: last assigned field id
changed");
}
@Test
@@ -2369,11 +2336,10 @@ public abstract class CatalogTests<C extends Catalog &
SupportsNamespaces> {
// even though the new spec is identical, the assertion that the last
assigned id has not
// changed will fail
- AssertHelpers.assertThrows(
- "Should reject concurrent spec update",
- CommitFailedException.class,
- "last assigned partition id changed",
- secondReplace::commitTransaction);
+ Assertions.assertThatThrownBy(secondReplace::commitTransaction)
+ .isInstanceOf(CommitFailedException.class)
+ .hasMessageStartingWith(
+ "Commit failed: Requirement failed: last assigned partition id
changed");
}
@Test
diff --git
a/core/src/test/java/org/apache/iceberg/catalog/TestTableIdentifierParser.java
b/core/src/test/java/org/apache/iceberg/catalog/TestTableIdentifierParser.java
index ab6e9893e5..05ca49293f 100644
---
a/core/src/test/java/org/apache/iceberg/catalog/TestTableIdentifierParser.java
+++
b/core/src/test/java/org/apache/iceberg/catalog/TestTableIdentifierParser.java
@@ -18,7 +18,7 @@
*/
package org.apache.iceberg.catalog;
-import org.apache.iceberg.AssertHelpers;
+import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
@@ -67,58 +67,44 @@ public class TestTableIdentifierParser {
@Test
public void testFailParsingWhenNullOrEmptyJson() {
String nullJson = null;
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize null JSON string",
- IllegalArgumentException.class,
- "Cannot parse table identifier from invalid JSON: null",
- () -> TableIdentifierParser.fromJson(nullJson));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(nullJson))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse table identifier from invalid JSON: null");
String emptyString = "";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize an empty string",
- IllegalArgumentException.class,
- "Cannot parse table identifier from invalid JSON: ''",
- () -> TableIdentifierParser.fromJson(emptyString));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(emptyString))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse table identifier from invalid JSON: ''");
String emptyJson = "{}";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize an empty JSON
string",
- IllegalArgumentException.class,
- "Cannot parse missing string: name",
- () -> TableIdentifierParser.fromJson(emptyJson));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(emptyJson))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse missing string: name");
String emptyJsonArray = "[]";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize an empty JSON array",
- IllegalArgumentException.class,
- "Cannot parse missing or non-object table identifier: []",
- () -> TableIdentifierParser.fromJson(emptyJsonArray));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(emptyJsonArray))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse missing or non-object table identifier: []");
}
@Test
public void testFailParsingWhenMissingRequiredFields() {
String identifierMissingName = "{\"namespace\":[\"accounting\",\"tax\"]}";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize table with missing
name",
- IllegalArgumentException.class,
- "Cannot parse missing string: name",
- () -> TableIdentifierParser.fromJson(identifierMissingName));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(identifierMissingName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse missing string: name");
}
@Test
public void testFailWhenFieldsHaveInvalidValues() {
String invalidNamespace =
"{\"namespace\":\"accounting.tax\",\"name\":\"paid\"}";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize table with invalid
namespace",
- IllegalArgumentException.class,
- "Cannot parse from non-array value: namespace: \"accounting.tax\"",
- () -> TableIdentifierParser.fromJson(invalidNamespace));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(invalidNamespace))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse from non-array value: namespace:
\"accounting.tax\"");
String invalidName =
"{\"namespace\":[\"accounting\",\"tax\"],\"name\":1234}";
- AssertHelpers.assertThrows(
- "TableIdentifierParser should fail to deserialize table with invalid
name",
- IllegalArgumentException.class,
- "Cannot parse to a string value: name: 1234",
- () -> TableIdentifierParser.fromJson(invalidName));
+ Assertions.assertThatThrownBy(() ->
TableIdentifierParser.fromJson(invalidName))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot parse to a string value: name: 1234");
}
}
