Merge pull request #215 from strangelookingnerd/improve_assertions

Replace try-catch constructs in tests with assertThrows
apache · Jan 11, 2025 · 7a3b96f · 7a3b96f
2 parents 51ce0f8 + 1c92047
commit 7a3b96f
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 175 deletions.
diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java
@@ -17,6 +17,7 @@
 package org.apache.commons.jxpath.issues;
 
 import java.io.StringReader;
+import java.util.List;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -28,6 +29,9 @@
 import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
 
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 public class JXPath113Test extends AbstractJXPathTest
 {
 
@@ -36,7 +40,10 @@ public void testIssue113() throws Exception
     {
         final Document doc = JAXP.getDocument("<xml/>");
         final JXPathContext context = JXPathContext.newContext(doc);
-        context.selectNodes("//following-sibling::node()");
+
+        List result = context.selectNodes("//following-sibling::node()");
+        assertNotNull(result);
+        assertTrue(result.isEmpty());
     }
 
     static class JAXP
@@ -54,21 +61,12 @@ public static Document getDocument(final InputSource is) throws Exception
             return builder.parse(is);
         }
 
-        private static DocumentBuilder getDocumentBuilder()
-        {
-            try
-            {
-                final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
-                factory.setValidating(false);
-                factory.setNamespaceAware(true);
-                factory.setExpandEntityReferences(false);
-                return factory.newDocumentBuilder();
-            }
-            catch (final ParserConfigurationException e)
-            {
-                throw new Error("JAXP config error:" + e.getMessage(), e);
-            }
-
+        private static DocumentBuilder getDocumentBuilder() throws ParserConfigurationException {
+            final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            factory.setValidating(false);
+            factory.setNamespaceAware(true);
+            factory.setExpandEntityReferences(false);
+            return factory.newDocumentBuilder();
         }
     }
 

