Upgrade to JUnit 5

.github/workflows/ci-build.yml

@@ -13,9 +13,7 @@ jobs:
       matrix:
         java_version:
           - 11
-          # Errors with `java.lang.IllegalArgumentException: ArquillianServletRunnerEE9 not found. Could not determine ContextRoot from ProtocolMetadata, please contact DeployableContainer developer.`
-          # Bumping various Arquillian dependencies does not fix, leave as just Java 11
-          # - 17  
+          - 21
 
     steps:
       - name: Setup Java

pom.xml

@@ -126,7 +126,7 @@
 			<dependency>
 				<groupId>org.jboss.arquillian</groupId>
 				<artifactId>arquillian-bom</artifactId>
-				<version>1.7.0.Final</version>
+				<version>1.9.5.Final</version>
 				<scope>import</scope>
 				<type>pom</type>
 			</dependency>
@@ -153,14 +153,14 @@
 			<version>1.2.13</version>
 		</dependency>
 		<dependency>
-			<groupId>junit</groupId>
-			<artifactId>junit</artifactId>
-			<version>4.13.2</version>
+			<groupId>org.junit.jupiter</groupId>
+			<artifactId>junit-jupiter</artifactId>
+			<version>5.13.1</version>
 			<scope>test</scope>
 		</dependency>
 		<dependency>
-			<groupId>org.jboss.arquillian.junit</groupId>
-			<artifactId>arquillian-junit-container</artifactId>
+			<groupId>org.jboss.arquillian.junit5</groupId>
+			<artifactId>arquillian-junit5-container</artifactId>
 			<scope>test</scope>
 		</dependency>
 		<!-- required for @Transactional annotation -->
@@ -208,13 +208,13 @@
 				<dependency>
 					<groupId>org.omnifaces.arquillian</groupId>
 					<artifactId>arquillian-glassfish-server-embedded</artifactId>
-					<version>1.4</version>
+					<version>1.8</version>
 					<scope>test</scope>
 				</dependency>
 				<dependency>
 					<groupId>org.glassfish.main.extras</groupId>
 					<artifactId>glassfish-embedded-all</artifactId>
-					<version>7.0.5</version>
+					<version>7.0.25</version>
 					<scope>test</scope>
 				</dependency>
 				<dependency>
@@ -242,9 +242,11 @@
 						GlassFish to retarget the derby log file -->
 					<plugin>
 						<artifactId>maven-surefire-plugin</artifactId>
-						<version>2.19.1</version>
+						<version>3.5.3</version>
 						<configuration>
-							<argLine>-Xmx768m -XX:MaxMetaspaceSize=256m</argLine>
+							<!-- Required for jdk 17+ -->
+							<argLine>--add-opens java.base/java.lang=ALL-UNNAMED</argLine>
+
 							<systemPropertyVariables>
 								<java.util.logging.config.file>
 									${project.build.testOutputDirectory}/logging.properties

src/test/java/org/icatproject/topcat/AdminResourceTest.java

@@ -1,41 +1,46 @@
 package org.icatproject.topcat;
 
-import java.util.*;
+import java.net.URLEncoder;
+import java.util.ArrayList;
 import java.util.Date;
-import java.io.File;
-import java.lang.reflect.*;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import jakarta.inject.Inject;
+import jakarta.json.JsonObject;
+import jakarta.ejb.EJB;
+import jakarta.ws.rs.core.Response;
 
 import org.jboss.arquillian.container.test.api.Deployment;
-import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.junit5.container.annotation.ArquillianTest;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.asset.EmptyAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 
-import static org.junit.Assert.*;
-import org.junit.*;
-import org.junit.runner.RunWith;
-import jakarta.inject.Inject;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+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 jakarta.json.*;
-import jakarta.ws.rs.core.Response;
-import jakarta.ejb.EJB;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import org.icatproject.topcat.httpclient.HttpClient;
-import org.icatproject.topcat.domain.*;
-import org.icatproject.topcat.exceptions.BadRequestException;
+import org.icatproject.topcat.domain.Download;
+import org.icatproject.topcat.domain.DownloadStatus;
+import org.icatproject.topcat.domain.DownloadType;
 import org.icatproject.topcat.exceptions.ForbiddenException;
-
-import java.net.URLEncoder;
-
 import org.icatproject.topcat.repository.CacheRepository;
 import org.icatproject.topcat.repository.ConfVarRepository;
 import org.icatproject.topcat.repository.DownloadRepository;
 import org.icatproject.topcat.repository.DownloadTypeRepository;
 import org.icatproject.topcat.web.rest.AdminResource;
 
