Skip to content

Commit

Permalink
Core: Remove usage of AssertHelpers (apache#7868)
Browse files Browse the repository at this point in the history
  • Loading branch information
liuxiaocs7 authored Jun 21, 2023
1 parent 9ffb762 commit 5c64100
Show file tree
Hide file tree
Showing 16 changed files with 214 additions and 242 deletions.
17 changes: 9 additions & 8 deletions core/src/test/java/org/apache/iceberg/ScanTestBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.iceberg.io.CloseableIterable;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.types.Types;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
Expand Down Expand Up @@ -67,14 +68,14 @@ public void testTableScanHonorsSelect() {

@Test
public void testTableBothProjectAndSelect() {
AssertHelpers.assertThrows(
"Cannot set projection schema when columns are selected",
IllegalStateException.class,
() -> newScan().select(Arrays.asList("id")).project(SCHEMA.select("data")));
AssertHelpers.assertThrows(
"Cannot select columns when projection schema is set",
IllegalStateException.class,
() -> newScan().project(SCHEMA.select("data")).select(Arrays.asList("id")));
Assertions.assertThatThrownBy(
() -> newScan().select(Arrays.asList("id")).project(SCHEMA.select("data")))
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot set projection schema when columns are selected");
Assertions.assertThatThrownBy(
() -> newScan().project(SCHEMA.select("data")).select(Arrays.asList("id")))
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot select columns when projection schema is set");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.iceberg;

import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -127,23 +128,21 @@ public void testMultipleRootSnapshots() throws Exception {
// scan should fail because snapshot B is not an ancestor of snapshot D
IncrementalAppendScan scanShouldFail =
newScan().fromSnapshotExclusive(snapshotBId).toSnapshot(snapshotDId);
AssertHelpers.assertThrows(
"Should throw exception",
IllegalArgumentException.class,
String.format(
"Starting snapshot (exclusive) %d is not a parent ancestor of end snapshot %d",
snapshotBId, snapshotDId),
() -> Iterables.size(scanShouldFail.planFiles()));
Assertions.assertThatThrownBy(() -> Iterables.size(scanShouldFail.planFiles()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
String.format(
"Starting snapshot (exclusive) %d is not a parent ancestor of end snapshot %d",
snapshotBId, snapshotDId));

// scan should fail because snapshot B is not an ancestor of snapshot D
IncrementalAppendScan scanShouldFailInclusive =
newScan().fromSnapshotInclusive(snapshotBId).toSnapshot(snapshotDId);
AssertHelpers.assertThrows(
"Should throw exception",
IllegalArgumentException.class,
String.format(
"Starting snapshot (inclusive) %d is not an ancestor of end snapshot %d",
snapshotBId, snapshotDId),
() -> Iterables.size(scanShouldFailInclusive.planFiles()));
Assertions.assertThatThrownBy(() -> Iterables.size(scanShouldFailInclusive.planFiles()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
String.format(
"Starting snapshot (inclusive) %d is not an ancestor of end snapshot %d",
snapshotBId, snapshotDId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
Expand Down Expand Up @@ -256,11 +257,9 @@ public void testDeleteFilesAreNotSupported() {

table.newRowDelta().addDeletes(FILE_A2_DELETES).commit();

AssertHelpers.assertThrows(
"Should complain about delete files",
UnsupportedOperationException.class,
"Delete files are currently not supported",
() -> plan(newScan()));
Assertions.assertThatThrownBy(() -> plan(newScan()))
.isInstanceOf(UnsupportedOperationException.class)
.hasMessage("Delete files are currently not supported in changelog scans");
}

// plans tasks and reorders them to have deterministic order
Expand Down
77 changes: 37 additions & 40 deletions core/src/test/java/org/apache/iceberg/TestCatalogUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,14 @@ public void loadCustomCatalog_NoArgConstructorNotFound() {
options.put("key", "val");
Configuration hadoopConf = new Configuration();
String name = "custom";
AssertHelpers.assertThrows(
"must have no-arg constructor",
IllegalArgumentException.class,
"NoSuchMethodException: org.apache.iceberg.TestCatalogUtil$TestCatalogBadConstructor.<init>()",
() ->
CatalogUtil.loadCatalog(
TestCatalogBadConstructor.class.getName(), name, options, hadoopConf));
Assertions.assertThatThrownBy(
() ->
CatalogUtil.loadCatalog(
TestCatalogBadConstructor.class.getName(), name, options, hadoopConf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize Catalog implementation")
.hasMessageContaining(
"NoSuchMethodException: org.apache.iceberg.TestCatalogUtil$TestCatalogBadConstructor.<init>()");
}

@Test
Expand All @@ -89,13 +90,13 @@ public void loadCustomCatalog_NotImplementCatalog() {
Configuration hadoopConf = new Configuration();
String name = "custom";

AssertHelpers.assertThrows(
"must implement catalog",
IllegalArgumentException.class,
"does not implement Catalog",
() ->
CatalogUtil.loadCatalog(
TestCatalogNoInterface.class.getName(), name, options, hadoopConf));
Assertions.assertThatThrownBy(
() ->
CatalogUtil.loadCatalog(
TestCatalogNoInterface.class.getName(), name, options, hadoopConf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize Catalog")
.hasMessageContaining("does not implement Catalog");
}

@Test
Expand All @@ -106,11 +107,10 @@ public void loadCustomCatalog_ConstructorErrorCatalog() {
String name = "custom";

String impl = TestCatalogErrorConstructor.class.getName();
AssertHelpers.assertThrows(
"must be able to initialize catalog",
IllegalArgumentException.class,
"NoClassDefFoundError: Error while initializing class",
() -> CatalogUtil.loadCatalog(impl, name, options, hadoopConf));
Assertions.assertThatThrownBy(() -> CatalogUtil.loadCatalog(impl, name, options, hadoopConf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize Catalog implementation")
.hasMessageContaining("NoClassDefFoundError: Error while initializing class");
}

@Test
Expand All @@ -120,11 +120,10 @@ public void loadCustomCatalog_BadCatalogNameCatalog() {
Configuration hadoopConf = new Configuration();
String name = "custom";
String impl = "CatalogDoesNotExist";
AssertHelpers.assertThrows(
"catalog must exist",
IllegalArgumentException.class,
"java.lang.ClassNotFoundException: CatalogDoesNotExist",
() -> CatalogUtil.loadCatalog(impl, name, options, hadoopConf));
Assertions.assertThatThrownBy(() -> CatalogUtil.loadCatalog(impl, name, options, hadoopConf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize Catalog implementation")
.hasMessageContaining("java.lang.ClassNotFoundException: CatalogDoesNotExist");
}

@Test
Expand Down Expand Up @@ -159,20 +158,20 @@ public void loadCustomFileIO_configurable() {

@Test
public void loadCustomFileIO_badArg() {
AssertHelpers.assertThrows(
"cannot find constructor",
IllegalArgumentException.class,
"missing no-arg constructor",
() -> CatalogUtil.loadFileIO(TestFileIOBadArg.class.getName(), Maps.newHashMap(), null));
Assertions.assertThatThrownBy(
() -> CatalogUtil.loadFileIO(TestFileIOBadArg.class.getName(), Maps.newHashMap(), null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize FileIO, missing no-arg constructor");
}

@Test
public void loadCustomFileIO_badClass() {
AssertHelpers.assertThrows(
"cannot cast",
IllegalArgumentException.class,
"does not implement FileIO",
() -> CatalogUtil.loadFileIO(TestFileIONotImpl.class.getName(), Maps.newHashMap(), null));
Assertions.assertThatThrownBy(
() ->
CatalogUtil.loadFileIO(TestFileIONotImpl.class.getName(), Maps.newHashMap(), null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Cannot initialize FileIO")
.hasMessageContaining("does not implement FileIO");
}

@Test
Expand All @@ -182,12 +181,10 @@ public void buildCustomCatalog_withTypeSet() {
options.put(CatalogUtil.ICEBERG_CATALOG_TYPE, "hive");
Configuration hadoopConf = new Configuration();
String name = "custom";

AssertHelpers.assertThrows(
"Should complain about both configs being set",
IllegalArgumentException.class,
"both type and catalog-impl are set",
() -> CatalogUtil.buildIcebergCatalog(name, options, hadoopConf));
Assertions.assertThatThrownBy(() -> CatalogUtil.buildIcebergCatalog(name, options, hadoopConf))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
"Cannot create catalog custom, both type and catalog-impl are set: type=hive, catalog-impl=CustomCatalog");
}

@Test
Expand Down
25 changes: 10 additions & 15 deletions core/src/test/java/org/apache/iceberg/TestCreateTransaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Sets;
import org.apache.iceberg.types.TypeUtil;
import org.apache.iceberg.types.Types;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -300,11 +301,9 @@ public void testCreateDetectsUncommittedChange() throws IOException {

txn.updateProperties().set("test-property", "test-value"); // not committed

AssertHelpers.assertThrows(
"Should reject commit when last operation has not committed",
IllegalStateException.class,
"Cannot create new DeleteFiles: last operation has not committed",
txn::newDelete);
Assertions.assertThatThrownBy(txn::newDelete)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot create new DeleteFiles: last operation has not committed");
}

@Test
Expand All @@ -323,11 +322,9 @@ public void testCreateDetectsUncommittedChangeOnCommit() throws IOException {

txn.updateProperties().set("test-property", "test-value"); // not committed

AssertHelpers.assertThrows(
"Should reject commit when last operation has not committed",
IllegalStateException.class,
"Cannot commit transaction: last operation has not committed",
txn::commitTransaction);
Assertions.assertThatThrownBy(txn::commitTransaction)
.isInstanceOf(IllegalStateException.class)
.hasMessage("Cannot commit transaction: last operation has not committed");
}

@Test
Expand Down Expand Up @@ -360,11 +357,9 @@ public void testCreateTransactionConflict() throws IOException {
Assert.assertFalse(
"Table should not have any snapshots", conflict.snapshots().iterator().hasNext());

AssertHelpers.assertThrows(
"Transaction commit should fail",
CommitFailedException.class,
"Commit failed: table was updated",
txn::commitTransaction);
Assertions.assertThatThrownBy(txn::commitTransaction)
.isInstanceOf(CommitFailedException.class)
.hasMessageStartingWith("Commit failed: table was updated");

Assert.assertEquals(
"Should clean up metadata",
Expand Down
45 changes: 20 additions & 25 deletions core/src/test/java/org/apache/iceberg/TestDataTableScan.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.assertj.core.api.Assertions;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
Expand Down Expand Up @@ -124,11 +125,10 @@ public void testScanFromRefWhenSnapshotSetFails() {
table.newFastAppend().appendFile(FILE_A).appendFile(FILE_B).commit();
table.manageSnapshots().createTag("tagB", table.currentSnapshot().snapshotId()).commit();

AssertHelpers.assertThrows(
"Should throw when attempting to use a ref for scanning when a snapshot is set",
IllegalArgumentException.class,
"Cannot override ref, already set snapshot id=1",
() -> table.newScan().useSnapshot(table.currentSnapshot().snapshotId()).useRef("tagB"));
Assertions.assertThatThrownBy(
() -> table.newScan().useSnapshot(table.currentSnapshot().snapshotId()).useRef("tagB"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot override ref, already set snapshot id=1");
}

@Test
Expand All @@ -138,11 +138,10 @@ public void testSettingSnapshotWhenRefSetFails() {
table.newFastAppend().appendFile(FILE_B).commit();
table.manageSnapshots().createTag("tagB", table.currentSnapshot().snapshotId()).commit();

AssertHelpers.assertThrows(
"Should throw when attempting to use a snapshot for scanning when a ref is set",
IllegalArgumentException.class,
"Cannot override snapshot, already set snapshot id=2",
() -> table.newScan().useRef("tagB").useSnapshot(snapshotA.snapshotId()));
Assertions.assertThatThrownBy(
() -> table.newScan().useRef("tagB").useSnapshot(snapshotA.snapshotId()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot override snapshot, already set snapshot id=2");
}

@Test
Expand All @@ -152,11 +151,11 @@ public void testBranchTimeTravelFails() {
.manageSnapshots()
.createBranch("testBranch", table.currentSnapshot().snapshotId())
.commit();
AssertHelpers.assertThrows(
"Should throw when attempting to use a snapshot for scanning when a ref is set",
IllegalArgumentException.class,
"Cannot override snapshot, already set snapshot id=1",
() -> table.newScan().useRef("testBranch").asOfTime(System.currentTimeMillis()));

Assertions.assertThatThrownBy(
() -> table.newScan().useRef("testBranch").asOfTime(System.currentTimeMillis()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot override snapshot, already set snapshot id=1");
}

@Test
Expand All @@ -166,20 +165,16 @@ public void testSettingMultipleRefsFails() {
table.newFastAppend().appendFile(FILE_B).commit();
table.manageSnapshots().createTag("tagB", table.currentSnapshot().snapshotId()).commit();

AssertHelpers.assertThrows(
"Should throw when attempting to use multiple refs",
IllegalArgumentException.class,
"Cannot override ref, already set snapshot id=2",
() -> table.newScan().useRef("tagB").useRef("tagA"));
Assertions.assertThatThrownBy(() -> table.newScan().useRef("tagB").useRef("tagA"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot override ref, already set snapshot id=2");
}

@Test
public void testSettingInvalidRefFails() {
AssertHelpers.assertThrows(
"Should throw when attempting to use an invalid ref for scanning",
IllegalArgumentException.class,
"Cannot find ref nonexisting",
() -> table.newScan().useRef("nonexisting"));
Assertions.assertThatThrownBy(() -> table.newScan().useRef("nonexisting"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot find ref nonexisting");
}

private void validateExpectedFileScanTasks(
Expand Down
Loading

0 comments on commit 5c64100

Please sign in to comment.