Move Assert.assertTrue(..) instance checks to AssertJ assertions (#2756)

Author: nastra

api/src/test/java/org/apache/iceberg/expressions/TestExpressionBinding.java

@@ -23,6 +23,7 @@
 import org.apache.iceberg.exceptions.ValidationException;
 import org.apache.iceberg.types.Types;
 import org.apache.iceberg.types.Types.StructType;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -182,7 +183,7 @@ public void testTransformExpressionBinding() {
     Expression bound = Binder.bind(STRUCT, equal(bucket("x", 16), 10));
     TestHelpers.assertAllReferencesBound("BoundTransform", bound);
     BoundPredicate<?> pred = TestHelpers.assertAndUnwrap(bound);
-    Assert.assertTrue("Should use a BoundTransform child", pred.term() instanceof BoundTransform);
+    Assertions.assertThat(pred.term()).as("Should use a BoundTransform child").isInstanceOf(BoundTransform.class);
     BoundTransform<?, ?> transformExpr = (BoundTransform<?, ?>) pred.term();
     Assert.assertEquals("Should use a bucket[16] transform", "bucket[16]", transformExpr.transform().toString());
   }

api/src/test/java/org/apache/iceberg/transforms/TestProjection.java

@@ -31,6 +31,7 @@
 import org.apache.iceberg.expressions.UnboundPredicate;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -259,12 +260,12 @@ public void testBadSparkPartitionFilter() {
 
     Expression projection = Projections.inclusive(spec).project(filter);
 
-    Assert.assertTrue(projection instanceof Or);
+    Assertions.assertThat(projection).isInstanceOf(Or.class);
     Or or1 = (Or) projection;
     UnboundPredicate<?> dateint1 = assertAndUnwrapUnbound(or1.left());
     Assert.assertEquals("Should be a dateint predicate", "dateint", dateint1.ref().name());
     Assert.assertEquals("Should be dateint=20180416", 20180416, dateint1.literal().value());
-    Assert.assertTrue(or1.right() instanceof Or);
+    Assertions.assertThat(or1.right()).isInstanceOf(Or.class);
     Or or2 = (Or) or1.right();
     UnboundPredicate<?> dateint2 = assertAndUnwrapUnbound(or2.left());
     Assert.assertEquals("Should be a dateint predicate", "dateint", dateint2.ref().name());

api/src/test/java/org/apache/iceberg/transforms/TestStartsWith.java

@@ -31,6 +31,7 @@
 import org.apache.iceberg.expressions.Projections;
 import org.apache.iceberg.expressions.UnboundPredicate;
 import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -55,7 +56,7 @@ public void testTruncateProjections() {
     assertProjectionStrict(spec, startsWith(COLUMN, "abab"), "abab", Expression.Operation.EQ);
 
     Expression projection = Projections.strict(spec).project(startsWith(COLUMN, "ababab"));
-    Assert.assertTrue(projection instanceof False);
+    Assertions.assertThat(projection).isInstanceOf(False.class);
   }
 
   @Test

aws/src/test/java/org/apache/iceberg/aws/glue/LockManagersTest.java

@@ -22,21 +22,21 @@
 import java.util.Map;
 import org.apache.iceberg.CatalogProperties;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
-import org.junit.Assert;
+import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
 public class LockManagersTest {
 
   @Test
   public void testLoadDefaultLockManager() {
-    Assert.assertTrue(LockManagers.defaultLockManager() instanceof LockManagers.InMemoryLockManager);
+    Assertions.assertThat(LockManagers.defaultLockManager()).isInstanceOf(LockManagers.InMemoryLockManager.class);
   }
 
   @Test
   public void testLoadCustomLockManager() {
     Map<String, String> properties = Maps.newHashMap();
     properties.put(CatalogProperties.LOCK_IMPL, CustomLockManager.class.getName());
-    Assert.assertTrue(LockManagers.from(properties) instanceof CustomLockManager);
+    Assertions.assertThat(LockManagers.from(properties)).isInstanceOf(CustomLockManager.class);
   }
 
   static class CustomLockManager implements LockManager {

core/src/test/java/org/apache/iceberg/TestCatalogUtil.java

@@ -32,6 +32,7 @@
 import org.apache.iceberg.io.InputFile;
 import org.apache.iceberg.io.OutputFile;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -44,7 +45,7 @@ public void loadCustomCatalog() {
     Configuration hadoopConf = new Configuration();
     String name = "custom";
     Catalog catalog = CatalogUtil.loadCatalog(TestCatalog.class.getName(), name, options, hadoopConf);
-    Assert.assertTrue(catalog instanceof TestCatalog);
+    Assertions.assertThat(catalog).isInstanceOf(TestCatalog.class);
     Assert.assertEquals(name, ((TestCatalog) catalog).catalogName);
     Assert.assertEquals(options, ((TestCatalog) catalog).flinkOptions);
   }
@@ -57,7 +58,7 @@ public void loadCustomCatalog_withHadoopConfig() {
     hadoopConf.set("key", "val");
     String name = "custom";
     Catalog catalog = CatalogUtil.loadCatalog(TestCatalogConfigurable.class.getName(), name, options, hadoopConf);
-    Assert.assertTrue(catalog instanceof TestCatalogConfigurable);
+    Assertions.assertThat(catalog).isInstanceOf(TestCatalogConfigurable.class);
     Assert.assertEquals(name, ((TestCatalogConfigurable) catalog).catalogName);
     Assert.assertEquals(options, ((TestCatalogConfigurable) catalog).flinkOptions);
     Assert.assertEquals(hadoopConf, ((TestCatalogConfigurable) catalog).configuration);
@@ -120,7 +121,7 @@ public void loadCustomFileIO_noArg() {
     Map<String, String> properties = Maps.newHashMap();
     properties.put("key", "val");
     FileIO fileIO = CatalogUtil.loadFileIO(TestFileIONoArg.class.getName(), properties, null);
-    Assert.assertTrue(fileIO instanceof TestFileIONoArg);
+    Assertions.assertThat(fileIO).isInstanceOf(TestFileIONoArg.class);
     Assert.assertEquals(properties, ((TestFileIONoArg) fileIO).map);
   }
 
@@ -129,7 +130,7 @@ public void loadCustomFileIO_hadoopConfigConstructor() {
     Configuration configuration = new Configuration();
     configuration.set("key", "val");
     FileIO fileIO = CatalogUtil.loadFileIO(HadoopFileIO.class.getName(), Maps.newHashMap(), configuration);
-    Assert.assertTrue(fileIO instanceof HadoopFileIO);
+    Assertions.assertThat(fileIO).isInstanceOf(HadoopFileIO.class);
     Assert.assertEquals("val", ((HadoopFileIO) fileIO).conf().get("key"));
   }
 
@@ -138,7 +139,7 @@ public void loadCustomFileIO_configurable() {
     Configuration configuration = new Configuration();
     configuration.set("key", "val");
     FileIO fileIO = CatalogUtil.loadFileIO(TestFileIOConfigurable.class.getName(), Maps.newHashMap(), configuration);
-    Assert.assertTrue(fileIO instanceof TestFileIOConfigurable);
+    Assertions.assertThat(fileIO).isInstanceOf(TestFileIOConfigurable.class);
     Assert.assertEquals(configuration, ((TestFileIOConfigurable) fileIO).configuration);
   }
 

core/src/test/java/org/apache/iceberg/TestSortOrderParser.java

@@ -47,7 +47,7 @@ public void testUnknownTransforms() {
 
     Assert.assertEquals(10, order.orderId());
     Assert.assertEquals(1, order.fields().size());
-    Assert.assertTrue(order.fields().get(0).transform() instanceof UnknownTransform);
+    org.assertj.core.api.Assertions.assertThat(order.fields().get(0).transform()).isInstanceOf(UnknownTransform.class);
     Assert.assertEquals("custom_transform", order.fields().get(0).transform().toString());
     Assert.assertEquals(2, order.fields().get(0).sourceId());
     Assert.assertEquals(DESC, order.fields().get(0).direction());

core/src/test/java/org/apache/iceberg/avro/AvroTestHelpers.java

@@ -27,6 +27,7 @@
 import org.apache.avro.generic.GenericData.Record;
 import org.apache.iceberg.types.Type;
 import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 
 import static org.apache.iceberg.avro.AvroSchemaUtil.toOption;
@@ -129,18 +130,18 @@ private static void assertEquals(Type type, Object expected, Object actual) {
         Assert.assertEquals("Primitive value should be equal to expected", expected, actual);
         break;
       case STRUCT:
-        Assert.assertTrue("Expected should be a Record", expected instanceof Record);
-        Assert.assertTrue("Actual should be a Record", actual instanceof Record);
+        Assertions.assertThat(expected).as("Expected should be a Record").isInstanceOf(Record.class);
+        Assertions.assertThat(actual).as("Actual should be a Record").isInstanceOf(Record.class);
         assertEquals(type.asStructType(), (Record) expected, (Record) actual);
         break;
       case LIST:
-        Assert.assertTrue("Expected should be a List", expected instanceof List);
-        Assert.assertTrue("Actual should be a List", actual instanceof List);
+        Assertions.assertThat(expected).as("Expected should be a List").isInstanceOf(List.class);
+        Assertions.assertThat(actual).as("Actual should be a List").isInstanceOf(List.class);
         assertEquals(type.asListType(), (List) expected, (List) actual);
         break;
       case MAP:
-        Assert.assertTrue("Expected should be a Map", expected instanceof Map);
-        Assert.assertTrue("Actual should be a Map", actual instanceof Map);
+        Assertions.assertThat(expected).as("Expected should be a Map").isInstanceOf(Map.class);
+        Assertions.assertThat(actual).as("Actual should be a Map").isInstanceOf(Map.class);
         assertEquals(type.asMapType(), (Map<?, ?>) expected, (Map<?, ?>) actual);
         break;
       default:

core/src/test/java/org/apache/iceberg/util/TestExceptionUtil.java

@@ -20,6 +20,7 @@
 package org.apache.iceberg.util;
 
 import java.io.IOException;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -56,11 +57,11 @@ public void testRunSafely() {
       Assert.assertEquals("Should have 2 suppressed exceptions", 2, e.getSuppressed().length);
 
       Throwable throwSuppressed = e.getSuppressed()[0];
-      Assert.assertTrue("Should be an Exception", throwSuppressed instanceof Exception);
+      Assertions.assertThat(throwSuppressed).as("Should be an Exception").isInstanceOf(Exception.class);
       Assert.assertEquals("Should have correct message", "test catch suppression", throwSuppressed.getMessage());
 
       Throwable finallySuppressed = e.getSuppressed()[1];
-      Assert.assertTrue("Should be a RuntimeException", finallySuppressed instanceof RuntimeException);
+      Assertions.assertThat(finallySuppressed).as("Should be a RuntimeException").isInstanceOf(RuntimeException.class);
       Assert.assertEquals("Should have correct message", "test finally suppression", finallySuppressed.getMessage());
     }
   }
@@ -90,11 +91,11 @@ public void testRunSafelyTwoExceptions() {
       Assert.assertEquals("Should have 2 suppressed exceptions", 2, e.getSuppressed().length);
 
       Throwable throwSuppressed = e.getSuppressed()[0];
-      Assert.assertTrue("Should be an Exception", throwSuppressed instanceof Exception);
+      Assertions.assertThat(throwSuppressed).as("Should be an Exception").isInstanceOf(Exception.class);
       Assert.assertEquals("Should have correct message", "test catch suppression", throwSuppressed.getMessage());
 
       Throwable finallySuppressed = e.getSuppressed()[1];
-      Assert.assertTrue("Should be a RuntimeException", finallySuppressed instanceof RuntimeException);
+      Assertions.assertThat(finallySuppressed).as("Should be a RuntimeException").isInstanceOf(RuntimeException.class);
       Assert.assertEquals("Should have correct message", "test finally suppression", finallySuppressed.getMessage());
     }
   }
@@ -125,11 +126,11 @@ public void testRunSafelyThreeExceptions() {
       Assert.assertEquals("Should have 2 suppressed exceptions", 2, e.getSuppressed().length);
 
       Throwable throwSuppressed = e.getSuppressed()[0];
-      Assert.assertTrue("Should be an Exception", throwSuppressed instanceof Exception);
+      Assertions.assertThat(throwSuppressed).as("Should be an Exception").isInstanceOf(Exception.class);
       Assert.assertEquals("Should have correct message", "test catch suppression", throwSuppressed.getMessage());
 
       Throwable finallySuppressed = e.getSuppressed()[1];
-      Assert.assertTrue("Should be a RuntimeException", finallySuppressed instanceof RuntimeException);
+      Assertions.assertThat(finallySuppressed).as("Should be a RuntimeException").isInstanceOf(RuntimeException.class);
       Assert.assertEquals("Should have correct message", "test finally suppression", finallySuppressed.getMessage());
     }
   }
@@ -156,11 +157,12 @@ public void testRunSafelyRuntimeExceptions() {
       Assert.assertEquals("Should have 2 suppressed exceptions", 2, e.getSuppressed().length);
 
       Throwable throwSuppressed = e.getSuppressed()[0];
-      Assert.assertTrue("Should be an Exception", throwSuppressed instanceof Exception);
+      Assertions.assertThat(throwSuppressed).as("Should be an Exception").isInstanceOf(Exception.class);
       Assert.assertEquals("Should have correct message", "test catch suppression", throwSuppressed.getMessage());
 
       Throwable finallySuppressed = e.getSuppressed()[1];
-      Assert.assertTrue("Should be a CustomCheckedException", finallySuppressed instanceof CustomCheckedException);
+      Assertions.assertThat(finallySuppressed).as("Should be a CustomCheckedException")
+          .isInstanceOf(CustomCheckedException.class);
       Assert.assertEquals("Should have correct message", "test finally suppression", finallySuppressed.getMessage());
     }
   }

data/src/test/java/org/apache/iceberg/data/DataTestHelpers.java

@@ -23,6 +23,7 @@
 import java.util.Map;
 import org.apache.iceberg.types.Type;
 import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
 import org.junit.Assert;
 
 public class DataTestHelpers {
@@ -88,24 +89,24 @@ private static void assertEquals(Type type, Object expected, Object actual) {
         Assert.assertEquals("Primitive value should be equal to expected for type " + type, expected, actual);
         break;
       case FIXED:
-        Assert.assertTrue("Expected should be a byte[]", expected instanceof byte[]);
-        Assert.assertTrue("Actual should be a byte[]", actual instanceof byte[]);
+        Assertions.assertThat(expected).as("Expected should be a byte[]").isInstanceOf(byte[].class);
+        Assertions.assertThat(expected).as("Actual should be a byte[]").isInstanceOf(byte[].class);
         Assert.assertArrayEquals("Array contents should be equal",
             (byte[]) expected, (byte[]) actual);
         break;
       case STRUCT:
-        Assert.assertTrue("Expected should be a Record", expected instanceof Record);
-        Assert.assertTrue("Actual should be a Record", actual instanceof Record);
+        Assertions.assertThat(expected).as("Expected should be a Record").isInstanceOf(Record.class);
+        Assertions.assertThat(actual).as("Actual should be a Record").isInstanceOf(Record.class);
         assertEquals(type.asStructType(), (Record) expected, (Record) actual);
         break;
       case LIST:
-        Assert.assertTrue("Expected should be a List", expected instanceof List);
-        Assert.assertTrue("Actual should be a List", actual instanceof List);
+        Assertions.assertThat(expected).as("Expected should be a List").isInstanceOf(List.class);
+        Assertions.assertThat(actual).as("Actual should be a List").isInstanceOf(List.class);
         assertEquals(type.asListType(), (List) expected, (List) actual);
         break;
       case MAP:
-        Assert.assertTrue("Expected should be a Map", expected instanceof Map);
-        Assert.assertTrue("Actual should be a Map", actual instanceof Map);
+        Assertions.assertThat(expected).as("Expected should be a Map").isInstanceOf(Map.class);
+        Assertions.assertThat(actual).as("Actual should be a Map").isInstanceOf(Map.class);
         assertEquals(type.asMapType(), (Map<?, ?>) expected, (Map<?, ?>) actual);
         break;
       default:

flink/src/test/java/org/apache/iceberg/flink/TestCatalogTableLoader.java

@@ -35,6 +35,7 @@
 import org.apache.iceberg.io.FileIO;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.apache.iceberg.types.Types;
+import org.assertj.core.api.Assertions;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -107,7 +108,7 @@ private static void validateTableLoader(TableLoader loader) throws IOException,
 
   private static void validateHadoopConf(Table table) {
     FileIO io = table.io();
-    Assert.assertTrue("FileIO should be a HadoopFileIO", io instanceof HadoopFileIO);
+    Assertions.assertThat(io).as("FileIO should be a HadoopFileIO").isInstanceOf(HadoopFileIO.class);
     HadoopFileIO hadoopIO = (HadoopFileIO) io;
     Assert.assertEquals("my_value", hadoopIO.conf().get("my_key"));
   }