-import java.sql.*;
-
-@RunWith(Arquillian.class)
+@ArquillianTest
 public class AdminResourceTest {
 
 	/*
@@ -71,12 +76,12 @@ public static JavaArchive createDeployment() {
 	private static String adminSessionId;
 	private static String nonAdminSessionId;
 
-	@BeforeClass
+	@BeforeAll
 	public static void beforeAll() {
 		TestHelpers.installTrustManager();
 	}
 
-	@Before
+	@BeforeEach
 	public void setup() throws Exception {
 		HttpClient httpClient = new HttpClient("https://localhost:8181/icat");
 
@@ -194,7 +199,7 @@ public void testDownloadAPI() throws Exception {
 			downloads = (List<Download>) response.getEntity();
 
 			testDownload = findDownload(downloads, testDownload.getId());
-			assertTrue(testDownload.getIsDeleted() != currentDeleted);
+			assertNotEquals(testDownload.getIsDeleted(), currentDeleted);
 
 			// Test that getDownloadStatus() etc. produce an error response for a non-admin
 			// user
@@ -206,7 +211,6 @@ public void testDownloadAPI() throws Exception {
 						+ (String) response.getEntity());
 				fail("AdminResource.getDownloads did not raise exception for non-admin user");
 			} catch (ForbiddenException fe) {
-				assertTrue(true);
 			}
 
 			try {
@@ -217,7 +221,6 @@ public void testDownloadAPI() throws Exception {
 						+ (String) response.getEntity());
 				fail("AdminResource.setDownloadStatus did not raise exception for non-admin user");
 			} catch (ForbiddenException fe) {
-				assertTrue(true);
 			}
 
 			try {
@@ -228,7 +231,6 @@ public void testDownloadAPI() throws Exception {
 						+ (String) response.getEntity());
 				fail("AdminResource.deleteDownload did not raise exception for non-admin user");
 			} catch (ForbiddenException fe) {
-				assertTrue(true);
 			}
 		} finally {
 			// Remove the test download from the repository
@@ -308,7 +310,7 @@ public void testSetDownloadTypeStatus() throws Exception {
 		if (dt != null) {
 			System.out.println(
 					"DEBUG: AdminRT final download type status is {" + dt.getDisabled() + "," + dt.getMessage() + "}");
-			assertTrue(disabled != dt.getDisabled());
+			assertNotEquals(disabled, dt.getDisabled());
 			assertEquals(message, dt.getMessage());
 		}
 
@@ -322,7 +324,6 @@ public void testSetDownloadTypeStatus() throws Exception {
 					+ (String) response.getEntity());
 			fail("AdminResource.setDownloadTypeStatus did not raise exception for non-admin user");
 		} catch (ForbiddenException fe) {
-			assertTrue(true);
 		}
 
 		// Finally, ought to reset the disabled status to the original value!
@@ -406,7 +407,6 @@ public void testClearCachedSize() throws Exception {
 					+ (String) response.getEntity());
 			fail("AdminResource.clearCachedSize did not raise exception for non-admin user");
 		} catch (ForbiddenException fe) {
-			assertTrue(true);
 		}
 	}
 
@@ -437,7 +437,6 @@ public void testSetConfVar() throws Exception {
 					+ (String) response.getEntity());
 			fail("AdminResource.setConfVar did not raise exception for non-admin user");
 		} catch (ForbiddenException fe) {
-			assertTrue(true);
 		}
 	}
 

src/test/java/org/icatproject/topcat/CacheRepositoryTest.java

@@ -1,25 +1,22 @@
 package org.icatproject.topcat;
 
-import java.util.*;
-import java.lang.reflect.*;
+import jakarta.inject.Inject;
 
 import org.jboss.arquillian.container.test.api.Deployment;
-import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.arquillian.junit5.container.annotation.ArquillianTest;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.asset.EmptyAsset;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
-import static org.junit.Assert.*;
-import org.junit.*;
-import org.junit.runner.RunWith;
-import jakarta.inject.Inject;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
-import jakarta.ejb.EJB;
+import org.junit.jupiter.api.Test;
 
 import org.icatproject.topcat.domain.Cache;
 import org.icatproject.topcat.repository.CacheRepository;
 
-@RunWith(Arquillian.class)
+@ArquillianTest
 public class CacheRepositoryTest {
 
     @Deployment
@@ -44,6 +41,6 @@ public void testRemove() {
 		String key = "test:remove";
 		cacheRepository.put(key, "Hello World");
 		cacheRepository.remove(key);
-		assertEquals(null,cacheRepository.get(key));
+		assertNull(cacheRepository.get(key));
 	}
-}
+}

src/test/java/org/icatproject/topcat/FacilityMapTest.java

@@ -1,13 +1,14 @@
 package org.icatproject.topcat;
 
-import static org.junit.Assert.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.HashMap;
 import java.util.Map;
 
 import org.icatproject.topcat.exceptions.InternalException;
-import org.junit.*;
-import org.junit.function.ThrowingRunnable;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
 
 public class FacilityMapTest {
 
@@ -153,7 +154,7 @@ public void testGetIcatUrlFailure() throws InternalException{
 
 		FacilityMap facilityMap = new FacilityMap(props);
 
-		ThrowingRunnable runnable = () -> {facilityMap.getIcatUrl(null);};
+		Executable runnable = () -> {facilityMap.getIcatUrl(null);};
 		assertThrows(InternalException.class, runnable);
 	}
 }

src/test/java/org/icatproject/topcat/IcatClientTest.java

@@ -1,25 +1,38 @@
 package org.icatproject.topcat;
 
-import java.util.*;
-import java.lang.reflect.*;
-
-import static org.junit.Assert.*;
-import org.junit.*;
+import java.net.URLEncoder;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
-import jakarta.json.*;
-import jakarta.json.JsonValue.ValueType;
 import jakarta.ejb.EJB;
+import jakarta.json.Json;
+import jakarta.json.JsonArray;
+import jakarta.json.JsonArrayBuilder;
+import jakarta.json.JsonObject;
+import jakarta.json.JsonObjectBuilder;
+import jakarta.json.JsonValue;
+import jakarta.json.JsonValue.ValueType;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 import org.icatproject.topcat.httpclient.HttpClient;
 import org.icatproject.topcat.httpclient.Response;
-import org.icatproject.topcat.domain.*;
 import org.icatproject.topcat.exceptions.TopcatException;
-
-import java.net.URLEncoder;
-
 import org.icatproject.topcat.repository.CacheRepository;
 
-import java.sql.*;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
 
 public class IcatClientTest {
 
@@ -30,12 +43,12 @@ public class IcatClientTest {
 
 	private Connection connection;
 
-	@BeforeClass
+	@BeforeAll
 	public static void beforeAll() {
 		TestHelpers.installTrustManager();
 	}
 
-	@Before
+	@BeforeEach
 	public void setup() throws Exception {
 		HttpClient httpClient = new HttpClient("https://localhost:8181/icat");
 		String data = "json=" + URLEncoder.encode(

src/test/java/org/icatproject/topcat/IdsClientTest.java

@@ -1,10 +1,14 @@
 package org.icatproject.topcat;
 
-import java.util.*;
-import java.lang.reflect.*;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
 
-import static org.junit.Assert.*;
-import org.junit.*;
+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 org.junit.jupiter.api.Test;
 
 public class IdsClientTest {
 
@@ -54,16 +58,15 @@ public void testChunkOffsets() throws Exception {
 				}
 				allFound = allFound && foundInvestigation && foundDataset && foundDatafile;
 			}
-			assertTrue("Not all IDs found in chunks", allFound);
-			assertTrue("At least one ID was repeated", ! foundRepeats);
+			assertTrue(allFound, "Not all IDs found in chunks");
+			assertFalse(foundRepeats, "At least one ID was repeated");
 		}
 
 		String expected = "test?investigationIds=1,2,3&datasetIds=4,5,6&datafileIds=7,8,9";
 		List<String> offsets = (List<String>) chunkOffsets.invoke(idsClient, "test?", generateIds(1, 3), generateIds(4, 3), generateIds(7, 3));
 		String actual = offsets.get(0);
 
-		assertTrue("expected: " + expected + " actual: " + actual, expected.equals(actual));
-
+		assertEquals(expected, actual);
 	}
 
 	@Test
@@ -94,4 +97,4 @@ private boolean chunkContains(String chunk, String entityType, Long id) {
 		return chunk.matches(".*" + entityType + "Ids=[\\d+,]*" + id.toString() + "[,\\d+]*.*");
 	}
 
-}
+}