You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2020/10/16 16:25:07 UTC

[tomcat] branch 8.5.x updated: SpotBugs - fix various exception class related issues

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 461f8d7  SpotBugs - fix various exception class related issues
461f8d7 is described below

commit 461f8d787b15aac6cd717a5c15466117dc24a0a9
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Oct 16 17:06:16 2020 +0100

    SpotBugs - fix various exception class related issues
    
    I'd prefer not to use static imports at all but JUnit uses them
    extensively and avoiding them in these instances means unnecessarily
    verbose code.
---
 res/checkstyle/checkstyle.xml                              |  5 +++--
 test/javax/el/TestBeanELResolver.java                      |  7 +++++--
 .../apache/catalina/authenticator/TestBasicAuthParser.java | 14 +++-----------
 .../apache/catalina/core/TestSwallowAbortedUploads.java    | 11 +++++++----
 test/org/apache/catalina/realm/TestJNDIRealm.java          |  7 +++++--
 .../startup/TestHostConfigAutomaticDeployment.java         |  5 ++++-
 test/org/apache/catalina/startup/TestTomcat.java           |  7 +++++--
 .../apache/catalina/startup/TestWebappServiceLoader.java   |  7 +++++--
 test/org/apache/catalina/tribes/io/TestXByteBuffer.java    |  5 ++++-
 .../apache/jasper/compiler/TestELInterpreterFactory.java   | 14 ++++++++------
 10 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/res/checkstyle/checkstyle.xml b/res/checkstyle/checkstyle.xml
index 7257bd0..2eb1670 100644
--- a/res/checkstyle/checkstyle.xml
+++ b/res/checkstyle/checkstyle.xml
@@ -60,8 +60,9 @@
     <!-- Imports -->
     <module name="AvoidStarImport"/>
     <module name="AvoidStaticImport">
-        <property name="excludes"
-                  value="org.apache.catalina.startup.SimpleHttpClient.CRLF"/>
+        <property name="excludes" value="org.apache.catalina.startup.SimpleHttpClient.CRLF"/>
+        <property name="excludes" value="org.hamcrest.MatcherAssert.*"/>
+        <property name="excludes" value="org.hamcrest.core.IsInstanceOf.*"/>
     </module>
     <module name="IllegalImport">
         <property name="illegalPkgs" value="sun,junit.framework"/>
diff --git a/test/javax/el/TestBeanELResolver.java b/test/javax/el/TestBeanELResolver.java
index 7bd9bce..d1df8c6 100644
--- a/test/javax/el/TestBeanELResolver.java
+++ b/test/javax/el/TestBeanELResolver.java
@@ -21,6 +21,9 @@ import java.beans.PropertyDescriptor;
 import java.util.ArrayList;
 import java.util.Iterator;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -57,9 +60,9 @@ public class TestBeanELResolver {
         } catch (PropertyNotFoundException pnfe) {
             e = pnfe;
         }
-        Assert.assertTrue("Wrong exception type",
-                e instanceof PropertyNotFoundException);
+        assertThat("Wrong exception type", e, instanceOf(PropertyNotFoundException.class));
         String type = Bean.class.getName();
