Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace try-catch constructs in tests with assertThrows #215

Merged
merged 3 commits into from
Jan 11, 2025
Merged

Replace try-catch constructs in tests with assertThrows #215

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24bf0e9
Replace try-catch constructs in tests with assertThrows and assertDoe…
strangelookingnerd Jan 8, 2025
423ca0e
Add meaningful assertions
strangelookingnerd Jan 10, 2025
1c92047
Remove usage of assertDoesNotThrow()
strangelookingnerd Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions src/test/java/org/apache/commons/jxpath/issues/JXPath113Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{

Expand All @@ -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
Expand All @@ -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 {
strangelookingnerd marked this conversation as resolved.
Show resolved Hide resolved
final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setValidating(false);
factory.setNamespaceAware(true);
factory.setExpandEntityReferences(false);
return factory.newDocumentBuilder();
}
}

Expand Down
34 changes: 4 additions & 30 deletions src/test/java/org/apache/commons/jxpath/issues/JXPath172Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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"));
garydgregory marked this conversation as resolved.
Show resolved Hide resolved
assertThrows(JXPathNotFoundException.class, () -> context.getPointer("unexisting"));
assertThrows(JXPathNotFoundException.class, () -> context.getPointer("value.unexisting"));
}

/**
Expand Down
37 changes: 17 additions & 20 deletions src/test/java/org/apache/commons/jxpath/ri/ExceptionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImplTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;

public class JXPathContextReferenceImplTestCase {

/**
Expand All @@ -29,12 +31,10 @@ public class JXPathContextReferenceImplTestCase {
@Test
public void testInit() {
final ContainerPointerFactory factory = new ContainerPointerFactory();
try {
JXPathContextReferenceImpl.addNodePointerFactory(factory);
} finally {
while (JXPathContextReferenceImpl.removeNodePointerFactory(factory)) {
assertDoesNotThrow(() -> JXPathContextReferenceImpl.addNodePointerFactory(factory));
strangelookingnerd marked this conversation as resolved.
Show resolved Hide resolved

}
while (JXPathContextReferenceImpl.removeNodePointerFactory(factory)) {
// NOP
}
}
}
23 changes: 5 additions & 18 deletions src/test/java/org/apache/commons/jxpath/ri/StressTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;

/**
* Test thread safety.
Expand All @@ -32,7 +32,6 @@ public class StressTest {
private static final int THREAD_DURATION = 1000;
private static JXPathContext context;
private static int count;
private static Throwable exception;

@Test
public void testThreads() throws Throwable {
Expand All @@ -47,25 +46,16 @@ public void testThreads() throws Throwable {
}

for (final Thread element : threadArray) {
try {
element.join();
}
catch (final InterruptedException e) {
fail("Interrupted");
}
}

if (exception != null) {
throw exception;
assertDoesNotThrow(() -> element.join(), "Interrupted");
strangelookingnerd marked this conversation as resolved.
Show resolved Hide resolved
}
assertEquals(THREAD_COUNT * THREAD_DURATION, count, "Test count");
}

private static final class StressRunnable implements Runnable {
@Override
public void run() {
for (int j = 0; j < THREAD_DURATION && exception == null; j++) {
try {
for (int j = 0; j < THREAD_DURATION; j++) {
assertDoesNotThrow(() -> {
strangelookingnerd marked this conversation as resolved.
Show resolved Hide resolved
final double random = 1 + Math.random();
final double sum =
((Double) context.getValue("/ + " + random))
Expand All @@ -74,10 +64,7 @@ public void run() {
synchronized (context) {
count++;
}
}
catch (final Throwable t) {
exception = t;
}
});
}
}
}
Expand Down
10 changes: 2 additions & 8 deletions src/test/java/org/apache/commons/jxpath/ri/compiler/CoreFunctionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
22 changes: 3 additions & 19 deletions src/test/java/org/apache/commons/jxpath/ri/compiler/VariableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
strangelookingnerd marked this conversation as resolved.
Show resolved Hide resolved
"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");
}

Expand Down
14 changes: 4 additions & 10 deletions src/test/java/org/apache/commons/jxpath/ri/model/AbstractBeanModelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
20 changes: 5 additions & 15 deletions src/test/java/org/apache/commons/jxpath/ri/model/AbstractXMLModelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
Loading
Loading