Cleanup tests (#1856)

* Cleanup tests

* cleanup assertions

* use proper assertion

* use proper assertion

* corrected cast

* jsonEquals matcher

* fix tests

* more processing checks

* remove unnecessary toString

* update ResourceIT test

* create single resource data

* Check returned error.

Co-authored-by: Aaron Klish <aklish@gmail.com>

elide-async/src/test/java/com/yahoo/elide/async/export/formatter/CSVExportFormatterTest.java

@@ -6,7 +6,9 @@
 package com.yahoo.elide.async.export.formatter;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.mockito.Mockito.any;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -92,7 +94,7 @@ public void testResourceToCSV() {
         when(scope.getEntityProjection()).thenReturn(projection);
 
         String output = formatter.format(persistentResource, 1);
-        assertEquals(true, output.contains(row));
+        assertTrue(output.contains(row));
     }
 
     @Test
@@ -101,7 +103,7 @@ public void testNullResourceToCSV() {
         PersistentResource persistentResource = null;
 
         String output = formatter.format(persistentResource, 1);
-        assertEquals(null, output);
+        assertNull(output);
     }
 
     @Test
@@ -114,7 +116,7 @@ public void testNullProjectionHeader() {
         EntityProjection projection = null;
 
         String output = formatter.preFormat(projection, queryObj);
-        assertEquals(null, output);
+        assertNull(output);
     }
 
     @Test
@@ -186,6 +188,6 @@ public void testHeaderSkip() {
         EntityProjection projection = EntityProjection.builder().type(TableExport.class).attributes(attributes).build();
 
         String output = formatter.preFormat(projection, queryObj);
-        assertEquals(null, output);
+        assertNull(output);
     }
 }

elide-async/src/test/java/com/yahoo/elide/async/export/formatter/JSONExportFormatterTest.java

@@ -5,7 +5,8 @@
  */
 package com.yahoo.elide.async.export.formatter;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -91,7 +92,7 @@ public void testFormat() {
         when(scope.getEntityProjection()).thenReturn(projection);
 
         String output = formatter.format(persistentResource, 1);
-        assertEquals(true, output.contains(start));
+        assertTrue(output.contains(start));
     }
 
     @Test
@@ -122,7 +123,7 @@ public void testResourceToJSON() {
         when(scope.getEntityProjection()).thenReturn(projection);
 
         String output = formatter.resourceToJSON(elide.getMapper().getObjectMapper(), persistentResource);
-        assertEquals(true, output.contains(start));
+        assertTrue(output.contains(start));
     }
 
     @Test
@@ -131,6 +132,6 @@ public void testNullResourceToJSON() {
         PersistentResource persistentResource = null;
 
         String output = formatter.format(persistentResource, 1);
-        assertEquals(null, output);
+        assertNull(output);
     }
 }

elide-async/src/test/java/com/yahoo/elide/async/operation/GraphQLTableExportOperationTest.java

@@ -7,7 +7,7 @@
 
 import static com.yahoo.elide.core.dictionary.EntityDictionary.NO_VERSION;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -101,7 +101,7 @@ public void testProcessQuery() throws URISyntaxException, IOException  {
         TableExportResult queryResultObj = (TableExportResult) graphQLOperation.call();
 
         assertEquals(200, queryResultObj.getHttpStatus());
-        assertTrue("https://elide.io/export/edc4a871-dff2-4054-804e-d80075cf827d".equals(queryResultObj.getUrl().toString()));
+        assertEquals("https://elide.io/export/edc4a871-dff2-4054-804e-d80075cf827d", queryResultObj.getUrl().toString());
         assertEquals(1, queryResultObj.getRecordCount());
     }
 
@@ -160,8 +160,8 @@ public void testProcessQueryWithRelationship() {
         assertEquals(200, queryResultObj.getHttpStatus());
         assertEquals("Export is not supported for Query that requires traversing Relationships.",
                         queryResultObj.getMessage());
-        assertEquals(null, queryResultObj.getRecordCount());
-        assertEquals(null, queryResultObj.getUrl());
+        assertNull(queryResultObj.getRecordCount());
+        assertNull(queryResultObj.getUrl());
     }
 
     @Test