diff --git a/src/test/java/org/apache/commons/jxpath/issues/JXPath172Test.java b/src/test/java/org/apache/commons/jxpath/issues/JXPath172Test.java
@@ -28,7 +28,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class JXPath172Test extends AbstractJXPathTest
 {
@@ -90,36 +90,10 @@ public void testIssue172_NestedPropertyUnexisting()
     public void testIssue172_propertyDoesNotExist_NotLenient()
     {
         final JXPathContext context = getContext(null, false);
-        try
-        {
-            final Object bRet = context.selectSingleNode("unexisting");
-            fail("" + bRet);
-        }
-        catch (final JXPathNotFoundException e)
-        {
-
-        }
-
-        try
-        {
-            final Pointer pointer = context.getPointer("unexisting");
-            fail(" " + pointer);
-        }
-        catch (final JXPathNotFoundException e)
-        {
-
-        }
-
-        try
-        {
-            final Pointer pointer = context.getPointer("value.unexisting");
-            fail(" " + pointer);
-        }
-        catch (final JXPathNotFoundException e)
-        {
-
-        }
 
+        assertThrows(JXPathNotFoundException.class, () -> context.selectSingleNode("unexisting"));
+        assertThrows(JXPathNotFoundException.class, () -> context.getPointer("unexisting"));
+        assertThrows(JXPathNotFoundException.class, () -> context.getPointer("value.unexisting"));
     }
 
     /**

diff --git a/src/test/java/org/apache/commons/jxpath/ri/ExceptionHandlerTest.java b/src/test/java/org/apache/commons/jxpath/ri/ExceptionHandlerTest.java
@@ -21,6 +21,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
@@ -57,34 +58,30 @@ public Object getFoo() {
 
     @Test
     public void testHandleFoo() throws Exception {
-        try {
-            context.getValue("foo");
-            fail("expected Throwable");
-        } catch (Throwable t) {
-            while (t != null) {
-                if ("foo unavailable".equals(t.getMessage())) {
-                    return;
-                }
-                t = t.getCause();
+        Throwable t = assertThrows(Throwable.class, () -> context.getValue("foo"),
+            "expected Throwable");
+
+        while (t != null) {
+            if ("foo unavailable".equals(t.getMessage())) {
+                return;
             }
-            fail("expected \"foo unavailable\" in throwable chain");
+            t = t.getCause();
         }
+        fail("expected \"foo unavailable\" in throwable chain");
     }
 
     @Test
     public void testHandleBarBaz() throws Exception {
-        try {
-            context.getValue("bar/baz");
-            fail("expected Throwable");
-        } catch (Throwable t) {
-            while (t != null) {
-                if ("baz unavailable".equals(t.getMessage())) {
-                    return;
-                }
-                t = t.getCause();
+        Throwable t = assertThrows(Throwable.class, () -> context.getValue("bar/baz"),
+            "expected Throwable");
+
+        while (t != null) {
+            if ("baz unavailable".equals(t.getMessage())) {
+                return;
             }
-            fail("expected \"baz unavailable\" in throwable chain");
+            t = t.getCause();
         }
+        fail("expected \"baz unavailable\" in throwable chain");
     }
 
     public Bar getBar() {

diff --git a/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java b/src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java
@@ -33,7 +33,7 @@ public void testInit() {
             JXPathContextReferenceImpl.addNodePointerFactory(factory);
         } finally {
             while (JXPathContextReferenceImpl.removeNodePointerFactory(factory)) {
-
+                // NOP
             }
         }
     }

diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java
@@ -63,14 +63,8 @@ public void testCoreFunctions() {
         assertXPathValue(context, "ends-with('xabc', 'ab')", Boolean.FALSE);
         assertXPathValue(context, "contains('xabc', 'ab')", Boolean.TRUE);
         assertXPathValue(context, "contains('xabc', 'ba')", Boolean.FALSE);
-        assertXPathValue(
-            context,
-            "substring-before('1999/04/01', '/')",
-            "1999");
-        assertXPathValue(
-            context,
-            "substring-after('1999/04/01', '/')",
-            "04/01");
+        assertXPathValue(context,"substring-before('1999/04/01', '/')","1999");
+        assertXPathValue(context,"substring-after('1999/04/01', '/')","04/01");
         assertXPathValue(context, "substring('12345', 2, 3)", "234");
         assertXPathValue(context, "substring('12345', 2)", "2345");
         assertXPathValue(context, "substring('12345', 1.5, 2.6)", "234");

diff --git a/src/test/java/org/apache/commons/jxpath/ri/compiler/VariableTest.java b/src/test/java/org/apache/commons/jxpath/ri/compiler/VariableTest.java
@@ -25,7 +25,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Test basic functionality of JXPath - infoset types,
@@ -73,26 +73,10 @@ public void testVariablesInExpressions() {
 
     @Test
     public void testInvalidVariableName() {
-        boolean exception = false;
-        try {
-            context.getValue("$none");
-        }
-        catch (final Exception ex) {
-            exception = true;
-        }
-        assertTrue(
-            exception,
+        assertThrows(Exception.class, () -> context.getValue("$none"),
             "Evaluating '$none', expected exception - did not get it");
 
-        exception = false;
-        try {
-            context.setValue("$none", Integer.valueOf(1));
-        }
-        catch (final Exception ex) {
-            exception = true;
-        }
-        assertTrue(
-            exception,
+        assertThrows(Exception.class, () -> context.setValue("$none", Integer.valueOf(1)),
             "Setting '$none = 1', expected exception - did not get it");
     }
 

diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/AbstractBeanModelTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/AbstractBeanModelTest.java
@@ -39,7 +39,7 @@
 
 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.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Abstract superclass for Bean access with JXPath.
@@ -827,19 +827,13 @@ public void testCreatePath() {
             Integer.valueOf(1),
             "/nestedBean/int");
 
-        boolean ex = false;
-        try {
+        assertThrows(Exception.class, () ->
             assertXPathCreatePath(
                 context,
                 "/nestedBean/beans[last() + 1]",
                 Integer.valueOf(1),
-                "/nestedBean/beans[last() + 1]");
-        }
-        catch (final Exception e) {
-            ex = true;
-        }
-        assertTrue(ex, "Exception thrown on invalid path for creation");
-
+                "/nestedBean/beans[last() + 1]"),
+            "Exception thrown on invalid path for creation");
     }
 
     @Test

diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/AbstractXMLModelTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/AbstractXMLModelTest.java
@@ -28,7 +28,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Abstract superclass for pure XPath 1.0.  Subclasses
@@ -340,25 +340,15 @@ public void testAxisChild() {
 
         assertXPathValue(context, "/vendor/contact[@name='jim']", "Jim");
 
-        boolean nsv = false;
-        try {
+        assertThrows(JXPathException.class, () -> {
             context.setLenient(false);
             context.getValue("/vendor/contact[@name='jane']");
-        }
-        catch (final JXPathException ex) {
-            nsv = true;
-        }
-        assertTrue(nsv, "No such value: /vendor/contact[@name='jim']");
+        }, "No such value: /vendor/contact[@name='jim']");
 
-        nsv = false;
-        try {
+        assertThrows(JXPathException.class, () -> {
             context.setLenient(false);
             context.getValue("/vendor/contact[@name='jane']/*");
-        }
-        catch (final JXPathException ex) {
-            nsv = true;
-        }
-        assertTrue(nsv, "No such value: /vendor/contact[@name='jane']/*");
+        }, "No such value: /vendor/contact[@name='jane']/*");
 
         // child:: with a wildcard
         assertXPathValue(

diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/MixedModelTest.java
@@ -36,7 +36,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Tests JXPath with mixed model: beans, maps, DOM etc.
@@ -453,14 +453,8 @@ public void testErrorProperty() {
             "e",
             new ExceptionPropertyTestBean());
 
-        boolean ex = false;
-        try {
-            assertXPathValue(context, "$e/errorString", null);
-        }
-        catch (final Throwable t) {
-            ex = true;
-        }
-        assertTrue(ex, "Legitimate exception accessing property");
+        assertThrows(Throwable.class, () -> assertXPathValue(context, "$e/errorString", null),
+            "Legitimate exception accessing property");
 
         assertXPathPointer(context, "$e/errorString", "$e/errorString");
 
@@ -510,23 +504,11 @@ public void testMatrix() {
             Integer.valueOf(2),
             "$wholebean/matrix[1]/.[1]");
 
-        boolean ex = false;
-        try {
-            context.setValue("$wholebean/matrix[1]/.[2]", "4");
-        }
-        catch (final Exception e) {
-            ex = true;
-        }
-        assertTrue(ex, "Exception setting value of non-existent element");
+        assertThrows(Exception.class, () -> context.setValue("$wholebean/matrix[1]/.[2]", "4"),
+            "Exception setting value of non-existent element");
 
-        ex = false;
-        try {
-            context.setValue("$wholebean/matrix[2]/.[1]", "4");
-        }
-        catch (final Exception e) {
-            ex = true;
-        }
-        assertTrue(ex, "Exception setting value of non-existent element");
+        assertThrows(Exception.class, () -> context.setValue("$wholebean/matrix[2]/.[1]", "4"),
+           "Exception setting value of non-existent element");
     }
 
     @Test

diff --git a/src/test/java/org/apache/commons/jxpath/ri/model/beans/BadlyImplementedFactoryTest.java b/src/test/java/org/apache/commons/jxpath/ri/model/beans/BadlyImplementedFactoryTest.java
@@ -29,7 +29,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Badly-implemented Factory test.  From JIRA JXPATH-68.
@@ -52,12 +52,9 @@ public boolean createObject(final JXPathContext context, final Pointer pointer,
 
     @Test
     public void testBadFactoryImplementation() {
-        try {
-            context.createPath("foo/bar");
-            fail("should fail with JXPathException caused by JXPathAbstractFactoryException");
-        } catch (final JXPathException e) {
-            assertInstanceOf(JXPathAbstractFactoryException.class, e.getCause());
-        }
+        JXPathException e = assertThrows(JXPathException.class, () -> context.createPath("foo/bar"),
+            "should fail with JXPathException caused by JXPathAbstractFactoryException");
+        assertInstanceOf(JXPathAbstractFactoryException.class, e.getCause());
     }
 
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -33,7 +33,7 @@ public void testInit() { @@
                 JXPathContextReferenceImpl.addNodePointerFactory(factory);
             } finally {
                 while (JXPathContextReferenceImpl.removeNodePointerFactory(factory)) {
+                    // NOP
                 }
             }
         }
@@ Expand Down @@