+        @SuppressWarnings("null") // Not possible due to test above
         String msg = e.getMessage();
         Assert.assertTrue("No reference to type [" + type +
                 "] where property cannot be found in [" + msg + "]",
diff --git a/test/org/apache/catalina/authenticator/TestBasicAuthParser.java b/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
index 9f918d2..b5cfbed 100644
--- a/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
+++ b/test/org/apache/catalina/authenticator/TestBasicAuthParser.java
@@ -165,22 +165,14 @@ public class TestBasicAuthParser {
     /*
      * Confirm the Basic parser rejects an invalid authentication method.
      */
-    @Test
+    @Test(expected = IllegalArgumentException.class)
     public void testAuthMethodBadMethod() throws Exception {
         final String METHOD = "BadMethod";
         final BasicAuthHeader AUTH_HEADER =
                 new BasicAuthHeader(METHOD, USER_NAME, PASSWORD);
         @SuppressWarnings("unused")
-        BasicAuthenticator.BasicCredentials credentials = null;
-        try {
-            credentials = new BasicAuthenticator.BasicCredentials(
-                AUTH_HEADER.getHeader(), StandardCharsets.UTF_8, true);
-            Assert.fail("IllegalArgumentException expected");
-        }
-        catch (Exception e) {
-            Assert.assertTrue(e instanceof IllegalArgumentException);
-            Assert.assertTrue(e.getMessage().contains("header method"));
-        }
+        BasicAuthenticator.BasicCredentials credentials =
+                new BasicAuthenticator.BasicCredentials(AUTH_HEADER.getHeader(), StandardCharsets.UTF_8, true);
     }
 
     /*
diff --git a/test/org/apache/catalina/core/TestSwallowAbortedUploads.java b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
index 83b1634..7061ade 100644
--- a/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
+++ b/test/org/apache/catalina/core/TestSwallowAbortedUploads.java
@@ -35,6 +35,9 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.Part;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -128,8 +131,8 @@ public class TestSwallowAbortedUploads extends TomcatBaseTest {
         log.info("Limited, swallow disabled");
         AbortedUploadClient client = new AbortedUploadClient();
         Exception ex = doAbortedUploadTest(client, true, false);
-        Assert.assertTrue("Limited upload with swallow disabled does not generate client exception",
-                   ex instanceof java.net.SocketException);
+        assertThat("Limited upload with swallow disabled does not generate client exception",
+                   ex, instanceOf(java.net.SocketException.class));
         client.reset();
     }
 
@@ -174,8 +177,8 @@ public class TestSwallowAbortedUploads extends TomcatBaseTest {
         log.info("Aborted (413), swallow disabled");
         AbortedPOSTClient client = new AbortedPOSTClient();
         Exception ex = doAbortedPOSTTest(client, HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, false);
-        Assert.assertTrue("Limited upload with swallow disabled does not generate client exception",
-                   ex instanceof java.net.SocketException);
+        assertThat("Limited upload with swallow disabled does not generate client exception",
+                ex, instanceOf(java.net.SocketException.class));
         client.reset();
     }
 
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java b/test/org/apache/catalina/realm/TestJNDIRealm.java
index 203e794..5ffac18 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -29,6 +29,9 @@ import javax.naming.directory.InitialDirContext;
 import javax.naming.directory.SearchControls;
 import javax.naming.directory.SearchResult;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -87,7 +90,7 @@ public class TestJNDIRealm {
                 realm.authenticate(USER, expectedResponse, NONCE, null, null, null, REALM, HA2);
 
         // THEN
-        Assert.assertTrue(principal instanceof GenericPrincipal);
+        assertThat(principal, instanceOf(GenericPrincipal.class));
         Assert.assertEquals(PASSWORD, ((GenericPrincipal)principal).getPassword());
     }
 
@@ -105,7 +108,7 @@ public class TestJNDIRealm {
                 realm.authenticate(USER, expectedResponse, NONCE, null, null, null, REALM, HA2);
 
         // THEN
-        Assert.assertTrue(principal instanceof GenericPrincipal);
+        assertThat(principal, instanceOf(GenericPrincipal.class));
         Assert.assertEquals(ha1(), ((GenericPrincipal)principal).getPassword());
     }
 
diff --git a/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java b/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
index 9769f77..cfe5c46 100644
--- a/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
+++ b/test/org/apache/catalina/startup/TestHostConfigAutomaticDeployment.java
@@ -22,6 +22,9 @@ import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -1884,7 +1887,7 @@ public class TestHostConfigAutomaticDeployment extends TomcatBaseTest {
         // Check the Context class
         Context ctxt = (Context) host.findChild(APP_NAME.getName());
 
-        Assert.assertTrue(ctxt instanceof TesterContext);
+        assertThat(ctxt, instanceOf(TesterContext.class));
     }
 
 
diff --git a/test/org/apache/catalina/startup/TestTomcat.java b/test/org/apache/catalina/startup/TestTomcat.java
index f4d89b1..1ed56c1 100644
--- a/test/org/apache/catalina/startup/TestTomcat.java
+++ b/test/org/apache/catalina/startup/TestTomcat.java
@@ -32,6 +32,9 @@ import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -571,7 +574,7 @@ public class TestTomcat extends TomcatBaseTest {
             tomcat.start();
             Assert.fail();
         } catch (Throwable t) {
-            Assert.assertTrue(getRootCause(t) instanceof LifecycleException);
+            assertThat(getRootCause(t), instanceOf(LifecycleException.class));
         }
     }
 
@@ -589,7 +592,7 @@ public class TestTomcat extends TomcatBaseTest {
             tomcat.start();
             Assert.fail();
         } catch (Throwable t) {
-            Assert.assertTrue(getRootCause(t) instanceof MultiThrowable);
+            assertThat(getRootCause(t), instanceOf(MultiThrowable.class));
         }
     }
 
diff --git a/test/org/apache/catalina/startup/TestWebappServiceLoader.java b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
index 9a5d788..358ce60 100644
--- a/test/org/apache/catalina/startup/TestWebappServiceLoader.java
+++ b/test/org/apache/catalina/startup/TestWebappServiceLoader.java
@@ -27,6 +27,9 @@ import java.util.List;
 import javax.servlet.ServletContainerInitializer;
 import javax.servlet.ServletContext;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -162,7 +165,7 @@ public class TestWebappServiceLoader {
         try {
             loader.loadServices(ServletContainerInitializer.class, names);
         } catch (IOException e) {
-            Assert.assertTrue(e.getCause() instanceof ClassCastException);
+            assertThat(e.getCause(), instanceOf(ClassCastException.class));
         } finally {
             control.verify();
         }
@@ -181,7 +184,7 @@ public class TestWebappServiceLoader {
         try {
             loader.loadServices(ServletContainerInitializer.class, names);
         } catch (IOException e) {
-            Assert.assertTrue(e.getCause() instanceof ReflectiveOperationException);
+            assertThat(e.getCause(), instanceOf(ReflectiveOperationException.class));
         } finally {
             control.verify();
         }
diff --git a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
index 5bbb7c2..cf7ff86 100644
--- a/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
+++ b/test/org/apache/catalina/tribes/io/TestXByteBuffer.java
@@ -16,6 +16,9 @@
  */
 package org.apache.catalina.tribes.io;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -32,7 +35,7 @@ public class TestXByteBuffer {
         String test = "This is as test.";
         byte[] msg = XByteBuffer.serialize(test);
         Object obj = XByteBuffer.deserialize(msg);
-        Assert.assertTrue(obj instanceof String);
+        assertThat(obj, instanceOf(String.class));
         Assert.assertEquals(test, obj);
     }
 }
diff --git a/test/org/apache/jasper/compiler/TestELInterpreterFactory.java b/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
index b22f09d..da8ee5e 100644
--- a/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
+++ b/test/org/apache/jasper/compiler/TestELInterpreterFactory.java
@@ -22,6 +22,9 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -53,10 +56,9 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
 
         ServletContext context = ctx.getServletContext();
 
-        ELInterpreter interpreter =
-                ELInterpreterFactory.getELInterpreter(context);
+        ELInterpreter interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof DefaultELInterpreter);
+        assertThat(interpreter, instanceOf(DefaultELInterpreter.class));
 
         context.removeAttribute(ELInterpreter.class.getName());
 
@@ -64,7 +66,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
                 SimpleELInterpreter.class.getName());
         interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
 
         context.removeAttribute(ELInterpreter.class.getName());
 
@@ -72,7 +74,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
         context.setAttribute(ELInterpreter.class.getName(), simpleInterpreter);
         interpreter = ELInterpreterFactory.getELInterpreter(context);
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
         Assert.assertTrue(interpreter == simpleInterpreter);
 
         context.removeAttribute(ELInterpreter.class.getName());
@@ -83,7 +85,7 @@ public class TestELInterpreterFactory extends TomcatBaseTest {
 
         interpreter = ELInterpreterFactory.getELInterpreter(ctx.getServletContext());
         Assert.assertNotNull(interpreter);
-        Assert.assertTrue(interpreter instanceof SimpleELInterpreter);
+        assertThat(interpreter, instanceOf(SimpleELInterpreter.class));
     }
 
     public static class Bug54239Listener implements ServletContextListener {


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org