@@ -181,8 +181,8 @@ public void testProcessQueryWithMultipleProjection() {
         assertEquals(200, queryResultObj.getHttpStatus());
         assertEquals("Export is only supported for single Query with one root projection.",
                         queryResultObj.getMessage());
-        assertEquals(null, queryResultObj.getRecordCount());
-        assertEquals(null, queryResultObj.getUrl());
+        assertNull(queryResultObj.getRecordCount());
+        assertNull(queryResultObj.getUrl());
     }
 
     @Test
@@ -202,8 +202,8 @@ public void testProcessMultipleQuery() {
         assertEquals(200, queryResultObj.getHttpStatus());
         assertEquals("Export is only supported for single Query with one root projection.",
                         queryResultObj.getMessage());
-        assertEquals(null, queryResultObj.getRecordCount());
-        assertEquals(null, queryResultObj.getUrl());
+        assertNull(queryResultObj.getRecordCount());
+        assertNull(queryResultObj.getUrl());
     }
 
     /**

elide-async/src/test/java/com/yahoo/elide/async/operation/JSONAPIAsyncQueryOperationTest.java

@@ -6,6 +6,7 @@
 package com.yahoo.elide.async.operation;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -76,6 +77,6 @@ public void testProcessQueryNonSuccessResponse() throws URISyntaxException {
         AsyncQueryResult queryResultObj = (AsyncQueryResult) jsonOperation.call();
         assertEquals(responseBody, queryResultObj.getResponseBody());
         assertEquals(201, queryResultObj.getHttpStatus());
-        assertEquals(null, queryResultObj.getRecordCount());
+        assertNull(queryResultObj.getRecordCount());
     }
 }

elide-async/src/test/java/com/yahoo/elide/async/operation/JsonAPITableExportOperationTest.java

@@ -8,7 +8,6 @@
 import static com.yahoo.elide.core.dictionary.EntityDictionary.NO_VERSION;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -103,7 +102,7 @@ public void testProcessQuery() throws URISyntaxException, IOException  {
         TableExportResult queryResultObj = (TableExportResult) jsonAPIOperation.call();
 
         assertEquals(200, queryResultObj.getHttpStatus());
-        assertTrue("https://elide.io/export/edc4a871-dff2-4054-804e-d80075cf827d".equals(queryResultObj.getUrl().toString()));
+        assertEquals("https://elide.io/export/edc4a871-dff2-4054-804e-d80075cf827d", queryResultObj.getUrl().toString());
         assertEquals(1, queryResultObj.getRecordCount());
         assertNull(queryResultObj.getMessage());
     }
@@ -164,8 +163,8 @@ public void testProcessQueryWithRelationship() {
         assertEquals(200, queryResultObj.getHttpStatus());
         assertEquals("Export is not supported for Query that requires traversing Relationships.",
                         queryResultObj.getMessage());
-        assertEquals(null, queryResultObj.getRecordCount());
-        assertEquals(null, queryResultObj.getUrl());
+        assertNull(queryResultObj.getRecordCount());
+        assertNull(queryResultObj.getUrl());
     }
 
     /**

elide-core/src/main/java/com/yahoo/elide/core/exceptions/InvalidParameterizedAttributeException.java

@@ -13,8 +13,7 @@
  */
 public class InvalidParameterizedAttributeException extends HttpStatusException {
     public InvalidParameterizedAttributeException(Attribute attribute) {
-        super(HttpStatus.SC_BAD_REQUEST, "No attribute found with matching parameters for attribute: "
-                + attribute.toString());
+        super(HttpStatus.SC_BAD_REQUEST, "No attribute found with matching parameters for attribute: " + attribute);
     }
 
     public InvalidParameterizedAttributeException(String attributeName, Argument argument) {

elide-core/src/test/java/com/yahoo/elide/core/PersistentResourceTest.java

@@ -13,7 +13,6 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -2108,10 +2107,7 @@ public void testTransferPermissionSuccessOnClearSingularRelationship() {
     public void testCollectionChangeSpecType() {
         Function<String, BiFunction<ChangeSpec, BiFunction<Collection, Collection, Boolean>, Boolean>> collectionCheck =
                 (fieldName) -> (spec, condFn) -> {
-                    if (!fieldName.equals(spec.getFieldName())) {
-                        fail("Should not reach here");
-                        throw new IllegalStateException();
-                    }
+                    assertEquals(fieldName, spec.getFieldName());
                     return condFn.apply((Collection) spec.getOriginal(), (Collection) spec.getModified());
                 };
 
@@ -2172,14 +2168,8 @@ public void testCollectionChangeSpecType() {
     @Test
     public void testAttrChangeSpecType() {
         BiFunction<ChangeSpec, BiFunction<String, String, Boolean>, Boolean> attrCheck = (spec, checkFn) -> {
-            if (!(spec.getModified() instanceof String) && spec.getModified() != null) {
-                fail("Should not reach here");
-                return false;
-            }
-            if (!"testAttr".equals(spec.getFieldName())) {
-                fail("Should not reach here");
-                return false;
-            }
+            assertTrue(spec.getModified() instanceof String || spec.getModified() == null);
+            assertEquals("testAttr", spec.getFieldName());
             return checkFn.apply((String) spec.getOriginal(), (String) spec.getModified());
         };
 
@@ -2196,14 +2186,8 @@ public void testAttrChangeSpecType() {
     @Test
     public void testRelationChangeSpecType() {
             BiFunction<ChangeSpec, BiFunction<ChangeSpecChild, ChangeSpecChild, Boolean>, Boolean> relCheck = (spec, checkFn) -> {
-                if (!(spec.getModified() instanceof ChangeSpecChild) && spec.getModified() != null) {
-                    fail("Should not reach here");
-                    return false;
-                }
-                if (!"child".equals(spec.getFieldName())) {
-                    fail("Should not reach here");
-                    return false;
-                }
+                assertTrue(spec.getModified() instanceof ChangeSpecChild ||  spec.getModified() == null);
+                assertEquals("child", spec.getFieldName());
                 return checkFn.apply((ChangeSpecChild) spec.getOriginal(), (ChangeSpecChild) spec.getModified());
             };
 

elide-core/src/test/java/com/yahoo/elide/core/audit/LogMessageImplTest.java

@@ -6,11 +6,11 @@
 package com.yahoo.elide.core.audit;
 
 import static com.yahoo.elide.core.dictionary.EntityDictionary.NO_VERSION;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 import com.yahoo.elide.ElideSettingsBuilder;
 import com.yahoo.elide.core.PersistentResource;
 import com.yahoo.elide.core.RequestScope;
@@ -142,20 +142,11 @@ public String getMessage() {
                 throw testException;
             }
         };
-        try {
-            testAuditLogger.log(failMessage);
-            Thread.sleep(Math.floorMod(ThreadLocalRandom.current().nextInt(), 100));
-            testAuditLogger.commit();
-            fail("Exception expected");
-        } catch (TestLoggerException e) {
-            assertSame(e, testException);
-        }
+
+        assertSame(assertThrows(TestLoggerException.class, () -> testAuditLogger.log(failMessage)), testException);
+        Thread.sleep(Math.floorMod(ThreadLocalRandom.current().nextInt(), 100));
 
         // should not cause another exception
-        try {
-            testAuditLogger.commit();
-        } catch (TestLoggerException e) {
-            fail("Exception not cleared from previous logger commit");
-        }
+        assertDoesNotThrow(() -> testAuditLogger.commit(), "Exception not cleared from previous logger commit");
     }
 }

elide-core/src/test/java/com/yahoo/elide/core/dictionary/EntityDictionaryTest.java

@@ -675,7 +675,7 @@ class SuperclassBinding {
 
         bindEntity(SuperclassBinding.class);
 
-        assertEquals(null, getEntityBinding(new ClassType(SuperclassBinding.class)).entityClass);
+        assertNull(getEntityBinding(new ClassType(SuperclassBinding.class)).entityClass);
         assertEquals(new ClassType(SuperclassBinding.class), lookupEntityClass(new ClassType(SuperclassBinding.class)));
     }
 
@@ -758,7 +758,7 @@ class SubsubclassBinding extends SubclassBinding {
 
         assertEquals(new ClassType(SuperclassBinding.class), lookupIncludeClass(new ClassType(SuperclassBinding.class)));
         assertEquals(new ClassType(SuperclassBinding.class), lookupIncludeClass(new ClassType(SubclassBinding.class)));
-        assertEquals(null, lookupIncludeClass(new ClassType(SubsubclassBinding.class)));
+        assertNull(lookupIncludeClass(new ClassType(SubsubclassBinding.class)));
     }
 
     @Test
@@ -932,7 +932,7 @@ public String toString() {
 
         assertThrows(InvalidAttributeException.class, () -> setValue(book, "badfield", "Elide"));
         assertEquals("1234", getId(book));
-        assertEquals(true, isRoot(new ClassType(Book.class)));
+        assertTrue(isRoot(new ClassType(Book.class)));
 
         assertEquals(new ClassType(Book.class), lookupBoundClass(new ClassType(Book.class)));
         assertNull(lookupBoundClass(new ClassType(String.class)));

elide-core/src/test/java/com/yahoo/elide/core/filter/FilterPredicateTest.java

@@ -140,7 +140,7 @@ void testMultipleFieldMultipleValues() {
                     assertEquals(Arrays.asList("def", "jkl"), predicate.getValues());
                     break;
                 default:
-                    fail(predicate.toString() + " case not covered");
+                    fail(predicate + " case not covered");
             }
         }
     }

elide-core/src/test/java/com/yahoo/elide/core/utils/coerce/CoerceUtilTest.java

@@ -68,7 +68,7 @@ public void testNoConversions() throws Exception {
         assertEquals(1, CoerceUtil.coerce(1, (Class<Object>) null),
                      "coerce returns value if target class null");
 
-        assertEquals(null, CoerceUtil.coerce(null, Object.class),
+        assertNull(CoerceUtil.coerce(null, Object.class),
                      "coerce returns value if value is null");
 
         assertEquals(1, (Object) CoerceUtil.coerce(1, int.class),

elide-datastore/elide-datastore-aggregation/src/main/java/com/yahoo/elide/datastores/aggregation/queryengines/sql/query/QueryTranslator.java

@@ -58,7 +58,7 @@ public SQLQuery.SQLQueryBuilder visitQuery(Query query) {
         if (query.isNested()) {
             SQLQuery innerQuery = builder.build();
 
-            builder = SQLQuery.builder().fromClause("(" + innerQuery.toString() + ") AS "
+            builder = SQLQuery.builder().fromClause("(" + innerQuery + ") AS "
                     + applyQuotes(query.getSource().getAlias()));
         }
 

elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/dynamic/TableTypeTest.java

@@ -96,7 +96,7 @@ void testTableAnnotations() throws Exception {
         assertEquals(CardinalitySize.MEDIUM, tableMeta.size());
         assertEquals("A test table", tableMeta.description());
         assertEquals("category1", tableMeta.category());
-        assertEquals(true, tableMeta.isFact());
+        assertTrue(tableMeta.isFact());
         assertEquals(tags, new HashSet<>(Arrays.asList(tableMeta.tags())));
         assertEquals("a==b", tableMeta.filterTemplate());
 

elide-datastore/elide-datastore-aggregation/src/test/java/com/yahoo/elide/datastores/aggregation/framework/SQLUnitTest.java

@@ -10,7 +10,7 @@
 import static com.yahoo.elide.core.utils.TypeHelper.getClassType;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import com.yahoo.elide.core.Path;
 import com.yahoo.elide.core.dictionary.EntityDictionary;
@@ -536,18 +536,11 @@ protected void compareQueryLists(String expected, List<String> actual) {
      * Helper for comparing lists of queries.
      */
     protected void compareQueryLists(List<String> expected, List<String> actual) {
-        if (expected == null && actual == null) {
+        if (expected == actual) {
             return;
         }
-        if (expected == null) {
-            fail("Expected a null query List, but actual was non-null");
-            return;
-        }
-        if (actual == null) {
-            fail("Expected a non-null query List, but actual was null");
-            return;
-        }
-
+        assertNotNull(expected, "Expected a null query List, but actual was non-null");
+        assertNotNull(actual, "Expected a non-null query List, but actual was null");
         assertEquals(expected.size(), actual.size(), "Query List sizes do not match");