diff --git a/pom.xml b/pom.xml index b5b93b1d9d..f9042c6c15 100644 --- a/pom.xml +++ b/pom.xml @@ -22,8 +22,7 @@ 2.20 - 2.0.9 - 4.13.2 + 5.11.0 5.14.1 4.13.2 0.16.0 @@ -202,12 +201,6 @@ swagger-annotations-jakarta ${swagger-core.version} - - junit - junit - ${junit-4.version} - test - org.junit.jupiter junit-jupiter-engine @@ -255,23 +248,22 @@ ${testcontainers.version} test - - org.powermock - powermock-module-junit4 - ${powermock.version} + org.easymock + easymock + 5.6.0 test - org.powermock - powermock-api-easymock - ${powermock.version} + org.mockito + mockito-core + ${mockito.version} test - org.easymock - easymock - 4.3 + org.mockito + mockito-junit-jupiter + ${mockito.version} test @@ -568,16 +560,6 @@ org.apache.maven.plugins maven-surefire-plugin 3.2.5 - - - - org.apache.maven.surefire - surefire-junit47 - 3.5.4 - - 3 true diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java index 282696c113..d6e10f3c5e 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java @@ -46,11 +46,11 @@ import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.users.IUserGroupPersistenceManager; +import uk.ac.cam.cl.dtg.util.NameOrderer; import uk.ac.cam.cl.dtg.util.mappers.UserMapper; import java.util.ArrayList; import java.util.Collection; -import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.LinkedList; @@ -181,7 +181,7 @@ public List getUsersInGroup(final UserGroupDTO group) throws List users = userManager.findUsers(groupMemberIds); // Sort the users by name - this.orderUsersByName(users); + NameOrderer.orderUsersByName(users); return users; } @@ -210,24 +210,6 @@ public GroupMembershipStatus getGroupMembershipStatus(Long userId, Long groupId) return this.getUserMembershipMapForGroup(groupId).get(userId).getStatus(); } - /** - * Helper method to consistently sort users by given name then family name in a case-insensitive order. - * @param users - * - list of users. - */ - private void orderUsersByName(final List users) { - // Remove apostrophes so that string containing them are ordered in the same way as in Excel. - // I.e. we want that "O'Aaa" < "Obbb" < "O'Ccc" - Comparator excelStringOrder = Comparator.nullsLast((String a, String b) -> - String.CASE_INSENSITIVE_ORDER.compare(a.replaceAll("'", ""), b.replaceAll("'", ""))); - - // If names differ only by an apostrophe (i.e. "O'A" and "Oa"), break ties using name including any apostrophes: - users.sort(Comparator - .comparing(RegisteredUserDTO::getFamilyName, excelStringOrder) - .thenComparing(RegisteredUserDTO::getGivenName, excelStringOrder) - .thenComparing(RegisteredUserDTO::getFamilyName)); - } - /** * getAllGroupsOwnedAndManagedByUser. * diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAccountManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAccountManager.java index d34729803c..e4f0b75980 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAccountManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAccountManager.java @@ -780,7 +780,7 @@ public final boolean isRegisteredUserLoggedIn(final HttpServletRequest request) * @return Returns the current UserDTO if we can get it or null if user is not currently logged in * @throws NoUserLoggedInException - When the session has expired or there is no user currently logged in. */ - public final RegisteredUserDTO getCurrentRegisteredUser(final HttpServletRequest request) + public RegisteredUserDTO getCurrentRegisteredUser(final HttpServletRequest request) throws NoUserLoggedInException { Objects.requireNonNull(request); @@ -875,7 +875,7 @@ public List findUsers(final Collection userIds) throws * @throws SegueDatabaseException - If there is another database error */ @Override - public final RegisteredUserDTO getUserDTOById(final Long id) throws NoUserException, SegueDatabaseException { + public RegisteredUserDTO getUserDTOById(final Long id) throws NoUserException, SegueDatabaseException { return this.getUserDTOById(id, false); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAuthenticationManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAuthenticationManager.java index 89e3ed3f5d..e032b79cee 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAuthenticationManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAuthenticationManager.java @@ -1058,7 +1058,7 @@ private void createSession(final HttpServletRequest request, final HttpServletRe * - the real user we are to validate this cookie against. * @return true if it is still valid, false if not. */ - private boolean isValidUsersSession(final Map sessionInformation, final RegisteredUser userFromDatabase) { + public boolean isValidUsersSession(final Map sessionInformation, final RegisteredUser userFromDatabase) { Objects.requireNonNull(sessionInformation); Objects.requireNonNull(userFromDatabase); @@ -1120,7 +1120,7 @@ private boolean isValidUsersSession(final Map sessionInformation * - Boolean data to encode in the cookie - true if a partial login * @return HMAC signature. */ - private String calculateSessionHMAC(final String key, final String userId, final String currentDate, final String sessionToken, + public static String calculateSessionHMAC(final String key, final String userId, final String currentDate, final String sessionToken, @Nullable final Set caveatFlags) { StringBuilder sb = new StringBuilder(); sb.append(userId); diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java index d3b7799ffd..dad9647a34 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/dao/content/GitContentManager.java @@ -197,7 +197,7 @@ public final ContentDTO getContentById(final String id) throws ContentManagerExc * @return the content DTO object. * @throws ContentManagerException on failure to return the object or null. */ - public final ContentDTO getContentById(final String id, final boolean failQuietly) throws ContentManagerException { + public ContentDTO getContentById(final String id, final boolean failQuietly) throws ContentManagerException { return this.contentSubclassMapper.getDTOByDO(this.getContentDOById(id, failQuietly)); } @@ -242,7 +242,7 @@ public final Content getContentDOById(final String id) throws ContentManagerExce * @return the content DTO object. * @throws ContentManagerException on failure to return the object or null. */ - public final Content getContentDOById(final String id, final boolean failQuietly) throws ContentManagerException { + public Content getContentDOById(final String id, final boolean failQuietly) throws ContentManagerException { if (null == id || id.isEmpty()) { return null; } @@ -596,7 +596,7 @@ public final ResultsWrapper findByFieldNamesRandomOrder( } @Deprecated - public final ResultsWrapper findByFieldNamesRandomOrder( + public ResultsWrapper findByFieldNamesRandomOrder( final List fieldsToMatch, final Integer startIndex, final Integer limit, @Nullable final Long randomSeed ) throws ContentManagerException { diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java index 2ab80eecd3..5b115f5a08 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java @@ -562,7 +562,7 @@ private String fixMediaSrc(final String canonicalSourceFile, final String origin * content object to flatten * @return Set of content objects comprised of all children and the parent. */ - private Set flattenContentObjects(final Content content) { + public static Set flattenContentObjects(final Content content) { Set setOfContentObjects = new HashSet<>(); if (!content.getChildren().isEmpty()) { @@ -682,7 +682,7 @@ private synchronized void registerUnits(final IsaacNumericQuestion q, Map gitCache, final Set tagsList, final Map allUnits, @@ -931,7 +931,7 @@ private String collateExpandableChildren(Content content) { * @param sha version to validate integrity of. * @param content a single item of content */ - private void recordContentTypeSpecificError(final String sha, final Content content, final Map> indexProblemCache) { + public void recordContentTypeSpecificError(final String sha, final Content content, final Map> indexProblemCache) { // ensure content does not have children and a value if (content.getValue() != null && !content.getChildren().isEmpty()) { String id = content.getId(); diff --git a/src/main/java/uk/ac/cam/cl/dtg/util/NameOrderer.java b/src/main/java/uk/ac/cam/cl/dtg/util/NameOrderer.java new file mode 100644 index 0000000000..1e40a1e930 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/util/NameOrderer.java @@ -0,0 +1,36 @@ +package uk.ac.cam.cl.dtg.util; + +import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; + +import java.util.Comparator; +import java.util.List; + +/** + * A utility class to order names lexicographically. + */ +public class NameOrderer { + + /** + * It does not make sense to create one of these! + */ + private NameOrderer() { + } + + /** + * Helper method to consistently sort users by given name then family name in a case-insensitive order. + * @param users + * - list of users. + */ + public static void orderUsersByName(final List users) { + // Remove apostrophes so that string containing them are ordered in the same way as in Excel. + // I.e. we want that "O'Aaa" < "Obbb" < "O'Ccc" + Comparator excelStringOrder = Comparator.nullsLast((String a, String b) -> + String.CASE_INSENSITIVE_ORDER.compare(a.replaceAll("'", ""), b.replaceAll("'", ""))); + + // If names differ only by an apostrophe (i.e. "O'A" and "Oa"), break ties using name including any apostrophes: + users.sort(Comparator + .comparing(RegisteredUserDTO::getFamilyName, excelStringOrder) + .thenComparing(RegisteredUserDTO::getGivenName, excelStringOrder) + .thenComparing(RegisteredUserDTO::getFamilyName)); + } +} diff --git a/src/main/java/uk/ac/cam/cl/dtg/util/YamlLoader.java b/src/main/java/uk/ac/cam/cl/dtg/util/YamlLoader.java index 274d6c3ec8..6a7eb10083 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/util/YamlLoader.java +++ b/src/main/java/uk/ac/cam/cl/dtg/util/YamlLoader.java @@ -66,9 +66,23 @@ public Set getKeys() { return this.loadedConfig.keySet(); } + /** + * Factory method that can be overridden for testing + */ + protected Yaml createYaml() { + return new Yaml(); + } + + /** + * Factory method that can be overridden for testing + */ + protected FileInputStream createFileInputStream(String path) throws IOException { + return new FileInputStream(path); + } + @Override protected synchronized void loadConfig() throws IOException { - Yaml yaml = new Yaml(); + Yaml yaml = createYaml(); String[] configPaths = this.configPath.split(","); @@ -77,7 +91,7 @@ protected synchronized void loadConfig() throws IOException { // check to see if this a resource or a file somewhere else if (getClass().getClassLoader().getResourceAsStream(path) == null) { - try (FileInputStream ioStream = new FileInputStream(path)) { + try (FileInputStream ioStream = createFileInputStream(path)) { // then we have to look further afield subConfig = yaml.load(ioStream); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/IsaacTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/IsaacTest.java index 517bd5cab5..234bd2be13 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/IsaacTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/IsaacTest.java @@ -17,7 +17,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import org.junit.Before; +import org.junit.jupiter.api.BeforeEach; import uk.ac.cam.cl.dtg.isaac.api.Constants; import uk.ac.cam.cl.dtg.isaac.api.managers.GameManager; import uk.ac.cam.cl.dtg.isaac.api.managers.QuizManager; @@ -56,13 +56,13 @@ import static org.easymock.EasyMock.anyLong; import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.getCurrentArguments; import static org.easymock.EasyMock.partialMockBuilder; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replay; -import static org.powermock.api.easymock.PowerMock.reset; -import static org.powermock.api.easymock.PowerMock.verify; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.reset; +import static org.easymock.EasyMock.verify; public class IsaacTest { protected static Date somePastDate = new Date(System.currentTimeMillis() - 7*24*60*60*1000); @@ -130,7 +130,7 @@ public class IsaacTest { protected Map defaultsMap = new HashMap<>(); - @Before + @BeforeEach public final void initializeIsaacTest() throws SegueDatabaseException, ContentManagerException { initializeIsaacObjects(); initializeMocks(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractFacadeTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractFacadeTest.java index f6d00ed4b4..75831f998a 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractFacadeTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractFacadeTest.java @@ -17,11 +17,7 @@ import com.google.api.client.util.Maps; import com.google.common.base.Joiner; -import org.junit.Before; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.BeforeEach; import uk.ac.cam.cl.dtg.isaac.IsaacTest; import uk.ac.cam.cl.dtg.isaac.dto.SegueErrorResponse; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; @@ -47,14 +43,14 @@ import java.util.stream.Collectors; import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.getCurrentArguments; -import static org.junit.Assert.assertEquals; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.createPartialMock; -import static org.powermock.api.easymock.PowerMock.replay; -import static org.powermock.api.easymock.PowerMock.verifyAll; +import static org.easymock.EasyMock.partialMockBuilder; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; /** * A test base for testing Facades, specifically targeted around testing facades as different users. @@ -83,9 +79,6 @@ * * @deprecated in favour of IsaacIntegrationTest */ -@RunWith(PowerMockRunner.class) -@PrepareForTest({UserAccountManager.class}) -@PowerMockIgnore({ "jakarta.ws.*", "jakarta.management.*", "jakarta.script.*" }) @Deprecated abstract public class AbstractFacadeTest extends IsaacTest { protected Request request; @@ -98,14 +91,17 @@ abstract public class AbstractFacadeTest extends IsaacTest { private Map userSummaries = Maps.newHashMap(); - @Before + @BeforeEach public void abstractFacadeTestSetup() { httpServletRequest = createMock(HttpServletRequest.class); replay(httpServletRequest); request = createNiceMock(Request.class); // We don't particularly care about what gets called on this. replay(request); - userManager = createPartialMock(UserAccountManager.class, "getCurrentUser", "getCurrentRegisteredUser", "convertToUserSummaryObject", "getUserDTOById"); + userManager = partialMockBuilder(UserAccountManager.class) + .addMockedMethods("getCurrentUser", "getCurrentRegisteredUser", "convertToUserSummaryObject") + .addMockedMethod("getUserDTOById", Long.class) // This is overloaded so we have to specify the signature + .createMock(); registerDefaultsFor(userManager, m -> { expect(m.convertToUserSummaryObject(anyObject(RegisteredUserDTO.class))).andStubAnswer(() -> { @@ -271,7 +267,8 @@ protected RegisteredUserDTO currentUser() { /////////////////////////////////////////////////////////////////////////// private void assertErrorResponse(Status expectedStatus, Response actual) { - assertEquals("Expected status " + expectedStatus.name() + ", got " + actual.getStatusInfo().toEnum().name() + " with details " + extractErrorInfo(actual), expectedStatus.getStatusCode(), actual.getStatus()); + assertEquals(expectedStatus.getStatusCode(), actual.getStatus(), + "Expected status " + expectedStatus.name() + ", got " + actual.getStatusInfo().toEnum().name() + " with details " + extractErrorInfo(actual)); } private String extractErrorInfo(Response response) { @@ -444,7 +441,7 @@ private void runStepsAs(@Nullable RegisteredUserDTO user, Endpoint endpoint) { private void verifyEndpoint(String when, Endpoint endpoint) { try { runSteps(endpoint); - verifyAll(); + verify(userManager, httpServletRequest, request); } catch (AssertionError e) { String message = when + ", test failed, expected:\r\n"; message += steps.stream().map(s -> " " + s.toString() + "\r\n").collect(Collectors.joining()); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java index f2272c2d5f..4cc911c254 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTestWithREST.java @@ -6,12 +6,13 @@ import org.eclipse.jetty.servlet.ServletHolder; import org.jboss.resteasy.plugins.server.servlet.HttpServletDispatcher; import org.json.JSONObject; -import org.junit.function.ThrowingRunnable; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.function.Executable; import uk.ac.cam.cl.dtg.isaac.dos.users.RegisteredUser; import uk.ac.cam.cl.dtg.isaac.dto.LocalAuthDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; +import jakarta.servlet.http.HttpSession; import jakarta.ws.rs.client.Client; import jakarta.ws.rs.client.ClientBuilder; import jakarta.ws.rs.client.Entity; @@ -35,21 +36,21 @@ * interacting with the server. As the server runs in-process, mocking and debugging still work. */ public class IsaacIntegrationTestWithREST extends AbstractIsaacIntegrationTest { - private final HashSet cleanups = new HashSet<>(); + private final Set cleanups = new HashSet<>(); @SuppressWarnings({"checkstyle:EmptyCatchBlock", "checkstyle:MissingJavadocMethod"}) @AfterEach public void doCleanup() { - for (var cleanup : cleanups) { + for (Executable cleanup : cleanups) { try { - cleanup.run(); + cleanup.execute(); } catch (final Throwable ignored) { } } } - public void registerCleanup(final ThrowingRunnable cleanup) { + public void registerCleanup(final Executable cleanup) { this.cleanups.add(cleanup); } @@ -61,10 +62,10 @@ static class TestServer { private String sessionId; private final Server server; private final ServletContextHandler ctx; - private final Consumer registerCleanup; + private final Consumer registerCleanup; private TestServer( - final Server server, final ServletContextHandler ctx, final Consumer registerCleanup + final Server server, final ServletContextHandler ctx, final Consumer registerCleanup ) { this.server = server; this.ctx = ctx; @@ -72,16 +73,16 @@ private TestServer( } public static TestServer start( - final Set facades, final Consumer registerCleanup + final Set facades, final Consumer registerCleanup ) throws Exception { TestApp.facades = facades; - var server = new Server(0); - var ctx = new ServletContextHandler(ServletContextHandler.SESSIONS); + Server server = new Server(0); + ServletContextHandler ctx = new ServletContextHandler(ServletContextHandler.SESSIONS); ctx.setContextPath("/"); server.setHandler(ctx); - var servlet = new ServletHolder(new HttpServletDispatcher()); + ServletHolder servlet = new ServletHolder(new HttpServletDispatcher()); servlet.setInitParameter("jakarta.ws.rs.Application", TestApp.class.getName()); ctx.addServlet(servlet, "/*"); @@ -91,14 +92,14 @@ public static TestServer start( } public TestServer setSessionAttributes(final Map attributes) { - var session = ctx.getSessionHandler().newHttpSession(new Request(null, null)); + HttpSession session = ctx.getSessionHandler().newHttpSession(new Request(null, null)); attributes.keySet().forEach(k -> session.setAttribute(k, attributes.get(k))); sessionId = session.getId(); return this; } public TestClient client() { - var baseUrl = "http://localhost:" + server.getURI().getPort(); + String baseUrl = "http://localhost:" + server.getURI().getPort(); RequestBuilder builder = (null == this.sessionId) ? r -> r : r -> r.cookie("JSESSIONID", sessionId); return new TestClient(baseUrl, registerCleanup, builder); } @@ -115,12 +116,12 @@ public Set getSingletons() { static class TestClient { String baseUrl; - Consumer registerCleanup; + Consumer registerCleanup; RequestBuilder builder; RegisteredUserDTO currentUser; Client client; - TestClient(final String baseUrl, final Consumer registerCleanup, final RequestBuilder builder) { + TestClient(final String baseUrl, final Consumer registerCleanup, final RequestBuilder builder) { this.baseUrl = baseUrl; this.registerCleanup = registerCleanup; this.builder = builder; @@ -128,22 +129,22 @@ static class TestClient { } public TestResponse get(final String url) { - var request = client.target(baseUrl + url).request(MediaType.APPLICATION_JSON); - var response = builder.apply(request).get(); + Invocation.Builder request = client.target(baseUrl + url).request(MediaType.APPLICATION_JSON); + Response response = builder.apply(request).get(); registerCleanup.accept(response::close); return new TestResponse(response); } public TestResponse post(final String url, final Object body) { - var request = client.target(baseUrl + url).request(MediaType.APPLICATION_JSON); - var response = builder.apply(request).post(Entity.json(body)); + Invocation.Builder request = client.target(baseUrl + url).request(MediaType.APPLICATION_JSON); + Response response = builder.apply(request).post(Entity.json(body)); registerCleanup.accept(response::close); return new TestResponse(response); } public TestClient loginAs(final RegisteredUser user) { - var request = client.target(baseUrl + "/auth/SEGUE/authenticate").request(MediaType.APPLICATION_JSON); - var body = new LocalAuthDTO(); + Invocation.Builder request = client.target(baseUrl + "/auth/SEGUE/authenticate").request(MediaType.APPLICATION_JSON); + LocalAuthDTO body = new LocalAuthDTO(); body.setEmail(user.getEmail()); body.setPassword("test1234"); this.currentUser = builder.apply(request).post(Entity.json(body), RegisteredUserDTO.class); @@ -173,9 +174,9 @@ void assertNoUserLoggedIn() { } void assertUserLoggedIn(final Number userId) { - var base64Cookie = this.response.getCookies().get("SEGUE_AUTH_COOKIE").getValue(); - var cookieBytes = java.util.Base64.getDecoder().decode(base64Cookie); - var cookie = new JSONObject(new String(cookieBytes)); + String base64Cookie = this.response.getCookies().get("SEGUE_AUTH_COOKIE").getValue(); + byte[] cookieBytes = java.util.Base64.getDecoder().decode(base64Cookie); + JSONObject cookie = new JSONObject(new String(cookieBytes)); assertEquals(userId, cookie.getLong("id")); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeTest.java index 22a345f300..b136c37f4a 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuestionFacadeTest.java @@ -15,11 +15,7 @@ */ package uk.ac.cam.cl.dtg.isaac.api; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.AbstractUserPreferenceManager; import uk.ac.cam.cl.dtg.isaac.dos.IUserStreaksManager; import uk.ac.cam.cl.dtg.isaac.dos.UserPreference; @@ -42,19 +38,15 @@ import jakarta.ws.rs.core.Response.Status; import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.expect; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.createNiceMock; -import static org.powermock.api.easymock.PowerMock.replayAll; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; -@RunWith(PowerMockRunner.class) -@PrepareForTest(GitContentManager.class) -@PowerMockIgnore("javax.management.*") public class QuestionFacadeTest extends AbstractFacadeTest { private AbstractConfigLoader properties; @@ -82,7 +74,8 @@ private void setUpQuestionFacade() throws ContentManagerException { expect(contentManager.getContentDOById(studentQuizDO.getId())).andStubReturn(studentQuizDO); expect(contentManager.getContentDOById(questionPageQuestionDO.getId())).andStubReturn(questionPageQuestionDO); - replayAll(); + replay(requestForCaching, contentManager, contentSubclassMapper, questionManager, userStreaksManager, + userAssociationManager); } @Test @@ -136,6 +129,7 @@ public final void assertUserCanAnswerLLMQuestions_NonConsentingUser_ShouldThrowE UserPreference userPreference = new UserPreference(adminUser.getId(), "CONSENT", "OPENAI", false); expect(userPreferenceManager.getUserPreference("CONSENT", LLM_PROVIDER_NAME, adminUser.getId())).andReturn(userPreference); + replay(properties, userPreferenceManager); setUpQuestionFacade(); // Act & Assert @@ -163,6 +157,7 @@ public final void assertUserCanAnswerLLMQuestions_NoUses_ShouldThrowException() misuseMonitor = createMock(IMisuseMonitor.class); expect(misuseMonitor.getRemainingUses(adminUser.getId().toString(), "LLMFreeTextQuestionAttemptMisuseHandler")).andReturn(0); + replay(properties, userPreferenceManager, misuseMonitor); setUpQuestionFacade(); // Act & Assert @@ -187,14 +182,14 @@ public final void assertUserCanAnswerLLMQuestions_ConsentingUser_ShouldReturnOka userPreferenceManager = createMock(AbstractUserPreferenceManager.class); UserPreference userPreference = new UserPreference(adminUser.getId(), "CONSENT", "OPENAI", true); expect(userPreferenceManager.getUserPreference("CONSENT", LLM_PROVIDER_NAME, adminUser.getId())).andReturn(userPreference); - + replay(properties, misuseMonitor, userPreferenceManager); setUpQuestionFacade(); // Act RegisteredUserDTO outUser = questionFacade.assertUserCanAnswerLLMQuestions(adminUser); // Assert - assertThat(outUser, instanceOf(RegisteredUserDTO.class)); + assertInstanceOf(RegisteredUserDTO.class, outUser); assertEquals(adminUser.getId(), outUser.getId()); } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacadeTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacadeTest.java index 2aa25667ab..cd5af009b0 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacadeTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacadeTest.java @@ -18,12 +18,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.easymock.IAnswer; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.managers.DueBeforeNowException; import uk.ac.cam.cl.dtg.isaac.api.managers.DuplicateAssignmentException; import uk.ac.cam.cl.dtg.isaac.api.managers.QuizAssignmentManager; @@ -68,20 +64,17 @@ import static org.easymock.EasyMock.anyLong; import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.createNiceMock; import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.getCurrentArguments; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.createNiceMock; -import static org.powermock.api.easymock.PowerMock.expectLastCall; -import static org.powermock.api.easymock.PowerMock.replay; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; -@RunWith(PowerMockRunner.class) -@PrepareForTest(GitContentManager.class) -@PowerMockIgnore("javax.management.*") public class QuizFacadeTest extends AbstractFacadeTest { private QuizFacade quizFacade; @@ -96,7 +89,7 @@ public class QuizFacadeTest extends AbstractFacadeTest { private UserAssociationManager associationManager; private List studentOwnAttempts; - @Before + @BeforeEach public void setUp() throws ContentManagerException { studentOwnAttempts = ImmutableList.of(ownAttempt, ownCompletedAttempt, attemptOnNullFeedbackModeQuiz); @@ -176,7 +169,7 @@ public void setUp() throws ContentManagerException { expect(contentManager.getContentDOById(studentQuizDO.getId())).andStubReturn(studentQuizDO); expect(contentManager.getContentDOById(questionPageQuestionDO.getId())).andStubReturn(questionPageQuestionDO); - replay(requestForCaching, properties, logManager, contentManager, contentSummarizerService, quizManager, quizAssignmentManager, + replay(requestForCaching, properties, logManager, contentManager, contentSummarizerService, quizAssignmentManager, assignmentService, quizAttemptManager, quizQuestionManager, associationManager); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java index 2f4d3caac4..94ec547797 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java @@ -4,8 +4,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dao.EventBookingPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dos.AssociationToken; import uk.ac.cam.cl.dtg.isaac.dos.EventStatus; @@ -39,11 +39,10 @@ import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.expectLastCall; -import static org.easymock.EasyMock.mock; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; /** @@ -65,7 +64,7 @@ public class EventBookingManagerTest { /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { this.dummyEmailManager = createMock(EmailManager.class); this.dummyEventBookingPersistenceManager = createMock(EventBookingPersistenceManager.class); @@ -662,9 +661,9 @@ public void getPlacesAvailable_checkEventCapacity_capacityCalculatedCorrectly() // Run the test for a student event replay(mockedObjects); Long actualPlacesAvailable = ebm.getPlacesAvailable(testEvent); - Long expectedPlacesAvailable = (long)initialNumberOfPlaces - 1 - 10; - assertEquals("STUDENT events should only count confirmed and waiting list student places in availability calculations", - expectedPlacesAvailable, actualPlacesAvailable); + Long expectedPlacesAvailable = (long) initialNumberOfPlaces - 1 - 10; + assertEquals(expectedPlacesAvailable, actualPlacesAvailable, + "STUDENT events should only count confirmed and waiting list student places in availability calculations"); verify(mockedObjects); } @@ -706,8 +705,8 @@ public void getEventPage_checkWaitingListOnlyEventCapacity_capacityCalculatedCor replay(mockedObjects); Long placesAvailable = ebm.getPlacesAvailable(testEvent); Long expectedPlacesAvailable = 1L; - assertEquals("WAITING_LIST_ONLY events should only count confirmed places in availability calculations", - placesAvailable, expectedPlacesAvailable); + assertEquals(expectedPlacesAvailable, placesAvailable, + "WAITING_LIST_ONLY events should only count confirmed places in availability calculations"); verify(mockedObjects); } @@ -751,8 +750,8 @@ public void getEventPage_checkStudentEventReservedBookings_capacityCalculatedCor replay(mockedObjects); Long placesAvailable = ebm.getPlacesAvailable(testEvent); Long expectedPlacesAvailable = 0L; - assertEquals("RESERVED bookings should count towards the places available in availability calculations", - expectedPlacesAvailable, placesAvailable); + assertEquals(expectedPlacesAvailable, placesAvailable, + "RESERVED bookings should count towards the places available in availability calculations"); verify(mockedObjects); } @@ -818,9 +817,8 @@ public void isUserAbleToManageEvent_checkUsersWithDifferentRoles_success() throw ); for (RegisteredUserDTO user : expectedResults.keySet()) { - assertEquals(String.format("Test case: %s", user.getRole()), - expectedResults.get(user), - eventBookingManager.isUserAbleToManageEvent(user, testEvent)); + assertEquals(expectedResults.get(user), eventBookingManager.isUserAbleToManageEvent(user, testEvent), + String.format("Test case: %s", user.getRole())); } verify(mockedObjects); } @@ -849,7 +847,7 @@ class TestCase {IsaacEventPageDTO eventPageDTO; Boolean expected; String asserti for (TestCase testCase : testCases) { boolean actual = EventBookingManager.eventAllowsGroupBookings(testCase.eventPageDTO); - assertEquals(testCase.assertion, testCase.expected, actual); + assertEquals(testCase.expected, actual, testCase.assertion); } } @@ -914,7 +912,7 @@ public void requestReservations_reserveSpacesWhenThereAreAvailableSpaces_success replay(mockedObjects); List actualResults = eventBookingManager.requestReservations(testCase.event, students, testCase.teacher); List expectedResults = ImmutableList.of(testCase.student1Booking, testCase.student2Booking); - assertEquals("N results should be returned unaltered", expectedResults, actualResults); + assertEquals(expectedResults, actualResults, "N results should be returned unaltered"); verify(mockedObjects); } @@ -1052,7 +1050,8 @@ public void requestReservations_cancelledReservationsDoNotCountTowardsReservatio replay(mockedObjects); List actualResults = eventBookingManager.requestReservations(testCase.event, students, testCase.teacher); List expectedResults = ImmutableList.of(testCase.student1Booking); - assertEquals("Student 1 should get reserved despite the existing cancelled reservation", expectedResults, actualResults); + assertEquals(expectedResults, actualResults, + "Student 1 should get reserved despite the existing cancelled reservation"); verify(mockedObjects); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java index e9418bdea9..23b243fdca 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManagerTest.java @@ -18,13 +18,8 @@ import org.easymock.Capture; import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dao.GameboardPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dto.GameFilter; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; @@ -42,15 +37,11 @@ import java.util.Objects; import java.util.stream.Collectors; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.powermock.api.easymock.PowerMock.replay; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; - -@RunWith(PowerMockRunner.class) -@PrepareForTest(GitContentManager.class) -@PowerMockIgnore("javax.management.*") public class GameManagerTest { private GitContentManager dummyContentManager; @@ -59,13 +50,13 @@ public class GameManagerTest { private QuestionManager dummyQuestionManager; private AbstractConfigLoader dummyConfigLoader; - @Before + @BeforeEach public void setUp() { - this.dummyContentManager = PowerMock.createMock(GitContentManager.class); - this.dummyGameboardPersistenceManager = PowerMock.createMock(GameboardPersistenceManager.class); - this.dummyMapper = PowerMock.createMock(MainMapper.class); - this.dummyQuestionManager = PowerMock.createMock(QuestionManager.class); - this.dummyConfigLoader = PowerMock.createMock(YamlLoader.class); + this.dummyContentManager = EasyMock.createMock(GitContentManager.class); + this.dummyGameboardPersistenceManager = EasyMock.createMock(GameboardPersistenceManager.class); + this.dummyMapper = EasyMock.createMock(MainMapper.class); + this.dummyQuestionManager = EasyMock.createMock(QuestionManager.class); + this.dummyConfigLoader = EasyMock.createMock(YamlLoader.class); EasyMock.expect(dummyConfigLoader.getProperty(GAMEBOARD_QUESTION_LIMIT)).andStubReturn("30"); replay(dummyConfigLoader); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAssignmentManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAssignmentManagerTest.java index db73398b9e..29aebcc790 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAssignmentManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAssignmentManagerTest.java @@ -15,8 +15,8 @@ */ package uk.ac.cam.cl.dtg.isaac.api.managers; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.services.EmailService; import uk.ac.cam.cl.dtg.isaac.dao.IQuizAssignmentPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dos.QuizFeedbackMode; @@ -30,13 +30,14 @@ import java.util.List; import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.eq; import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.resetToNice; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static uk.ac.cam.cl.dtg.segue.api.Constants.HOST_NAME; public class QuizAssignmentManagerTest extends AbstractManagerTest { @@ -48,7 +49,7 @@ public class QuizAssignmentManagerTest extends AbstractManagerTest { private QuizAssignmentDTO newAssignment; - @Before + @BeforeEach public void setUp() throws ContentManagerException, SegueDatabaseException { AbstractConfigLoader properties = createMock(AbstractConfigLoader.class); emailService = createMock(EmailService.class); @@ -61,7 +62,7 @@ public void setUp() throws ContentManagerException, SegueDatabaseException { replay(properties, emailService, quizAssignmentPersistenceManager); } - @Before + @BeforeEach public void initializeAdditionalObjects() { newAssignment = new QuizAssignmentDTO( null, studentQuiz.getId(), @@ -100,22 +101,24 @@ public void createAnotherAssignmentAfterFirstIsDueSucceeds() throws SegueDatabas quizAssignmentManager.createAssignment(newAssignment); } - @Test(expected = DueBeforeNowException.class) - public void createAssignmentFailsInThePast() throws SegueDatabaseException, ContentManagerException { - newAssignment.setDueDate(somePastDate); - - quizAssignmentManager.createAssignment(newAssignment); + @Test + public void createAssignmentFailsInThePast() { + assertThrows(DueBeforeNowException.class, () -> { + newAssignment.setDueDate(somePastDate); + quizAssignmentManager.createAssignment(newAssignment); + }); } - @Test(expected = DuplicateAssignmentException.class) - public void createDuplicateAssignmentFails() throws SegueDatabaseException, ContentManagerException { - - withMock(quizAssignmentPersistenceManager, m -> { - expect(m.getAssignmentsByQuizIdAndGroup( - studentQuiz.getId(), studentGroup.getId())).andReturn(Collections.singletonList(studentAssignment)); + @Test + public void createDuplicateAssignmentFails() { + assertThrows(DuplicateAssignmentException.class, () -> { + withMock(quizAssignmentPersistenceManager, m -> { + expect(m.getAssignmentsByQuizIdAndGroup( + studentQuiz.getId(), studentGroup.getId())).andReturn(Collections.singletonList(studentAssignment)); + }); + + quizAssignmentManager.createAssignment(newAssignment); }); - - quizAssignmentManager.createAssignment(newAssignment); } @Test diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAttemptManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAttemptManagerTest.java index 8d332126a9..233f9d8ba1 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAttemptManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizAttemptManagerTest.java @@ -17,8 +17,8 @@ import org.easymock.EasyMock; import org.easymock.IArgumentMatcher; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dao.IQuizAttemptPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dto.QuizAttemptDTO; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; @@ -28,10 +28,11 @@ import java.util.List; import java.util.Objects; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; -import static org.junit.Assert.assertEquals; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replay; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class QuizAttemptManagerTest extends AbstractManagerTest { private static final Long TEST_ID = 0xC0000000000L; @@ -39,7 +40,7 @@ public class QuizAttemptManagerTest extends AbstractManagerTest { private IQuizAttemptPersistenceManager quizAttemptPersistenceManager; - @Before + @BeforeEach public void setUp() { quizAttemptPersistenceManager = createMock(IQuizAttemptPersistenceManager.class); @@ -56,11 +57,12 @@ public void fetchOrCreateWithExistingAttempt() throws AttemptCompletedException, assertEquals(studentAttempt, attempt); } - @Test(expected = AttemptCompletedException.class) + @Test public void fetchOrCreateWithExistingCompletedAttemptFails() throws AttemptCompletedException, SegueDatabaseException { - withMock(quizAttemptPersistenceManager, forStudentAssignmentReturn(completedAttempt)); - - quizAttemptManager.fetchOrCreate(studentAssignment, student); + assertThrows(AttemptCompletedException.class, () -> { + withMock(quizAttemptPersistenceManager, forStudentAssignmentReturn(completedAttempt)); + quizAttemptManager.fetchOrCreate(studentAssignment, student); + }); } @Test diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java index b62560692f..ae47e7b790 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizManagerTest.java @@ -1,8 +1,8 @@ package uk.ac.cam.cl.dtg.isaac.api.managers; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.services.ContentSummarizerService; import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuizDTO; import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuizSectionDTO; @@ -15,11 +15,11 @@ import java.util.List; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replayAll; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class QuizManagerTest extends AbstractManagerTest { @@ -27,7 +27,7 @@ public class QuizManagerTest extends AbstractManagerTest { private AbstractConfigLoader properties; private IsaacQuizDTO brokenQuiz; - @Before + @BeforeEach public void setUp() { properties = createMock(AbstractConfigLoader.class); @@ -39,7 +39,7 @@ public void setUp() { brokenQuiz = new IsaacQuizDTO(); brokenQuiz.setChildren(ImmutableList.of(quizSection1, new ContentDTO(), quizSection2)); - replayAll(); + replay(properties, contentService, contentManager, contentSummarizerService); } @Test diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizQuestionManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizQuestionManagerTest.java index f175bd82b0..1ca7f763c3 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizQuestionManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/QuizQuestionManagerTest.java @@ -17,8 +17,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dao.IQuizQuestionAttemptPersistenceManager; import uk.ac.cam.cl.dtg.isaac.dos.QuizFeedbackMode; import uk.ac.cam.cl.dtg.isaac.dto.QuizAttemptDTO; @@ -46,14 +46,14 @@ import static java.util.Collections.singleton; import static java.util.Collections.singletonList; +import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replay; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; public class QuizQuestionManagerTest extends AbstractManagerTest { @@ -75,8 +75,9 @@ public class QuizQuestionManagerTest extends AbstractManagerTest { private Map answerMap; private QuizAttemptManager quizAttemptManager; - @Before + @BeforeEach public void setUp() { + initializeAdditionalObjects(); quizQuestionAttemptPersistenceManager = createMock(IQuizQuestionAttemptPersistenceManager.class); questionManager = createMock(QuestionManager.class); MainMapper contentMapper = createMock(MainMapper.class); @@ -95,7 +96,6 @@ public void setUp() { replay(quizQuestionAttemptPersistenceManager, questionManager, contentMapper, quizAttemptManager); } - @Before public void initializeAdditionalObjects() { correctAnswer = new Choice(); correctAnswerDTO = new ChoiceDTO(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManagerTest.java index bcaf2a1e81..116f98b372 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManagerTest.java @@ -2,8 +2,8 @@ import com.google.api.client.util.Lists; import com.google.api.client.util.Maps; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import uk.ac.cam.cl.dtg.isaac.dos.LightweightQuestionValidationResponse; @@ -13,8 +13,8 @@ import java.util.List; import java.util.Map; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; import static uk.ac.cam.cl.dtg.segue.api.managers.QuestionManager.extractPageIdFromQuestionId; @@ -38,7 +38,7 @@ public class UserAttemptManagerTest { private final LightweightQuestionValidationResponse someOtherCorrectAttempt = new LightweightQuestionValidationResponse("some-other-part-id", true, null); - @Before + @BeforeEach public void setUp() throws Exception { fakeQuestionSummary = new ContentSummaryDTO(); fakeQuestionSummary.setId(QUESTION_ID); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java index 104f1b4036..d505d62038 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/app/GameboardsFacadeTest.java @@ -16,9 +16,8 @@ package uk.ac.cam.cl.dtg.isaac.app; import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.Constants; import uk.ac.cam.cl.dtg.isaac.api.GameboardsFacade; import uk.ac.cam.cl.dtg.isaac.api.managers.FastTrackManger; @@ -41,7 +40,7 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the user manager class. @@ -62,7 +61,7 @@ public class GameboardsFacadeTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.dummyPropertiesLoader = createMock(AbstractConfigLoader.class); this.dummyGameManager = createMock(GameManager.class); @@ -81,7 +80,6 @@ public final void setUp() throws Exception { * @throws ContentManagerException */ @Test - @PowerMockIgnore({ "jakarta.ws.*" }) public final void isaacEndPoint_checkEmptyGameboardCausesErrorNoUser_SegueErrorResponseShouldBeReturned() throws SegueDatabaseException, ContentManagerException { GameboardsFacade gameboardFacade = new GameboardsFacade( diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java index 04f5403f06..ea7ec55ea5 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookingsTest.java @@ -1,8 +1,8 @@ package uk.ac.cam.cl.dtg.isaac.dos.eventbookings; import com.fasterxml.jackson.databind.ObjectMapper; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; import uk.ac.cam.cl.dtg.isaac.dos.users.Role; @@ -18,7 +18,7 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; public class PgEventBookingsTest { private PgEventBookings buildPgEventBookings() { @@ -31,7 +31,7 @@ private PgEventBookings buildPgEventBookings() { private PreparedStatement dummyPreparedStatement; private ResultSet dummyResultSet; - @Before + @BeforeEach public final void setUp() throws Exception { this.dummyPostgresSqlDb = createMock(PostgresSqlDb.class); this.dummyObjectMapper = createMock(ObjectMapper.class); @@ -87,7 +87,7 @@ public void getEventBookingStatusCounts_checkEventBookingStatusCounts_canCopeWit PgEventBookings pgEventBookings = this.buildPgEventBookings(); Map> actualStatusCounts = pgEventBookings.getEventBookingStatusCounts("someEventId", true); - assertEquals("Every row should be represented in the result", expectedStatusCounts, actualStatusCounts); + assertEquals(expectedStatusCounts, actualStatusCounts, "Every row should be represented in the result"); verify(mockedObjects); } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java index 05fabe8ec5..6f2022cbba 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacClozeValidatorTest.java @@ -17,8 +17,8 @@ import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacClozeQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.ItemValidationResponse; @@ -33,11 +33,11 @@ import java.util.Collections; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class IsaacClozeValidatorTest { private IsaacClozeValidator validator; @@ -55,7 +55,7 @@ public class IsaacClozeValidatorTest { /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacClozeValidator(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java index de4442f993..47d6976767 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacCoordinateValidatorTest.java @@ -17,8 +17,8 @@ import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacCoordinateQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.content.Choice; @@ -29,10 +29,10 @@ import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; public class IsaacCoordinateValidatorTest { @@ -51,7 +51,7 @@ public class IsaacCoordinateValidatorTest { /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacCoordinateValidator(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java index e22824e4c7..fd3fd269b9 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacDndValidatorTest.java @@ -18,10 +18,9 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.Logger; -import org.junit.experimental.theories.DataPoints; -import org.junit.experimental.theories.Theories; -import org.junit.experimental.theories.Theory; -import org.junit.runner.RunWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import uk.ac.cam.cl.dtg.isaac.api.Constants; import uk.ac.cam.cl.dtg.isaac.api.TestAppender; import uk.ac.cam.cl.dtg.isaac.dos.DndValidationResponse; @@ -47,13 +46,12 @@ import java.util.stream.Stream; import static java.lang.String.format; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + -@RunWith(Theories.class) -@SuppressWarnings("checkstyle:MissingJavadocType") public class IsaacDndValidatorTest { public static final Item item_3cm = item("6d3d", "3 cm"); public static final Item item_4cm = item("6d3e", "4 cm"); @@ -62,146 +60,285 @@ public class IsaacDndValidatorTest { public static final Item item_12cm = item("6d3h", "12 cm"); public static final Item item_13cm = item("6d3i", "13 cm"); - @DataPoints - public static CorrectnessTestCase[] correctnessTestCases = { - new CorrectnessTestCase().setTitle("singleCorrectMatch_Correct") - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .expectCorrect(true), - - new CorrectnessTestCase().setTitle("singleCorrectNotMatch_Incorrect") - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .setAnswer(choice(item(item_4cm, "leg_1"), item(item_5cm, "leg_2"), item(item_3cm, "hypothenuse"))) - .expectCorrect(false), - - new CorrectnessTestCase().setTitle("singleCorrectPartialMatch_Incorrect") - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .setAnswer(choice(item(item_5cm, "leg_1"), item(item_4cm, "leg_2"), item(item_3cm, "hypothenuse"))) - .expectCorrect(false), - - new CorrectnessTestCase().setTitle("sameAnswerCorrectAndIncorrect_Correct") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(incorrectChoice(item(item_3cm, "leg_1")), correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectCorrect(true) - }; - - @Theory + + private static Stream correctnessTestCases() { + return Stream.of( + new CorrectnessTestCase().setTitle("singleCorrectMatch_Correct") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .expectCorrect(true), + + new CorrectnessTestCase().setTitle("singleCorrectNotMatch_Incorrect") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .setAnswer(choice(item(item_4cm, "leg_1"), item(item_5cm, "leg_2"), item(item_3cm, "hypothenuse"))) + .expectCorrect(false), + + new CorrectnessTestCase().setTitle("singleCorrectPartialMatch_Incorrect") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .setAnswer(choice(item(item_5cm, "leg_1"), item(item_4cm, "leg_2"), item(item_3cm, "hypothenuse"))) + .expectCorrect(false), + + new CorrectnessTestCase().setTitle("sameAnswerCorrectAndIncorrect_Correct") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(incorrectChoice(item(item_3cm, "leg_1")), correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectCorrect(true) + ).map(Arguments::of); + } + + private static Stream explanationTestCases() { + return Stream.of( + new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1")), + incorrectChoice(ExplanationTestCase.testFeedback, item(item_4cm, "leg_1"))) + .setAnswer(choice(item(item_4cm, "leg_1"))) + .expectCorrect(false).expectExplanation(ExplanationTestCase.testFeedback.getValue()), + + new ExplanationTestCase().setTitle("exactMatchCorrect_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(ExplanationTestCase.testFeedback, item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectCorrect(true).expectExplanation(ExplanationTestCase.testFeedback.getValue()), + + new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1")), + incorrectChoice(item(item_4cm, "leg_1"))) + .tapQuestion(q -> q.setDefaultFeedback(ExplanationTestCase.testFeedback)) + .setAnswer(choice(item(item_4cm, "leg_1"))) + .expectCorrect(false).expectExplanation(ExplanationTestCase.testFeedback.getValue()), + + new ExplanationTestCase().setTitle("matchIncorrectSubset_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), + incorrectChoice(ExplanationTestCase.testFeedback, item(item_5cm, "leg_1"))) + .setAnswer(choice(item(item_5cm, "leg_1"), item(item_6cm, "leg_2"))) + .expectCorrect(false).expectExplanation(ExplanationTestCase.testFeedback.getValue()), + + new ExplanationTestCase().setTitle("multiMatchIncorrectSubset_shouldReturnMatching") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), + incorrectChoice(new Content("leg_1 can't be 5"), item(item_5cm, "leg_1")), + incorrectChoice(new Content("leg_2 can't be 6"), item(item_6cm, "leg_2"))) + .setAnswer(choice(item(item_5cm, "leg_1"), item(item_6cm, "leg_2"))) + .expectCorrect(false).expectExplanation("leg_1 can't be 5"), + + new ExplanationTestCase().setTitle("unMatchedIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) + .setAnswer(choice(item(item_4cm, "leg_1"))) + .expectCorrect(false).expectExplanation("default feedback for question"), + + new ExplanationTestCase().setTitle("partialMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), + incorrectChoice(new Content("feedback for choice"), item(item_5cm, "leg_1"), item(item_6cm, "leg_2"))) + .tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) + .setAnswer(choice(item(item_5cm, "leg_1"), item(item_12cm, "leg_2"))) + .expectCorrect(false).expectExplanation("default feedback for question"), + + new ExplanationTestCase().setTitle("matchedCorrectWithDefaultFeedback_shouldReturnDefaultFeedback") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectExplanation("default feedback for question").expectCorrect(true), + + new ExplanationTestCase().setTitle("matchedCorrectNoDefaultFeedback_shouldReturnNone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectNoExplanation().expectCorrect(true), + + new ExplanationTestCase().setTitle("noDefaultIncorrect_shouldReturnNone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_4cm, "leg_1"))) + .expectNoExplanation().expectCorrect(false), + + // these highlight inconsistent behaviour when a question violates our requirements for drop zones + new ExplanationTestCase().setTitle("unrecognisedDropZone_shouldReturnNone") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_2"))) + .expectNoExplanation().expectCorrect(false), + + new ExplanationTestCase().setTitle("correctAnswerHasUnusedDropZones_notAcceptedCorrect") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1")), + correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"))) + .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"))) + .expectCorrect(false) + .expectExplanation("The question is invalid, because it has an answer with more items than we have gaps.") + .expectNoDropZones() + ).map(Arguments::of); + } + + private static Stream dropZonesCorrectTestCases() { + Supplier disabledItemFeedbackNoDropZones = () -> new DropZonesTestCase() + .tapQuestion(q -> q.setDetailedItemFeedback(false)).expectNoDropZones(); + Supplier enabledItemFeedback = () -> new DropZonesTestCase() + .tapQuestion(q -> q.setDetailedItemFeedback(true)); + + return Stream.of( + disabledItemFeedbackNoDropZones.get().setTitle("incorrectNotRequested_NotReturned") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_4cm, "leg_1"))).expectCorrect(false), + + disabledItemFeedbackNoDropZones.get().setTitle("correctNotRequested_NotReturned") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"))).expectCorrect(true), + + enabledItemFeedback.get().setTitle("allCorrect_ShouldReturnAllCorrect") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .expectCorrect(true).expectDropZonesCorrect(d -> d.setLeg1(true).setLeg2(true).setHypothenuse(true)), + + enabledItemFeedback.get().setTitle("someIncorrect_ShouldReturnWhetherCorrect") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .setAnswer(choice(item(item_6cm, "leg_1"), item(item_5cm, "leg_2"), item(item_5cm, "hypothenuse"))) + .expectCorrect(false).expectDropZonesCorrect(d -> d.setLeg1(false).setLeg2(false).setHypothenuse(true)), + + enabledItemFeedback.get().setTitle("multipleCorrectAnswers_decidesCorrectnessBasedOnClosestMatch") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")), + correctChoice(item(item_5cm, "leg_1"), item(item_12cm, "leg_2"), item(item_13cm, "hypothenuse"))) + .setAnswer(choice(item(item_5cm, "leg_1"))) + .expectCorrect(false).expectDropZonesCorrect(d -> d.setLeg1(true)), + + // these highlight inconsistent behaviour when a question violates our requirements for drop zones + enabledItemFeedback.get().setTitle("unrecognisedDropZone_treatsItAsAnyWrongAnswer") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_2"))) + .expectCorrect(false).expectDropZonesCorrect(d -> d.setLeg2(false)), + + enabledItemFeedback.get().setTitle("correctAnswerHasUnusedDropZones_acceptedCorrect") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectCorrect(true).expectDropZonesCorrect(d -> d.setLeg1(true)) + ).map(Arguments::of); + } + + private static Stream answerValidationTestCases() { + return Stream.of( + new AnswerValidationTestCase().setTitle("itemsNull") + .setAnswer(choice()) + .expectExplanation("You provided an empty answer."), + + new AnswerValidationTestCase().setTitle("itemsEmpty") + .setAnswer(new DndChoice()) + .expectExplanation("You provided an empty answer."), + + new AnswerValidationTestCase().setTitle("itemsNotEnough") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"))) + .setAnswer(choice(item(item_3cm, "leg_1"))) + .expectExplanation(IsaacDndValidator.FEEDBACK_ANSWER_NOT_ENOUGH) + .expectDropZonesCorrect(f -> f.setLeg1(true)), + + new AnswerValidationTestCase().setTitle("itemsTooMany") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_1"))) + .expectExplanation("You provided an answer with more items than we have gaps."), + + new AnswerValidationTestCase().setTitle("itemNotOnQuestion") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(new Item("bad_id", "some_value"), "leg_1"))) + .expectExplanation("You provided an answer with unrecognised items."), + + new AnswerValidationTestCase().setTitle("itemMissingId") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(new Item(null, null), "leg_1"))) + .expectExplanation("You provided an answer in an unrecognised format."), + + new AnswerValidationTestCase().setTitle("itemMissingDropZoneId") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"))) + .setAnswer(choice(item(item_3cm, null))) + .expectExplanation("You provided an answer in an unrecognised format."), + + new AnswerValidationTestCase().setTitle("itemsNotEnough_providesSpecificExplanationFirst") + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")), + incorrectChoice(new Content("Leg 1 should be less than 4 cm"), item(item_4cm, "leg_1"))) + .setAnswer(choice(item(item_4cm, "leg_1"))).expectExplanation("Leg 1 should be less than 4 cm") + .expectDropZonesCorrect(f -> f.setLeg1(false)) + ).map(Arguments::of); + } + + private static Stream questionValidationTestCases() { + Supplier itemUnrecognisedFormatCase = () -> new QuestionValidationTestCase() + .expectExplLog("The question is invalid, because it has an answer in an unrecognised format."); + Supplier noAnswersTestCase = () -> new QuestionValidationTestCase() + .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_NO_ANSWERS); + Supplier noCorrectAnswersTestCase = () -> new QuestionValidationTestCase() + .expectExplLog(Constants.FEEDBACK_NO_CORRECT_ANSWERS); + + return Stream.of( + noAnswersTestCase.get().setTitle("answersEmpty") + .setQuestion(q -> q.setChoices(List.of())), + noAnswersTestCase.get().setTitle("answersNull") + .setQuestion(q -> q.setChoices(null)), + noCorrectAnswersTestCase.get().setTitle("answersAllIncorrect") + .setQuestion(incorrectChoice(item(item_3cm, "leg_1"))), + noCorrectAnswersTestCase.get().setTitle("answerNoExplicitCorrectness_Incorrect") + .setQuestion(choice(item(item_3cm, "leg_1"))), + new QuestionValidationTestCase().setTitle("answerWrongType") + .setQuestion(q -> q.setChoices(List.of(parsonsChoice()))) + .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_INVALID_ANS), + new QuestionValidationTestCase().setTitle("answerItemsEmpty") + .setQuestion(correctChoice()) + .expectExplLog("The question is invalid, because it has an empty answer."), + new QuestionValidationTestCase().setTitle("answerItemsNull") + .setQuestion(q -> q.setChoices(Stream.of(new DndChoice()).peek(c -> c.setCorrect(true)).collect(Collectors.toList()))) + .expectExplLog("The question is invalid, because it has an empty answer."), + new QuestionValidationTestCase().setTitle("answerItemNonDnd") + .setQuestion(correctChoice(new DndItemEx("id", "value", "dropZoneId"))) + .expectExplLog("The question is invalid, because it has an invalid answer."), + itemUnrecognisedFormatCase.get().setTitle("answerItemMissingItemId") + .setQuestion(correctChoice(new DndItem(null, "value", "dropZoneId"))), + itemUnrecognisedFormatCase.get().setTitle("answerItemEmptyItemId") + .setQuestion(correctChoice(new DndItem("", "value", "dropZoneId"))), + itemUnrecognisedFormatCase.get().setTitle("answerItemMissingDropZoneId") + .setQuestion(correctChoice(new DndItem("item_id", "value", null))), + itemUnrecognisedFormatCase.get().setTitle("answerItemEmptyDropZoneId") + .setQuestion(correctChoice(new DndItem("item_id", "value", ""))), + new QuestionValidationTestCase().setTitle("itemsNull") + .tapQuestion(q -> q.setItems(null)) + .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_MISSING_ITEMS), + new QuestionValidationTestCase().setTitle("itemsEmpty") + .tapQuestion(q -> q.setItems(List.of())) + .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_MISSING_ITEMS), + new QuestionValidationTestCase().setTitle("answerInvalidItemReference") + .setChildren(List.of(new Content("[drop-zone:leg_1]"))) + .setQuestion(correctChoice(new DndItem("invalid_id", "some_value", "leg_1"))) + .expectExplLog("The question is invalid, because it has an answer with unrecognised items."), + new QuestionValidationTestCase().setTitle("answerDuplicateDropZones") + .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) + .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_3cm, "leg_1"))) + .expectExplLog("The question is invalid, because it has an answer with duplicate drop zones.") + ).map(Arguments::of); + } + + @ParameterizedTest + @MethodSource("correctnessTestCases") public final void testCorrectness(final CorrectnessTestCase testCase) { - var response = testValidate(testCase.question, testCase.answer); + DndValidationResponse response = testValidate(testCase.question, testCase.answer); assertEquals(testCase.correct, response.isCorrect()); } - @DataPoints - public static ExplanationTestCase[] explanationTestCases = { - new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnMatching") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion( - correctChoice(item(item_3cm, "leg_1")), - incorrectChoice(ExplanationTestCase.testFeedback, item(item_4cm, "leg_1")) - ).setAnswer(choice(item(item_4cm, "leg_1"))) - .expectCorrect(false) - .expectExplanation(ExplanationTestCase.testFeedback.getValue()), - - new ExplanationTestCase().setTitle("exactMatchCorrect_shouldReturnMatching") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(ExplanationTestCase.testFeedback, item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectCorrect(true) - .expectExplanation(ExplanationTestCase.testFeedback.getValue()), - - new ExplanationTestCase().setTitle("exactMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1")), incorrectChoice(item(item_4cm, "leg_1"))) - .tapQuestion(q -> q.setDefaultFeedback(ExplanationTestCase.testFeedback)) - .setAnswer(choice(item(item_4cm, "leg_1"))) - .expectCorrect(false) - .expectExplanation(ExplanationTestCase.testFeedback.getValue()), - - new ExplanationTestCase().setTitle("matchIncorrectSubset_shouldReturnMatching") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion( - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), - incorrectChoice(ExplanationTestCase.testFeedback, item(item_5cm, "leg_1")) - ).setAnswer(choice(item(item_5cm, "leg_1"), item(item_6cm, "leg_2"))) - .expectCorrect(false) - .expectExplanation(ExplanationTestCase.testFeedback.getValue()), - - new ExplanationTestCase().setTitle("multiMatchIncorrectSubset_shouldReturnMatching") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion( - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), - incorrectChoice(new Content("leg_1 can't be 5"), item(item_5cm, "leg_1")), - incorrectChoice(new Content("leg_2 can't be 6"), item(item_6cm, "leg_2")) - ).setAnswer(choice(item(item_5cm, "leg_1"), item(item_6cm, "leg_2"))) - .expectCorrect(false) - .expectExplanation("leg_1 can't be 5"), - - new ExplanationTestCase().setTitle("unMatchedIncorrect_shouldReturnDefaultFeedbackForQuestion") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) - .setAnswer(choice(item(item_4cm, "leg_1"))) - .expectCorrect(false) - .expectExplanation("default feedback for question"), - - new ExplanationTestCase().setTitle("partialMatchIncorrect_shouldReturnDefaultFeedbackForQuestion") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion( - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")), - incorrectChoice(new Content("feedback for choice"), item(item_5cm, "leg_1"), item(item_6cm, "leg_2")) - ).tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) - .setAnswer(choice(item(item_5cm, "leg_1"), item(item_12cm, "leg_2"))) - .expectCorrect(false) - .expectExplanation("default feedback for question"), - - new ExplanationTestCase().setTitle("matchedCorrectWithDefaultFeedback_shouldReturnDefaultFeedback") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .tapQuestion(q -> q.setDefaultFeedback(new Content("default feedback for question"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectExplanation("default feedback for question") - .expectCorrect(true), - - new ExplanationTestCase().setTitle("matchedCorrectNoDefaultFeedback_shouldReturnNone") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectNoExplanation() - .expectCorrect(true), - - new ExplanationTestCase().setTitle("noDefaultIncorrect_shouldReturnNone") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_4cm, "leg_1"))) - .expectNoExplanation() - .expectCorrect(false), - - // these highlight inconsistent behaviour when a question violates our requirements for drop zones - new ExplanationTestCase().setTitle("unrecognisedDropZone_shouldReturnNone") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_2"))) - .expectNoExplanation() - .expectCorrect(false), - - new ExplanationTestCase().setTitle("correctAnswerHasUnusedDropZones_notAcceptedCorrect") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion( - correctChoice(item(item_3cm, "leg_1")), - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2")) - ).setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"))) - .expectCorrect(false) - .expectExplanation("The question is invalid, because it has an answer with more items than we have gaps.") - .expectNoDropZones() - }; - - - @SuppressWarnings("checkstyle:MissingJavadocMethod") - @Theory + @ParameterizedTest + @MethodSource("explanationTestCases") public final void testExplanation(final ExplanationTestCase testCase) { - var response = testValidate(testCase.question, testCase.answer); - assertEquals(response.isCorrect(), testCase.correct); + DndValidationResponse response = testValidate(testCase.question, testCase.answer); + assertEquals(testCase.correct, response.isCorrect()); if (testCase.feedback != null) { assertNotNull(response.getExplanation()); assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); @@ -210,223 +347,48 @@ public final void testExplanation(final ExplanationTestCase testCase) { } } - static Supplier disabledItemFeedbackNoDropZones = () -> new DropZonesTestCase() - .tapQuestion(q -> q.setDetailedItemFeedback(false)) - .expectNoDropZones(); - - static Supplier enabledItemFeedback = () -> new DropZonesTestCase() - .tapQuestion(q -> q.setDetailedItemFeedback(true)); - - @DataPoints - public static DropZonesTestCase[] dropZonesCorrectTestCases = { - disabledItemFeedbackNoDropZones.get().setTitle("incorrectNotRequestsed_NotReturned") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_4cm, "leg_1"))) - .expectCorrect(false), - - disabledItemFeedbackNoDropZones.get().setTitle("correctNotRequested_NotReturned") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectCorrect(true), - - enabledItemFeedback.get().setTitle("allCorrect_ShouldReturnAllCorrect") - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .expectCorrect(true) - .expectDropZonesCorrect(d -> d.setLeg1(true).setLeg2(true).setHypothenuse(true)), - - enabledItemFeedback.get().setTitle("someIncorrect_ShouldReturnWhetherCorrect") - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .setAnswer(choice(item(item_6cm, "leg_1"), item(item_5cm, "leg_2"), item(item_5cm, "hypothenuse"))) - .expectCorrect(false) - .expectDropZonesCorrect(d -> d.setLeg1(false).setLeg2(false).setHypothenuse(true)), - - enabledItemFeedback.get().setTitle("multipleCorrectAnswers_decidesCorrectnessBasedOnClosesMatch") - .setQuestion( - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")), - correctChoice(item(item_5cm, "leg_1"), item(item_12cm, "leg_2"), item(item_13cm, "hypothenuse")) - ).setAnswer(choice(item(item_5cm, "leg_1"))) - .expectCorrect(false) - .expectDropZonesCorrect(d -> d.setLeg1(true)), - - // these highlight inconsistent behaviour when a question violates our requirements for drop zones - enabledItemFeedback.get().setTitle("unrecognisedDropZone_treatsItAsAnyWrongAnswer") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_2"))) - .expectCorrect(false) - .expectDropZonesCorrect(d -> d.setLeg2(false)), - - enabledItemFeedback.get().setTitle("correctAnswerHasUnusedDropZones_acceptedCorrect") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectCorrect(true) - .expectDropZonesCorrect(d -> d.setLeg1(true)), - }; - - @SuppressWarnings("checkstyle:MissingJavadocMethod") - @Theory + @ParameterizedTest + @MethodSource("dropZonesCorrectTestCases") public final void testDropZonesCorrect(final DropZonesTestCase testCase) { - var response = testValidate(testCase.question, testCase.answer); + DndValidationResponse response = testValidate(testCase.question, testCase.answer); assertEquals(testCase.correct, response.isCorrect()); assertEquals(testCase.dropZonesCorrect, response.getDropZonesCorrect()); } - @DataPoints - public static AnswerValidationTestCase[] answerValidationTestCases = { - new AnswerValidationTestCase().setTitle("itemsNull") - .setAnswer(choice()) - .expectExplanation("You provided an empty answer."), - - new AnswerValidationTestCase().setTitle("itemsEmpty") - .setAnswer(new DndChoice()) - .expectExplanation("You provided an empty answer."), - - new AnswerValidationTestCase().setTitle("itemsNotEnough") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"))) - .setAnswer(choice(item(item_3cm, "leg_1"))) - .expectExplanation(IsaacDndValidator.FEEDBACK_ANSWER_NOT_ENOUGH) - .expectDropZonesCorrect(f -> f.setLeg1(true)), - - new AnswerValidationTestCase().setTitle("itemsTooMany") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, "leg_1"), item(item_4cm, "leg_1"))) - .expectExplanation("You provided an answer with more items than we have gaps."), - - new AnswerValidationTestCase().setTitle("itemNotOnQuestion") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(new Item("bad_id", "some_value"), "leg_1"))) - .expectExplanation("You provided an answer with unrecognised items."), - - new AnswerValidationTestCase().setTitle("itemMissingId") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(new Item(null, null), "leg_1"))) - .expectExplanation("You provided an answer in an unrecognised format."), - - new AnswerValidationTestCase().setTitle("itemMissingDropZoneId") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"))) - .setAnswer(choice(item(item_3cm, null))) - .expectExplanation("You provided an answer in an unrecognised format."), - - new AnswerValidationTestCase().setTitle("itemsNotEnough_providesSpecificExplanationFirst") - .setQuestion( - correctChoice(item(item_3cm, "leg_1"), item(item_4cm, "leg_2"), item(item_5cm, "hypothenuse")), - incorrectChoice(new Content("Leg 1 should be less than 4 cm"), item(item_4cm, "leg_1")) - ).setAnswer(choice(item(item_4cm, "leg_1"))) - .expectExplanation("Leg 1 should be less than 4 cm") - .expectDropZonesCorrect(f -> f.setLeg1(false)), - }; - - @SuppressWarnings("checkstyle:MissingJavadocMethod") - @Theory + @ParameterizedTest + @MethodSource("answerValidationTestCases") public final void testAnswerValidation(final AnswerValidationTestCase testCase) { testCase.question.setDetailedItemFeedback(true); - var response = testValidate(testCase.question, testCase.answer); + DndValidationResponse response = testValidate(testCase.question, testCase.answer); assertFalse(response.isCorrect()); assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); assertEquals(testCase.dropZonesCorrect, response.getDropZonesCorrect()); } - static Supplier itemUnrecognisedFormatCase = () -> new QuestionValidationTestCase() - .expectExplLog("The question is invalid, because it has an answer in an unrecognised format."); - static Supplier noAnswersTestCase = () -> new QuestionValidationTestCase() - .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_NO_ANSWERS); - static Supplier noCorrectAnswersTestCase = () -> new QuestionValidationTestCase() - .expectExplLog(Constants.FEEDBACK_NO_CORRECT_ANSWERS); - - - @DataPoints - public static QuestionValidationTestCase[] questionValidationTestCases = { - noAnswersTestCase.get().setTitle("answersEmpty").setQuestion(q -> q.setChoices(List.of())), - - noAnswersTestCase.get().setTitle("answersNull").setQuestion(q -> q.setChoices(null)), - - noCorrectAnswersTestCase.get().setTitle("answersAllIncorrect") - .setQuestion(incorrectChoice(item(item_3cm, "leg_1"))), - - noCorrectAnswersTestCase.get().setTitle("answerNoExplicitCorrectness_Incorrect") - .setQuestion(choice(item(item_3cm, "leg_1"))), - - new QuestionValidationTestCase().setTitle("answerWrongType") - .setQuestion(q -> q.setChoices(List.of(parsonsChoice()))) - .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_INVALID_ANS), - - new QuestionValidationTestCase().setTitle("answerItemsEmpty").setQuestion(correctChoice()) - .expectExplLog("The question is invalid, because it has an empty answer."), - - new QuestionValidationTestCase().setTitle("answerItemsNull") - .setQuestion(q -> q.setChoices( - Stream.of(new DndChoice()).peek(c -> c.setCorrect(true)).collect(Collectors.toList()))) - .expectExplLog("The question is invalid, because it has an empty answer."), - - new QuestionValidationTestCase().setTitle("answerItemNonDnd") - .setQuestion(correctChoice(new DndItemEx("id", "value", "dropZoneId"))) - .expectExplLog("The question is invalid, because it has an invalid answer."), - - itemUnrecognisedFormatCase.get().setTitle("answerItemMissingItemId") - .setQuestion(correctChoice(new DndItem(null, "value", "dropZoneId"))), - - itemUnrecognisedFormatCase.get().setTitle("answerItemEmptyItemId") - .setQuestion(correctChoice(new DndItem("", "value", "dropZoneId"))), - - itemUnrecognisedFormatCase.get().setTitle("answerItemMissingDropZoneId") - .setQuestion(correctChoice(new DndItem("item_id", "value", null))), - - itemUnrecognisedFormatCase.get().setTitle("answerItemEmptyDropZoneId") - .setQuestion(correctChoice(new DndItem("item_id", "value", ""))), - - new QuestionValidationTestCase().setTitle("itemsNull") - .tapQuestion(q -> q.setItems(null)) - .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_MISSING_ITEMS), - - new QuestionValidationTestCase().setTitle("itemsEmpty") - .tapQuestion(q -> q.setItems(List.of())) - .expectExplLog(IsaacDndValidator.FEEDBACK_QUESTION_MISSING_ITEMS), - - new QuestionValidationTestCase().setTitle("answerInvalidItemReference") - .setChildren(List.of(new Content("[drop-zone:leg_1]"))) - .setQuestion(correctChoice(new DndItem("invalid_id", "some_value", "leg_1"))) - .expectExplLog("The question is invalid, because it has an answer with unrecognised items."), - - new QuestionValidationTestCase().setTitle("answerDuplicateDropZones") - .setChildren(List.of(new Content("[drop-zone:leg_1] [drop-zone:leg_2]"))) - .setQuestion(correctChoice(item(item_3cm, "leg_1"), item(item_3cm, "leg_1"))) - .expectExplLog("The question is invalid, because it has an answer with duplicate drop zones.") - }; - - @SuppressWarnings("checkstyle:MissingJavadocMethod") - @Theory + @ParameterizedTest + @MethodSource("questionValidationTestCases") public final void testQuestionValidation(final QuestionValidationTestCase testCase) { testCase.question.setDetailedItemFeedback(true); - var response = testValidate(testCase.question, testCase.answer); + DndValidationResponse response = testValidate(testCase.question, testCase.answer); assertFalse(response.isCorrect()); assertEquals(testCase.feedback.getValue(), response.getExplanation().getValue()); assertEquals(testCase.dropZonesCorrect, response.getDropZonesCorrect()); - var appender = testValidateWithLogs(testCase.question, testCase.answer); + TestAppender appender = testValidateWithLogs(testCase.question, testCase.answer); appender.assertLevel(Level.ERROR); appender.assertMessage(testCase.loggedMessage); } - private static DndValidationResponse testValidate(final IsaacDndQuestion question, final Choice choice) { return new IsaacDndValidator().validateQuestionResponse(question, choice); } private static TestAppender testValidateWithLogs(final IsaacDndQuestion question, final Choice choice) { - var appender = new TestAppender(); + TestAppender appender = new TestAppender(); Logger logger = (Logger) LogManager.getLogger(IsaacDndValidator.class); logger.addAppender(appender); logger.setLevel(Level.WARN); @@ -435,56 +397,50 @@ private static TestAppender testValidateWithLogs(final IsaacDndQuestion question testValidate(question, choice); return appender; } finally { - logger.removeAppender(new TestAppender()); + logger.removeAppender(appender); } } - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static DndChoice choice(final DndItem... list) { - var c = new DndChoice(); + DndChoice c = new DndChoice(); c.setItems(List.of(list)); c.setType("dndChoice"); return c; } - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static DndChoice correctChoice(final DndItem... list) { - var choice = choice(list); + DndChoice choice = choice(list); choice.setCorrect(true); return choice; } - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static DndChoice correctChoice(final ContentBase explanation, final DndItem... list) { - var choice = correctChoice(list); + DndChoice choice = correctChoice(list); choice.setExplanation(explanation); return choice; } private static DndChoice incorrectChoice(final DndItem... list) { - var choice = choice(list); + DndChoice choice = choice(list); choice.setCorrect(false); return choice; } - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static DndChoice incorrectChoice(final ContentBase explanation, final DndItem... list) { - var choice = incorrectChoice(list); + DndChoice choice = incorrectChoice(list); choice.setExplanation(explanation); return choice; } private static ParsonsChoice parsonsChoice() { - var parsonsChoice = new ParsonsChoice(); + ParsonsChoice parsonsChoice = new ParsonsChoice(); parsonsChoice.setCorrect(true); parsonsChoice.setItems(List.of(new Item("", ""))); return parsonsChoice; } - - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static DndItem item(final Item item, final String dropZoneId) { - var value = new DndItem(item.getId(), item.getValue(), dropZoneId); + DndItem value = new DndItem(item.getId(), item.getValue(), dropZoneId); value.setType("dndItem"); return value; } @@ -495,9 +451,8 @@ private static Item item(final String id, final String value) { return item; } - @SuppressWarnings("checkstyle:MissingJavadocMethod") public static IsaacDndQuestion question(final DndChoice... answers) { - var question = new IsaacDndQuestion(); + IsaacDndQuestion question = new IsaacDndQuestion(); question.setId(UUID.randomUUID().toString()); question.setItems(List.of(item_3cm, item_4cm, item_5cm, item_6cm, item_12cm, item_13cm)); question.setChoices(List.of(answers)); @@ -554,44 +509,44 @@ public T setQuestion(final DndChoice... choices) { this.question = question(choices); return self(); } - + public T setQuestion(final Consumer op) { - var question = question(); + IsaacDndQuestion question = question(); op.accept(question); this.question = question; return self(); } - + public T setChildren(final List content) { this.questionOps.add(q -> q.setChildren(content)); return self(); } - + public T tapQuestion(final Consumer op) { this.questionOps.add(op); return self(); } - + public T setAnswer(final DndChoice answer) { this.answer = answer; return self(); } - + public T expectCorrect(final boolean correct) { this.correct = correct; return self(); } - + public T expectExplanation(final String feedback) { this.feedback = new Content(feedback); return self(); } - + public T expectNoExplanation() { this.feedback = null; return self(); } - + public T expectExplLog(final String feedback) { this.feedback = new Content(feedback); this.logMessageOp = q -> { @@ -663,7 +618,7 @@ public static class ExplanationTestCase extends TestCase {} public static class DropZonesTestCase extends TestCase {} - public static class DndItemEx extends DndItem { + private static class DndItemEx extends DndItem { public DndItemEx(final String id, final String value, final String dropZoneId) { super(id, value, dropZoneId); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacGraphSketcherValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacGraphSketcherValidatorTest.java index d1f1aa33c8..c0eab1e844 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacGraphSketcherValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacGraphSketcherValidatorTest.java @@ -16,9 +16,8 @@ package uk.ac.cam.cl.dtg.isaac.quiz; import com.google.api.client.util.Lists; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacGraphSketcherQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.content.Choice; @@ -26,14 +25,13 @@ import java.util.List; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the Graph Sketcher Validator class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class IsaacGraphSketcherValidatorTest { private IsaacGraphSketcherValidator validator; @@ -41,7 +39,7 @@ public class IsaacGraphSketcherValidatorTest { * Initial configuration of tests. * */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacGraphSketcherValidator(); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacItemQuestionValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacItemQuestionValidatorTest.java index e01c3b4d58..19f7969ce5 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacItemQuestionValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacItemQuestionValidatorTest.java @@ -17,11 +17,8 @@ import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacItemQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; @@ -32,10 +29,11 @@ import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the Item Question Validator class. @@ -46,13 +44,10 @@ public class IsaacItemQuestionValidatorTest { private IsaacItemQuestion someItemQuestion; private String incorrectExplanation = "EXPLANATION"; - @Rule - public ExpectedException expectedException = ExpectedException.none(); - /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacItemQuestionValidator(); @@ -495,11 +490,11 @@ public final void isaacItemQuestionValidator_WrongQuestionType_ExceptionShouldBe IsaacQuickQuestion invalidQuestionType = new IsaacQuickQuestion(); invalidQuestionType.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("only works with IsaacItemQuestions"); - // This should throw an exception: - validator.validateQuestionResponse(invalidQuestionType, new ItemChoice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(invalidQuestionType, new ItemChoice()); + }); + assertTrue(exception.getMessage().contains("only works with IsaacItemQuestions")); } /* @@ -510,10 +505,10 @@ public final void isaacItemQuestionValidator_WrongChoiceType_ExceptionShouldBeTh IsaacItemQuestion itemQuestion = new IsaacItemQuestion(); itemQuestion.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Expected ItemChoice for IsaacItemQuestion"); - // This should throw an exception: - validator.validateQuestionResponse(itemQuestion, new Choice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(itemQuestion, new Choice()); + }); + assertTrue(exception.getMessage().contains("Expected ItemChoice for IsaacItemQuestion")); } } \ No newline at end of file diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacLLMFreeTextValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacLLMFreeTextValidatorTest.java index 45eb51714e..d2bd88b111 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacLLMFreeTextValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacLLMFreeTextValidatorTest.java @@ -20,63 +20,81 @@ import com.azure.ai.openai.models.ChatCompletions; import com.azure.ai.openai.models.ChatCompletionsOptions; import com.azure.ai.openai.models.ChatResponseMessage; -import org.easymock.EasyMock; import org.json.JSONObject; -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; import org.junit.jupiter.api.DisplayName; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; -import org.powermock.modules.junit4.PowerMockRunnerDelegate; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import uk.ac.cam.cl.dtg.isaac.dos.IsaacLLMFreeTextQuestion; import uk.ac.cam.cl.dtg.isaac.dos.LLMFreeTextQuestionValidationResponse; -import uk.ac.cam.cl.dtg.isaac.dos.content.*; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMFreeTextChoice; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMFreeTextMarkSchemeEntry; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMFreeTextMarkedExample; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMMarkingExpression; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMMarkingFunction; +import uk.ac.cam.cl.dtg.isaac.dos.content.LLMMarkingVariable; +import uk.ac.cam.cl.dtg.isaac.dos.content.Question; import uk.ac.cam.cl.dtg.util.YamlLoader; import java.io.IOException; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.easymock.EasyMock.*; -import static org.junit.Assert.*; -import static org.powermock.api.easymock.PowerMock.createMock; -import static org.powermock.api.easymock.PowerMock.replayAll; -import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.*; -import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.*; -import static uk.ac.cam.cl.dtg.segue.api.Constants.LLM_MARKER_MAX_ANSWER_LENGTH; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.advantageExamples; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.answer; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.client; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.createLLMFreeTextQuestion; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.formulaFn; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.formulaVar; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.genericExamples; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.pointExplanationExamples; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.propertiesForTest; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.question; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Factories.toMarkScheme; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.CORRECT; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.INCORRECT; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.NO_MARKS; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.ONE_MARK; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.TWO_MARKS; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.expectMark; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.getIntTestProperty; +import static uk.ac.cam.cl.dtg.isaac.quiz.IsaacLLMFreeTextValidatorTest.Helpers.validate; +import static uk.ac.cam.cl.dtg.segue.api.Constants.*; -@RunWith(Enclosed.class) public class IsaacLLMFreeTextValidatorTest { - @RunWith(PowerMockRunner.class) - @PowerMockRunnerDelegate(Parameterized.class) - @PrepareForTest({OpenAIClient.class, ChatCompletions.class, ChatChoice.class, ChatResponseMessage.class}) + + @Nested @DisplayName("Test that a mark is awarded based on the marking formula") - public static class TestFormulaBasedMarking { - @Parameter() - public String description; - @Parameter(1) - public IsaacLLMFreeTextQuestion question; - @Parameter(2) - public String breakdown; - @Parameter(3) - public boolean expectedResult; - @Parameter(4) - public int expectedMarkTotal; - - @Parameters(name = "{index}: {0}") - public static List data() { - return Stream.of(genericOneMarkCases(), genericTwoMarkCases(), advantageCases(), pointExplanationCases()) - .flatMap(Arrays::stream).collect(Collectors.toList()); + class TestFormulaBasedMarking { + + static Stream data() { + return Stream.of( + genericOneMarkCases(), + genericTwoMarkCases(), + advantageCases(), + pointExplanationCases() + ).flatMap(s -> s); } - @Test - public void test() throws Exception { - var resp = validate(question, answer(), client(breakdown)); + @ParameterizedTest(name = "{index}: {0}") + @MethodSource("data") + void test(final String description, final IsaacLLMFreeTextQuestion question, final String breakdown, + final boolean expectedResult, final int expectedMarkTotal) throws Exception { + LLMFreeTextQuestionValidationResponse resp = validate(question, answer(), client(breakdown)); expectMark(resp, expectedResult, expectedMarkTotal, toMarkScheme(breakdown)); } @@ -85,28 +103,30 @@ public void test() throws Exception { This mark scheme is used when no other valid mark scheme is specified for the question. It is the total number of marks received, capped by the required maxMarks. */ - private static Object[][] genericOneMarkCases() { - var question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", + private static Stream genericOneMarkCases() { + IsaacLLMFreeTextQuestion question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", 1, genericExamples(), null); - return new Object[][]{ - {"A one-mark answer for a default marking formula one-mark question gets recognised as correct", - question, "{\"reasonFoo\": 1, \"reasonBar\": 0, \"reasonFizz\": 0}", CORRECT, ONE_MARK}, - {"A three-mark answer for a default marking formula one-mark question gets recognised as correct", - question, "{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 0}", CORRECT, ONE_MARK}, - {"A zero-mark answer for a one-mark question gets recognised as incorrect", - question, "{\"reasonFoo\": 0, \"reasonBar\": 0, \"reasonFizz\": 0}", INCORRECT, NO_MARKS}}; + return Stream.of( + Arguments.of("A one-mark answer for a default marking formula one-mark question gets recognised as correct", + question, "{\"reasonFoo\": 1, \"reasonBar\": 0, \"reasonFizz\": 0}", CORRECT, ONE_MARK), + Arguments.of("A three-mark answer for a default marking formula one-mark question gets recognised as correct", + question, "{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 0}", CORRECT, ONE_MARK), + Arguments.of("A zero-mark answer for a one-mark question gets recognised as incorrect", + question, "{\"reasonFoo\": 0, \"reasonBar\": 0, \"reasonFizz\": 0}", INCORRECT, NO_MARKS) + ); } - private static Object[][] genericTwoMarkCases() { - var question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", + private static Stream genericTwoMarkCases() { + IsaacLLMFreeTextQuestion question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", 2, genericExamples(), null); - return new Object[][]{ - {"A two-mark answer for a default marking formula two-mark question gets recognised as correct", - question, "{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 0}", CORRECT, TWO_MARKS}, - {"A one-mark answer for a default marking formula two-mark question receives exactly one mark", - question, "{\"reasonFoo\": 1, \"reasonBar\": 0, \"reasonFizz\": 0}", CORRECT, ONE_MARK}}; + return Stream.of( + Arguments.of("A two-mark answer for a default marking formula two-mark question gets recognised as correct", + question, "{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 0}", CORRECT, TWO_MARKS), + Arguments.of("A one-mark answer for a default marking formula two-mark question receives exactly one mark", + question, "{\"reasonFoo\": 1, \"reasonBar\": 0, \"reasonFizz\": 0}", CORRECT, ONE_MARK) + ); } /* @@ -116,20 +136,21 @@ private static Object[][] genericTwoMarkCases() { This is a generally applicable mark scheme for any two mutually exclusive categories of marks. A common example of this is advantages and disadvantages. */ - private static Object[][] advantageCases() { - var formula = formulaFn("SUM", + private static Stream advantageCases() { + LLMMarkingFunction formula = formulaFn("SUM", formulaFn("MAX", formulaVar("adv1"), formulaVar("adv2")), formulaFn("MAX", formulaVar("dis1"), formulaVar("dis2"))); - var question = createLLMFreeTextQuestion("{\"adv1\": 1, \"adv2\": 1, \"dis1\": 1, \"dis2\": 1}", + IsaacLLMFreeTextQuestion question = createLLMFreeTextQuestion("{\"adv1\": 1, \"adv2\": 1, \"dis1\": 1, \"dis2\": 1}", 2, advantageExamples(), formula); - return new Object[][]{ - {"An answer containing an advantage and a disadvantage mark receives two marks", - question, "{\"adv1\": 1, \"adv2\": 0, \"dis1\": 1, \"dis2\": 0}", CORRECT, TWO_MARKS}, - {"An answer containing only a disadvantage mark receives one mark", - question, "{\"adv1\": 0, \"adv2\": 0, \"dis1\": 1, \"dis2\": 0}", CORRECT, ONE_MARK}, - {"An answer containing two advantage marks receives one mark", - question, "{\"adv1\": 1, \"adv2\": 1, \"dis1\": 0, \"dis2\": 0}", CORRECT, ONE_MARK}}; + return Stream.of( + Arguments.of("An answer containing an advantage and a disadvantage mark receives two marks", + question, "{\"adv1\": 1, \"adv2\": 0, \"dis1\": 1, \"dis2\": 0}", CORRECT, TWO_MARKS), + Arguments.of("An answer containing only a disadvantage mark receives one mark", + question, "{\"adv1\": 0, \"adv2\": 0, \"dis1\": 1, \"dis2\": 0}", CORRECT, ONE_MARK), + Arguments.of("An answer containing two advantage marks receives one mark", + question, "{\"adv1\": 1, \"adv2\": 1, \"dis1\": 0, \"dis2\": 0}", CORRECT, ONE_MARK) + ); } @@ -141,60 +162,59 @@ private static Object[][] advantageCases() { This is a generally applicable mark scheme for any grouped sets of marks where a prerequisite mark is required to receive a secondary mark. A common example of this is points and explanations. */ - private static Object[][] pointExplanationCases() { - var formula = formulaFn("SUM", + private static Stream pointExplanationCases() { + LLMMarkingFunction formula = formulaFn("SUM", formulaFn("MAX", formulaVar("pnt1"), formulaVar("pnt2")), formulaFn("MAX", formulaFn("MIN", formulaVar("pnt1"), formulaVar("expl1")), formulaFn("MIN", formulaVar("pnt2"), formulaVar("expl2")))); - var question = createLLMFreeTextQuestion("{\"pnt1\": 1, \"pnt2\": 1, \"expl1\": 1, \"expl2\": 1}", + IsaacLLMFreeTextQuestion question = createLLMFreeTextQuestion("{\"pnt1\": 1, \"pnt2\": 1, \"expl1\": 1, \"expl2\": 1}", 2, pointExplanationExamples(), formula); - return new Object[][]{ - {"An answer containing a point and matching explanation receives two marks", - question, "{\"pnt1\": 1, \"pnt2\": 0, \"expl1\": 1, \"expl2\": 0}", CORRECT, TWO_MARKS}, - {"An answer containing an explanation without a matching point receives zero marks", - question, "{\"pnt1\": 0, \"pnt2\": 0, \"expl1\": 1, \"expl2\": 0}", INCORRECT, NO_MARKS}, - {"An answer containing a point and a mismatched explanation receives one mark", - question, "{\"pnt1\": 1, \"pnt2\": 0, \"expl1\": 0, \"expl2\": 0}", CORRECT, ONE_MARK}}; + return Stream.of( + Arguments.of("An answer containing a point and matching explanation receives two marks", + question, "{\"pnt1\": 1, \"pnt2\": 0, \"expl1\": 1, \"expl2\": 0}", CORRECT, TWO_MARKS), + Arguments.of("An answer containing an explanation without a matching point receives zero marks", + question, "{\"pnt1\": 0, \"pnt2\": 0, \"expl1\": 1, \"expl2\": 0}", INCORRECT, NO_MARKS), + Arguments.of("An answer containing a point and a mismatched explanation receives one mark", + question, "{\"pnt1\": 1, \"pnt2\": 0, \"expl1\": 0, \"expl2\": 0}", CORRECT, ONE_MARK) + ); } } - @RunWith(PowerMockRunner.class) - @PrepareForTest({OpenAIClient.class, ChatCompletions.class, ChatChoice.class, ChatResponseMessage.class}) + @Nested @DisplayName("Test application behaviour in case of errors.") - public static class TestErrorHandling { + class TestErrorHandling { @Test @DisplayName("A response from the client not in the expected json format returns zero marks") - public void isaacLLMFreeTextValidator_ResponseInvalidFormat_MarkSchemeShouldIncludeNoMarks() throws Exception { - var question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", + void isaacLLMFreeTextValidator_ResponseInvalidFormat_MarkSchemeShouldIncludeNoMarks() throws Exception { + IsaacLLMFreeTextQuestion question = createLLMFreeTextQuestion("{\"reasonFoo\": 1, \"reasonBar\": 1, \"reasonFizz\": 1}", 1, genericExamples(), null); - var response = validate(question, answer(), client("Not a valid JSON response")); + LLMFreeTextQuestionValidationResponse response = validate(question, answer(), client("Not a valid JSON response")); expectMark(response, INCORRECT, 0, toMarkScheme("{\"reasonFoo\": 0, \"reasonBar\": 0, \"reasonFizz\": 0}")); } @Test @DisplayName("An answer exceeding the maximum answer length is handled with an exception") - public void isaacLLMFreeTextValidator_AnswerOverLengthLimit_ExceptionShouldBeThrown() throws Exception { - var maxAnswerLength = getIntTestProperty(LLM_MARKER_MAX_ANSWER_LENGTH, 4096); - var choice = new LLMFreeTextChoice(); + void isaacLLMFreeTextValidator_AnswerOverLengthLimit_ExceptionShouldBeThrown() throws Exception { + int maxAnswerLength = getIntTestProperty(LLM_MARKER_MAX_ANSWER_LENGTH, 4096); + LLMFreeTextChoice choice = new LLMFreeTextChoice(); choice.setValue(String.join("", Collections.nCopies((maxAnswerLength / 10 + 1), "Repeat Me "))); - var exception = assertThrows(IllegalArgumentException.class, () -> validate(question(), choice, client())); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> validate(question(), choice, client())); assertEquals("Answer is too long for LLM free-text question marking", exception.getMessage()); } @Test @DisplayName("Error from the client (e.g. timeout, rate limit, out of credits) is handled with an exception") - public void isaacLLMFreeTextValidator_ResponseError_ExceptionShouldBeThrown() { - var client = createMock(OpenAIClient.class); - EasyMock.expect(client.getChatCompletions(anyString(), isA(ChatCompletionsOptions.class))) - .andThrow(new RuntimeException("Test OpenAI Exception")); - replay(client); + void isaacLLMFreeTextValidator_ResponseError_ExceptionShouldBeThrown() { + OpenAIClient client = mock(OpenAIClient.class); + when(client.getChatCompletions(anyString(), any(ChatCompletionsOptions.class))) + .thenThrow(new RuntimeException("Test OpenAI Exception")); - var exception = assertThrows(ValidatorUnavailableException.class, () -> validate(question(), answer(), client)); + ValidatorUnavailableException exception = assertThrows(ValidatorUnavailableException.class, () -> validate(question(), answer(), client)); assertEquals("We are having problems marking LLM marked questions. Please try again later!", exception.getMessage()); @@ -202,17 +222,17 @@ public void isaacLLMFreeTextValidator_ResponseError_ExceptionShouldBeThrown() { @Test @DisplayName("Invalid question (missing maxMarks fields) is handled with an exception") - public void isaacLLMFreeTextValidator_MissingMaxMarks_ExceptionShouldBeThrown() { - var badFields = createLLMFreeTextQuestion(null, null, null, null); - var exception = assertThrows(IllegalArgumentException.class, () -> validate(badFields, answer(), client())); + void isaacLLMFreeTextValidator_MissingMaxMarks_ExceptionShouldBeThrown() { + IsaacLLMFreeTextQuestion badFields = createLLMFreeTextQuestion(null, null, null, null); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> validate(badFields, answer(), client())); assertEquals("This question cannot be answered correctly", exception.getMessage()); } @Test @DisplayName("Invalid question (not LLMFreeTextQuestion) is handled with an exception") - public void isaacLLMFreeTextValidator_NotLLMFreeTextQuestion_ExceptionShouldBeThrown() { - var badType = new Question(); - var exception = assertThrows(IllegalArgumentException.class, () -> validate(badType, answer(), client())); + void isaacLLMFreeTextValidator_NotLLMFreeTextQuestion_ExceptionShouldBeThrown() { + Question badType = new Question(); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> validate(badType, answer(), client())); assertEquals(badType.getId() + " is not a LLM free-text question", exception.getMessage()); } } @@ -234,10 +254,9 @@ protected static class Helpers { * @param client - use the mocked client instance to specify the mark breakdown the API should return * @return the response from the validator, examine this object in "assert". */ - public static LLMFreeTextQuestionValidationResponse validate(Question question, - LLMFreeTextChoice answer, + public static LLMFreeTextQuestionValidationResponse validate(Question question, LLMFreeTextChoice answer, OpenAIClient client) throws Exception { - var validator = new IsaacLLMFreeTextValidator(propertiesForTest(), client); + IsaacLLMFreeTextValidator validator = new IsaacLLMFreeTextValidator(propertiesForTest(), client); return (LLMFreeTextQuestionValidationResponse) validator.validateQuestionResponse(question, answer); } @@ -252,10 +271,8 @@ public static LLMFreeTextQuestionValidationResponse validate(Question question, * ONE_MARK, TWO_MARKS. * @param expectedMarks - double check that the breakdown is returned in the expected format */ - public static void expectMark(LLMFreeTextQuestionValidationResponse response, - boolean isCorrect, - int marks, - List expectedMarks) { + public static void expectMark(LLMFreeTextQuestionValidationResponse response, boolean isCorrect, + int marks, List expectedMarks) { assertEquals(isCorrect, response.isCorrect()); assertEquals(marks, (long) response.getMarks()); assertTrue(expectedMarks.containsAll(response.getMarkBreakdown())); @@ -309,19 +326,17 @@ public static OpenAIClient client() { } public static OpenAIClient client(final String llmResponse) { - // These must be PowerMocked since the classes are final in the Azure OpenAI library - var client = createMock(OpenAIClient.class); - var chatCompletions = createMock(ChatCompletions.class); - var chatChoice = createMock(ChatChoice.class); - var chatResponseMessage = createMock(ChatResponseMessage.class); - - EasyMock.expect(chatResponseMessage.getContent()).andReturn(llmResponse); - EasyMock.expect(chatChoice.getMessage()).andReturn(chatResponseMessage); - EasyMock.expect(chatCompletions.getChoices()).andReturn(Collections.singletonList(chatChoice)).times(2); - EasyMock.expect(client.getChatCompletions(anyString(), isA(ChatCompletionsOptions.class))) - .andReturn(chatCompletions); - - replayAll(); + OpenAIClient client = mock(OpenAIClient.class); + ChatCompletions chatCompletions = mock(ChatCompletions.class); + ChatChoice chatChoice = mock(ChatChoice.class); + ChatResponseMessage chatResponseMessage = mock(ChatResponseMessage.class); + + when(chatResponseMessage.getContent()).thenReturn(llmResponse); + when(chatChoice.getMessage()).thenReturn(chatResponseMessage); + when(chatCompletions.getChoices()).thenReturn(Collections.singletonList(chatChoice)); + when(client.getChatCompletions(anyString(), any(ChatCompletionsOptions.class))) + .thenReturn(chatCompletions); + return client; } @@ -376,9 +391,9 @@ public static YamlLoader propertiesForTest() throws IOException { } public static List toMarkScheme(String json) { - var parsed = new JSONObject(json); + JSONObject parsed = new JSONObject(json); return parsed.keySet().stream().map(key -> { - var output = new LLMFreeTextMarkSchemeEntry(); + LLMFreeTextMarkSchemeEntry output = new LLMFreeTextMarkSchemeEntry(); output.setJsonField(key); output.setMarks(parsed.getInt(key)); output.setShortDescription("Some description that does not matter for the test."); @@ -388,12 +403,12 @@ public static List toMarkScheme(String json) { private static List generateMarkedExamples(JSONObject... jsonMarkedExamples) { return Stream.of(jsonMarkedExamples).map(input -> { - var output = new LLMFreeTextMarkedExample(); + LLMFreeTextMarkedExample output = new LLMFreeTextMarkedExample(); output.setAnswer(input.getString("answer")); output.setMarksAwarded(input.getInt("marksAwarded")); - var jsonMarkedExampleMarks = input.getJSONObject("marks"); - var marks = jsonMarkedExampleMarks.keySet().stream() + JSONObject jsonMarkedExampleMarks = input.getJSONObject("marks"); + Map marks = jsonMarkedExampleMarks.keySet().stream() .collect(Collectors.toMap(mark -> mark, jsonMarkedExampleMarks::getInt)); output.setMarks(marks); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacNumericValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacNumericValidatorTest.java index bc8fd79628..2d10f12b20 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacNumericValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacNumericValidatorTest.java @@ -16,12 +16,8 @@ package uk.ac.cam.cl.dtg.isaac.quiz; import com.google.api.client.util.Lists; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.reflect.Whitebox; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.slf4j.Logger; import uk.ac.cam.cl.dtg.isaac.dos.IsaacNumericQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; @@ -36,17 +32,20 @@ import java.util.LinkedList; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.easymock.EasyMock.createMock; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the user manager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class IsaacNumericValidatorTest { + + private Logger log; private IsaacNumericValidator validator; private IsaacNumericQuestion numericQuestionNoUnits; private IsaacNumericQuestion numericQuestionWithUnits; @@ -55,15 +54,13 @@ public class IsaacNumericValidatorTest { private String correctIntegerAnswer = "42"; private String correctUnits = "m\\,s^{-1}"; - @Rule - public ExpectedException expectedException = ExpectedException.none(); - /** * Initial configuration of tests. * */ - @Before + @BeforeEach public final void setUp() { + log = createMock(Logger.class); validator = new IsaacNumericValidator(); // Set up a question object which does not require units: @@ -400,14 +397,14 @@ public final void isaacNumericValidator_TestQuestionWithSigFigRange_IncorrectSig Quantity q_5sf = new Quantity("1.6875"); // Test response: QuestionValidationResponse response_5sf = validator.validateQuestionResponse(someNumericQuestion, q_5sf); - assertFalse("expected 1.6875 not to match 1.6875 to 2 or 3 sf", response_5sf.isCorrect()); + assertFalse(response_5sf.isCorrect(), "expected 1.6875 not to match 1.6875 to 2 or 3 sf"); assertTrue(response_5sf.getExplanation().getTags().contains("sig_figs")); // Set up a user answer: Quantity q_1sf = new Quantity("2"); // Test response: QuestionValidationResponse response_1sf = validator.validateQuestionResponse(someNumericQuestion, q_1sf); - assertFalse("expected 2 not to match 1.6875 to 2 or 3 sf", response_1sf.isCorrect()); + assertFalse(response_1sf.isCorrect(), "expected 2 not to match 1.6875 to 2 or 3 sf"); assertTrue(response_1sf.getExplanation().getTags().contains("sig_figs")); } @@ -433,7 +430,7 @@ public final void isaacNumericValidator_TestQuestionWithSigFigRange_SigFigRespon Quantity q_5sf_corr = new Quantity("1.6875"); // Test response is sig fig message: QuestionValidationResponse response_5sf_corr = validator.validateQuestionResponse(someNumericQuestion, q_5sf_corr); - assertFalse("expected 1.6875 not to match 1.6875 to 2 or 3 sf", response_5sf_corr.isCorrect()); + assertFalse(response_5sf_corr.isCorrect(), "expected 1.6875 not to match 1.6875 to 2 or 3 sf"); assertTrue(response_5sf_corr.getExplanation().getTags().contains("sig_figs")); assertTrue(response_5sf_corr.getExplanation().getTags().contains("sig_figs_too_many")); @@ -441,14 +438,14 @@ public final void isaacNumericValidator_TestQuestionWithSigFigRange_SigFigRespon Quantity q_5sf_wrong = new Quantity("2.7986"); // Test response does not mention sig figs: QuestionValidationResponse response_5sf_wrong = validator.validateQuestionResponse(someNumericQuestion, q_5sf_wrong); - assertFalse("expected 2.7986 not to match 1.6875", response_5sf_wrong.isCorrect()); - assertFalse("expected 2.7986 without sig fig message", response_5sf_wrong.getExplanation().getTags().contains("sig_figs")); + assertFalse(response_5sf_wrong.isCorrect(), "expected 2.7986 not to match 1.6875"); + assertFalse(response_5sf_wrong.getExplanation().getTags().contains("sig_figs"), "expected 2.7986 without sig fig message"); // Set up a user answer: Quantity q_1sf = new Quantity("5"); // Test response: QuestionValidationResponse response_1sf = validator.validateQuestionResponse(someNumericQuestion, q_1sf); - assertFalse("expected 5 not to match 1.6875 to 2 or 3 sf", response_1sf.isCorrect()); + assertFalse(response_1sf.isCorrect(), "expected 5 not to match 1.6875 to 2 or 3 sf"); assertTrue(response_1sf.getExplanation().getTags().contains("sig_figs")); assertTrue(response_1sf.getExplanation().getTags().contains("sig_figs_too_few")); } @@ -622,11 +619,11 @@ public final void isaacNumericValidator_WrongQuestionType_ExceptionShouldBeThrow IsaacQuickQuestion invalidQuestionType = new IsaacQuickQuestion(); invalidQuestionType.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("only works with Isaac Numeric Questions"); - // This should throw an exception: - validator.validateQuestionResponse(invalidQuestionType, new Quantity()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(invalidQuestionType, new Quantity()); + }); + assertTrue(exception.getMessage().contains("only works with Isaac Numeric Questions")); } /* @@ -637,11 +634,11 @@ public final void isaacNumericValidator_WrongChoiceType_ExceptionShouldBeThrown( IsaacNumericQuestion numericQuestion = new IsaacNumericQuestion(); numericQuestion.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Expected Quantity for IsaacNumericQuestion"); - // This should throw an exception: - validator.validateQuestionResponse(numericQuestion, new Choice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(numericQuestion, new Choice()); + }); + assertTrue(exception.getMessage().contains("Expected Quantity for IsaacNumericQuestion")); } /* @@ -683,14 +680,14 @@ public final void isaacNumericValidator_CheckParsingAsNumberWorks() throws Excep List numbersToTest = Arrays.asList("42", "4.2e1", "4.2E1", "4.2x10^1", "4.2*10**1", "4.2×10^(1)", "4.2 \\times 10^{1}"); for (String numberToTest : numbersToTest) { - boolean result = ValidationUtils.numericValuesMatch(numberToMatch, numberToTest, 2, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); + boolean result = ValidationUtils.numericValuesMatch(numberToMatch, numberToTest, 2, log); assertTrue(result); } String powerOfTenToMatch = "10000"; List powersOfTenToTest = Arrays.asList("10000", "1x10^4", "1e4", "1E4", "1 x 10**4", "10^4", "10**(4)", "10^{4}", "100x10^2"); for (String powerOfTenToTest : powersOfTenToTest) { - boolean result = ValidationUtils.numericValuesMatch(powerOfTenToMatch, powerOfTenToTest, 1, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); + boolean result = ValidationUtils.numericValuesMatch(powerOfTenToMatch, powerOfTenToTest, 1, log); assertTrue(result); } } @@ -987,20 +984,22 @@ public final void isaacNumericValidator_correctAnswerWithDisregardSigFigs_respon // ---------- Helper methods to test internal functionality of the validator class ---------- private void testSigFigRoundingWorks(String inputValue, int sigFigToRoundTo, double expectedResult) throws Exception { - double result = ValidationUtils.roundStringValueToSigFigs(inputValue, sigFigToRoundTo, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); + double result = ValidationUtils.roundStringValueToSigFigs(inputValue, sigFigToRoundTo, log); - assertEquals("sigfig rounding failed for value '" + inputValue + "' to " + sigFigToRoundTo - + "sf: expected '" + expectedResult + "', got '" + result + "'", result, expectedResult, 0.0); + assertEquals(result, expectedResult, 0.0, "sigfig rounding failed for value '" + inputValue + "' to " + + sigFigToRoundTo + "sf: expected '" + expectedResult + "', got '" + result + "'"); } private void testSigFigExtractionWorks(String inputValue, int minAllowedSigFigs, int maxAllowedSigFigs, int expectedResult) throws Exception { - int result = ValidationUtils.numberOfSignificantFiguresToValidateWith(inputValue, minAllowedSigFigs, maxAllowedSigFigs, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); + int result = ValidationUtils.numberOfSignificantFiguresToValidateWith(inputValue, minAllowedSigFigs, maxAllowedSigFigs, log); + + assertTrue(result <= maxAllowedSigFigs && result >= minAllowedSigFigs, + "sigfig extraction out of range for value " + inputValue + " (min allowed: " + minAllowedSigFigs + + ", max allowed: " + maxAllowedSigFigs + ") got " + result); - assertTrue("sigfig extraction out of range for value " + inputValue + " (min allowed: " + minAllowedSigFigs - + ", max allowed: " + maxAllowedSigFigs + ") got " + result, result <= maxAllowedSigFigs && result >= minAllowedSigFigs); - assertEquals("sigfig extraction failed for value " + inputValue + ", expected: " + expectedResult - + " got " + result, result, expectedResult); + assertEquals(result, expectedResult, "sigfig extraction failed for value " + inputValue + ", expected: " + + expectedResult + " got " + result); } @@ -1016,17 +1015,17 @@ private void verifyCorrectNumberOfSigFigsNoRange(List numbersToTest, Lis for (String number : numbersToTest) { for (Integer sigFig : sigFigsToPass) { - boolean tooFew = ValidationUtils.tooFewSignificantFigures(number, sigFig, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); - assertFalse("Unexpected too few sig fig for " + number + " @ " + sigFig + "sf", tooFew); - boolean tooMany = ValidationUtils.tooManySignificantFigures(number, sigFig, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); - assertFalse("Unexpected too many sig fig for " + number + " @ " + sigFig + "sf", tooMany); + boolean tooFew = ValidationUtils.tooFewSignificantFigures(number, sigFig, log); + assertFalse(tooFew, "Unexpected too few sig fig for " + number + " @ " + sigFig + "sf"); + boolean tooMany = ValidationUtils.tooManySignificantFigures(number, sigFig, log); + assertFalse(tooMany, "Unexpected too many sig fig for " + number + " @ " + sigFig + "sf"); } for (Integer sigFig : sigFigsToFail) { - boolean tooFew = ValidationUtils.tooFewSignificantFigures(number, sigFig, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); - boolean tooMany = ValidationUtils.tooManySignificantFigures(number, sigFig, (Logger) Whitebox.getField(validator.getClass(), "log").get(validator)); + boolean tooFew = ValidationUtils.tooFewSignificantFigures(number, sigFig, log); + boolean tooMany = ValidationUtils.tooManySignificantFigures(number, sigFig, log); boolean incorrectSigFig = tooMany || tooFew; - assertTrue("Expected incorrect sig fig for " + number + " @ " + sigFig + "sf", incorrectSigFig); + assertTrue(incorrectSigFig, "Expected incorrect sig fig for " + number + " @ " + sigFig + "sf"); } } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacParsonsValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacParsonsValidatorTest.java index e8e7f05d3a..f6124c5bd1 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacParsonsValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacParsonsValidatorTest.java @@ -17,11 +17,8 @@ import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacParsonsQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; @@ -33,28 +30,25 @@ import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the Parsons Question Validator class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class IsaacParsonsValidatorTest { private IsaacParsonsValidator validator; private IsaacParsonsQuestion someParsonsQuestion; private String incorrectExplanation = "EXPLANATION"; - @Rule - public ExpectedException expectedException = ExpectedException.none(); - /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacParsonsValidator(); @@ -196,11 +190,11 @@ public final void isaacParsonsValidator_IncorrectItemType_ExceptionShouldBeThrow Item submittedItem2 = new Item("id002", null); c.setItems(ImmutableList.of(submittedItem1, submittedItem2)); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Expected ParsonsChoice to contain ParsonsItems"); - // This should throw an exception: - validator.validateQuestionResponse(someParsonsQuestion, c); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(someParsonsQuestion, c); + }); + assertTrue(exception.getMessage().contains("Expected ParsonsChoice to contain ParsonsItems")); } /* @@ -406,11 +400,11 @@ public final void isaacParsonsValidator_WrongQuestionType_ExceptionShouldBeThrow IsaacQuickQuestion invalidQuestionType = new IsaacQuickQuestion(); invalidQuestionType.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("only works with IsaacParsonsQuestions"); - // This should throw an exception: - validator.validateQuestionResponse(invalidQuestionType, new ParsonsChoice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(invalidQuestionType, new ParsonsChoice()); + }); + assertTrue(exception.getMessage().contains("only works with IsaacParsonsQuestions")); } /* @@ -421,10 +415,10 @@ public final void isaacParsonsValidator_WrongChoiceType_ExceptionShouldBeThrown( IsaacParsonsQuestion parsonsQuestion = new IsaacParsonsQuestion(); parsonsQuestion.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Expected ParsonsChoice for IsaacParsonsQuestion"); - // This should throw an exception: - validator.validateQuestionResponse(parsonsQuestion, new Choice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(parsonsQuestion, new Choice()); + }); + assertTrue(exception.getMessage().contains("Expected ParsonsChoice for IsaacParsonsQuestion")); } } \ No newline at end of file diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacReorderValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacReorderValidatorTest.java index 7d2af4e7ed..a562c5ec2d 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacReorderValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacReorderValidatorTest.java @@ -17,8 +17,8 @@ import com.google.api.client.util.Lists; import com.google.common.collect.ImmutableList; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacReorderQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; @@ -32,11 +32,11 @@ import java.util.Collections; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class IsaacReorderValidatorTest { private IsaacReorderValidator validator; @@ -53,7 +53,7 @@ public class IsaacReorderValidatorTest { /** * Initial configuration of tests. */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacReorderValidator(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacStringMatchValidatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacStringMatchValidatorTest.java index 5d8f4629ce..af5ad34b4a 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacStringMatchValidatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/quiz/IsaacStringMatchValidatorTest.java @@ -16,11 +16,8 @@ package uk.ac.cam.cl.dtg.isaac.quiz; import com.google.api.client.util.Lists; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacQuickQuestion; import uk.ac.cam.cl.dtg.isaac.dos.IsaacStringMatchQuestion; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; @@ -30,29 +27,26 @@ import java.util.List; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the String Match Validator class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class IsaacStringMatchValidatorTest { private IsaacStringMatchValidator validator; private IsaacStringMatchQuestion someStringMatchQuestion; private String caseSensitiveAnswer = "CaseSensitiveAnswer"; private String caseInsensitiveAnswer = "CaseInsensitiveAnswer"; - @Rule - public ExpectedException expectedException = ExpectedException.none(); - /** * Initial configuration of tests. * */ - @Before + @BeforeEach public final void setUp() { validator = new IsaacStringMatchValidator(); @@ -473,11 +467,11 @@ public final void isaacStringMatchValidator_WrongQuestionType_ExceptionShouldBeT IsaacQuickQuestion invalidQuestionType = new IsaacQuickQuestion(); invalidQuestionType.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("only works with Isaac String Match Questions"); - // This should throw an exception: - validator.validateQuestionResponse(invalidQuestionType, new StringChoice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(invalidQuestionType, new StringChoice()); + }); + assertTrue(exception.getMessage().contains("only works with Isaac String Match Questions")); } /* @@ -488,10 +482,10 @@ public final void isaacStringMatchValidator_WrongChoiceType_ExceptionShouldBeThr IsaacStringMatchQuestion someStringMatchQuestion = new IsaacStringMatchQuestion(); someStringMatchQuestion.setId("invalidQuestionType"); - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Expected StringChoice for IsaacStringMatchQuestion"); - // This should throw an exception: - validator.validateQuestionResponse(someStringMatchQuestion, new Choice()); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + validator.validateQuestionResponse(someStringMatchQuestion, new Choice()); + }); + assertTrue(exception.getMessage().contains("Expected StringChoice for IsaacStringMatchQuestion")); } -} +} \ No newline at end of file diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/util/ContentValidatorUtilsTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/util/ContentValidatorUtilsTest.java index 076b5b9f2e..084d1c7232 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/util/ContentValidatorUtilsTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/util/ContentValidatorUtilsTest.java @@ -1,9 +1,7 @@ package uk.ac.cam.cl.dtg.isaac.util; -import org.junit.experimental.theories.DataPoints; -import org.junit.experimental.theories.Theories; -import org.junit.experimental.theories.Theory; -import org.junit.runner.RunWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import uk.ac.cam.cl.dtg.isaac.dos.content.Content; import uk.ac.cam.cl.dtg.isaac.dos.content.Figure; import uk.ac.cam.cl.dtg.isaac.dos.content.FigureRegion; @@ -13,105 +11,102 @@ import java.util.LinkedList; import java.util.List; import java.util.function.Supplier; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; -@RunWith(Theories.class) @SuppressWarnings("checkstyle:MissingJavadocType") public class ContentValidatorUtilsTest { - static Supplier invalidDropZone = () -> new GetDropZonesTestCase() - .expectDropZones(); - @DataPoints - public static GetDropZonesTestCase[] getDropZonesTestCases = { - invalidDropZone.get().setChildren(List.of(new Content(""))), - - invalidDropZone.get().setChildren(List.of(new Content("no drop zone"))), - - invalidDropZone.get().setChildren(List.of(new Content("[drop-zone A1]"))), - - invalidDropZone.get().setChildren(List.of(new Content("[drop-zone: A1]"))), - - invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1 | w-100]"))), - - invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|w-100 h-50]"))), - - invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|h-100w-50]"))), - - new GetDropZonesTestCase().setTitle("noContent_noDropZones").setChildren(null).expectDropZones(), - - new GetDropZonesTestCase().setTitle("singleDropZoneSingleText_returnsDropZone") - .setChildren(List.of(new Content("[drop-zone:A1]"))) - .expectDropZones("A1"), - - new GetDropZonesTestCase().setTitle("singleDropZoneSingleContent_returnsDropZone") - .setChildren(List.of(new Content("[drop-zone:A1|w-100]"))) - .expectDropZones("A1"), - - new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentHeight_returnsDropZone") - .setChildren(List.of(new Content("[drop-zone:A1|h-100]"))) - .expectDropZones("A1"), - - new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentWidthHeight_returnsDropZone") - .setChildren(List.of(new Content("[drop-zone:A1|w-100h-50]"))) - .expectDropZones("A1"), - - new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentWithinLatex_returnsDropZone") - .setChildren(List.of(new Content("$$1 + \\text{[drop-zone:A1]}$$"))) - .expectDropZones("A1"), - - new GetDropZonesTestCase().setTitle("multiDropZoneSingleContent_returnsDropZones") - .setChildren(List.of(new Content("Some text [drop-zone:A1], other text [drop-zone:A2]"))) - .expectDropZones("A1", "A2"), - - new GetDropZonesTestCase().setTitle("multiDropZoneMultiContent_returnsDropZones") - .setChildren(List.of( - new Content("[drop-zone:A1] [drop-zone:A2]"), - new Content("[drop-zone:A3] [drop-zone:A4]") - )).expectDropZones("A1", "A2", "A3", "A4"), - - new GetDropZonesTestCase().setTitle("singleDropZoneNestedContent_returnsDropZones") - .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) - .tapQuestion(q -> ((Content) q.getChildren().get(0)).setChildren(List.of(new Content("[drop-zone:A1]")))) - .expectDropZones("A1", "A2"), - - new GetDropZonesTestCase().setTitle("figureContentWithoutDropZones_returnsNoZones") - .setChildren(List.of(new Figure())) - .expectDropZones(), - - new GetDropZonesTestCase().setTitle("figureContent_returnsDropZones") - .setChildren(List.of(createFigure("A1", "A2"))) - .expectDropZones("A1", "A2"), - - new GetDropZonesTestCase().setTitle("mixedButNoNesting_returnsDropZones") - .setChildren(new LinkedList<>(List.of(createFigure("A1", "A2"), new Content("[drop-zone:A3]")))) - .expectDropZones("A1", "A2", "A3"), - - new GetDropZonesTestCase().setTitle("mixedNested_returnsDropZones") - .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) - .tapQuestion(q -> { - Content content = (Content) q.getChildren().get(0); - content.setChildren(List.of( - new Content("[drop-zone:A1]"), - createFigure("F1", "F2") - )); - }).expectDropZones("A1", "F1", "F2", "A2") - }; + private static final Supplier invalidDropZone = () -> + new GetDropZonesTestCase().expectDropZones(); + + static Stream getDropZonesTestCases() { + return Stream.of( + invalidDropZone.get().setChildren(List.of(new Content(""))), + invalidDropZone.get().setChildren(List.of(new Content("no drop zone"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone A1]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone: A1]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1 | w-100]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|w-100 h-50]"))), + invalidDropZone.get().setChildren(List.of(new Content("[drop-zone:A1|h-100w-50]"))), + + new GetDropZonesTestCase().setTitle("noContent_noDropZones").setChildren(null).expectDropZones(), + + new GetDropZonesTestCase().setTitle("singleDropZoneSingleText_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1]"))) + .expectDropZones("A1"), + + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContent_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|w-100]"))) + .expectDropZones("A1"), + + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentHeight_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|h-100]"))) + .expectDropZones("A1"), + + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentWidthHeight_returnsDropZone") + .setChildren(List.of(new Content("[drop-zone:A1|w-100h-50]"))) + .expectDropZones("A1"), + + new GetDropZonesTestCase().setTitle("singleDropZoneSingleContentWithinLatex_returnsDropZone") + .setChildren(List.of(new Content("$$1 + \\text{[drop-zone:A1]}$$"))) + .expectDropZones("A1"), + + new GetDropZonesTestCase().setTitle("multiDropZoneSingleContent_returnsDropZones") + .setChildren(List.of(new Content("Some text [drop-zone:A1], other text [drop-zone:A2]"))) + .expectDropZones("A1", "A2"), + + new GetDropZonesTestCase().setTitle("multiDropZoneMultiContent_returnsDropZones") + .setChildren(List.of( + new Content("[drop-zone:A1] [drop-zone:A2]"), + new Content("[drop-zone:A3] [drop-zone:A4]") + )).expectDropZones("A1", "A2", "A3", "A4"), + + new GetDropZonesTestCase().setTitle("singleDropZoneNestedContent_returnsDropZones") + .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) + .tapQuestion(q -> ((Content) q.getChildren().get(0)).setChildren(List.of(new Content("[drop-zone:A1]")))) + .expectDropZones("A1", "A2"), + + new GetDropZonesTestCase().setTitle("figureContentWithoutDropZones_returnsNoZones") + .setChildren(List.of(new Figure())) + .expectDropZones(), + + new GetDropZonesTestCase().setTitle("figureContent_returnsDropZones") + .setChildren(List.of(createFigure("A1", "A2"))) + .expectDropZones("A1", "A2"), + + new GetDropZonesTestCase().setTitle("mixedButNoNesting_returnsDropZones") + .setChildren(new LinkedList<>(List.of(createFigure("A1", "A2"), new Content("[drop-zone:A3]")))) + .expectDropZones("A1", "A2", "A3"), + + new GetDropZonesTestCase().setTitle("mixedNested_returnsDropZones") + .setChildren(new LinkedList<>(List.of(new Content(), new Content("[drop-zone:A2]")))) + .tapQuestion(q -> { + Content content = (Content) q.getChildren().get(0); + content.setChildren(List.of( + new Content("[drop-zone:A1]"), + createFigure("F1", "F2") + )); + }).expectDropZones("A1", "F1", "F2", "A2") + ); + } - @Theory - public final void testGetDropZones(final GetDropZonesTestCase testCase) { - var dropZones = ContentValidatorUtils.DropZones.getFromQuestion(testCase.question); + @ParameterizedTest + @MethodSource("getDropZonesTestCases") + public void testGetDropZones(final GetDropZonesTestCase testCase) { + List dropZones = ContentValidatorUtils.DropZones.getFromQuestion(testCase.question); assertEquals(testCase.dropZones, dropZones); } private static Figure createFigure(final String... dropZones) { - var figure = new Figure(); - figure.setFigureRegions(new ArrayList<>(List.of())); - List.of(dropZones).forEach(dropZoneId -> { - var region = new FigureRegion(); + Figure figure = new Figure(); + figure.setFigureRegions(new ArrayList<>()); + for (String dropZoneId : dropZones) { + FigureRegion region = new FigureRegion(); region.setId(dropZoneId); figure.getFigureRegions().add(region); - }); + } return figure; } diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManagerTest.java index c6d2537fca..22f8424c30 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManagerTest.java @@ -18,11 +18,11 @@ import com.google.api.client.util.Lists; import com.google.api.client.util.Sets; import org.easymock.Capture; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.managers.GameManager; import uk.ac.cam.cl.dtg.segue.api.Constants; +import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserException; import uk.ac.cam.cl.dtg.segue.comm.EmailCommunicationMessage; import uk.ac.cam.cl.dtg.segue.comm.ICommunicator; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; @@ -40,14 +40,13 @@ import java.util.Set; import static org.easymock.EasyMock.*; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the user manager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class GroupManagerTest { private AbstractConfigLoader dummyPropertiesLoader; @@ -66,7 +65,7 @@ public class GroupManagerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.dummyMapper = createMock(MainMapper.class); this.dummyCommunicator = createMock(ICommunicator.class); @@ -93,6 +92,11 @@ public final void groupManager_createValidGroup_aGroupShouldBeCreated() { RegisteredUserDTO someGroupOwner = new RegisteredUserDTO(); someGroupOwner.setId(5339L); someGroupOwner.setEmail("test@test.com"); + + UserSummaryWithEmailAddressDTO someGroupOwnerSummary = new UserSummaryWithEmailAddressDTO(); + someGroupOwnerSummary.setId(5339L); + someGroupOwnerSummary.setEmail("test@test.com"); + Set someSetOfManagers = Sets.newHashSet(); Capture capturedGroup = Capture.newInstance(); @@ -110,7 +114,14 @@ public final void groupManager_createValidGroup_aGroupShouldBeCreated() { .andReturn(resultFromDB); expect(this.groupDataManager.getAdditionalManagerSetByGroupId(anyObject())) .andReturn(someSetOfManagers).atLeastOnce(); - expect(this.userManager.findUsers(someSetOfManagers)).andReturn(someListOfUsers); + try { + expect(this.userManager.getUserDTOById(null)).andReturn(someGroupOwner).atLeastOnce(); + } catch (NoUserException e) { + throw new RuntimeException(e); + } + expect(this.userManager.convertToDetailedUserSummaryObject(someGroupOwner, UserSummaryWithEmailAddressDTO.class)) + .andReturn(someGroupOwnerSummary).atLeastOnce(); + expect(this.userManager.findUsers(someSetOfManagers)).andReturn(someListOfUsers); expect(this.userManager.convertToDetailedUserSummaryObjectList(someListOfUsers, UserSummaryWithEmailAddressDTO.class)).andReturn(someListOfUsersDTOs); expect(this.dummyMapper.map(resultFromDB)).andReturn(mappedGroup).atLeastOnce(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAssociationManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAssociationManagerTest.java index 185e7b7ab7..337f448086 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAssociationManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserAssociationManagerTest.java @@ -17,9 +17,8 @@ import com.google.api.client.util.Sets; import org.easymock.Capture; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.AssociationToken; import uk.ac.cam.cl.dtg.isaac.dos.users.Role; import uk.ac.cam.cl.dtg.isaac.dto.UserGroupDTO; @@ -38,15 +37,14 @@ import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the user Association class. */ -@PowerMockIgnore({"jakarta.ws.*"}) public class UserAssociationManagerTest { private IAssociationDataManager dummyAssociationDataManager; private GroupManager dummyGroupDataManager; @@ -57,7 +55,7 @@ public class UserAssociationManagerTest { * * @throws Exception - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { dummyAssociationDataManager = createMock(IAssociationDataManager.class); dummyGroupDataManager = createMock(GroupManager.class); @@ -388,8 +386,8 @@ public final void userAssociationManager_TokenMustBeSixCharactersAndRandom() // These characters are allowed to be blank! continue; } - assertFalse("Token letter distribution not random; expected " + expectedAvg + " occurrences, found " + occurrences[x][y] + "!", - (occurrences[x][y] > expectedAvg + allowedDelta) || (occurrences[x][y] < expectedAvg - allowedDelta)); + assertFalse((occurrences[x][y] > expectedAvg + allowedDelta) || (occurrences[x][y] < expectedAvg - allowedDelta), + "Token letter distribution not random; expected " + expectedAvg + " occurrences, found " + occurrences[x][y] + "!"); } } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserManagerTest.java index 3d3a8b08f5..c13afd8a63 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/api/managers/UserManagerTest.java @@ -22,10 +22,8 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.StringUtils; import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.reflect.Whitebox; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.AbstractUserPreferenceManager; import uk.ac.cam.cl.dtg.isaac.dos.users.AnonymousUser; import uk.ac.cam.cl.dtg.isaac.dos.users.EmailVerificationStatus; @@ -73,15 +71,14 @@ import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the user manager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class UserManagerTest { private QuestionManager dummyQuestionDatabase; private IUserDataManager dummyDatabase; @@ -110,7 +107,7 @@ public class UserManagerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.dummyQuestionDatabase = createMock(QuestionManager.class); this.dummyDatabase = createMock(IUserDataManager.class); @@ -197,7 +194,7 @@ public final void getCurrentUser_IsAuthenticatedWithValidHMAC_userIsReturned() t returnUser.setId(validUserId); returnUser.setSessionToken(0); - Map sessionInformation = getSessionInformationAsAMap(authManager, validUserId.toString(), validDateString, returnUser.getSessionToken()); + Map sessionInformation = getSessionInformationAsAMap(validUserId.toString(), validDateString, returnUser.getSessionToken()); Cookie[] cookieWithSessionInfo = getCookieArray(sessionInformation); dummyDatabase.updateUserLastSeen(returnUser); @@ -332,6 +329,8 @@ public final void authenticateCallback_checkNewUserIsAuthenticated_createInterna au.setSessionId(someSegueAnonymousUserId); expect(this.dummyUserCache.storeAnonymousUser(au)).andReturn(au).atLeastOnce(); expect(this.dummyUserCache.getById(au.getSessionId())).andReturn(au).atLeastOnce(); + this.dummyUserCache.deleteAnonymousUser(au); + expectLastCall().once(); AnonymousUserDTO someAnonymousUserDTO = new AnonymousUserDTO(); someAnonymousUserDTO.setSessionId(someSegueAnonymousUserId); @@ -406,7 +405,7 @@ public final void authenticateCallback_checkNewUserIsAuthenticated_createInterna expect(dummyDatabase.getById(someSegueUserId)).andReturn(mappedUser); - Map sessionInformation = getSessionInformationAsAMap(authManager, someSegueUserId.toString(), + Map sessionInformation = getSessionInformationAsAMap(someSegueUserId.toString(), validDateString, mappedUser.getSessionToken()); Cookie[] cookieWithSessionInfo = getCookieArray(sessionInformation); @@ -544,14 +543,14 @@ public final void validateUsersSession_checkForValidHMAC_shouldReturnAsCorrect() calendar.add(Calendar.SECOND, 500); String validDateString = sdf.format(calendar.getTime()); - Map sessionInformation = getSessionInformationAsAMap(authManager, validUserId, validDateString, mappedUser.getSessionToken()); + Map sessionInformation = getSessionInformationAsAMap(validUserId, validDateString, mappedUser.getSessionToken()); replay(dummySession); replay(request); replay(dummyQuestionDatabase); // Act - boolean valid = Whitebox. invokeMethod(authManager, "isValidUsersSession", sessionInformation, mappedUser); + boolean valid = authManager.isValidUsersSession(sessionInformation, mappedUser); // Assert verify(dummyQuestionDatabase, dummySession, request); @@ -580,8 +579,8 @@ public final void validateUsersSession_badUsersSession_shouldReturnAsIncorrect() new Date(), Gender.MALE, null, new Date(), null, null, null, null, false); mappedUser.setSessionToken(0); - Map validSessionInformation = getSessionInformationAsAMap(authManager, validUserId, - validDateString, mappedUser.getSessionToken()); + Map validSessionInformation = getSessionInformationAsAMap(validUserId, validDateString, + mappedUser.getSessionToken()); Map tamperedSessionInformation = ImmutableMap.of( Constants.SESSION_USER_ID, validUserId, @@ -595,7 +594,7 @@ public final void validateUsersSession_badUsersSession_shouldReturnAsIncorrect() replay(dummyQuestionDatabase); // Act - boolean valid = Whitebox. invokeMethod(authManager, "isValidUsersSession", tamperedSessionInformation, mappedUser); + boolean valid = authManager.isValidUsersSession(tamperedSessionInformation, mappedUser); // Assert verify(dummyQuestionDatabase, dummySession, request); @@ -624,15 +623,15 @@ public final void validateUsersSession_expiredUsersSession_shouldReturnAsIncorre calendar.add(Calendar.SECOND, -60); // Expired 60 seconds ago String expiredDateString = sdf.format(calendar.getTime()); - Map validSessionInformation = getSessionInformationAsAMap(authManager, validUserId, - expiredDateString, mappedUser.getSessionToken()); + Map validSessionInformation = getSessionInformationAsAMap(validUserId, expiredDateString, + mappedUser.getSessionToken()); replay(dummySession); replay(request); replay(dummyQuestionDatabase); // Act - boolean valid = Whitebox. invokeMethod(authManager, "isValidUsersSession", validSessionInformation, mappedUser); + boolean valid = authManager.isValidUsersSession(validSessionInformation, mappedUser); // Assert verify(dummyQuestionDatabase, dummySession, request); @@ -662,7 +661,7 @@ public final void validateUsersSession_incorrectSessionToken_shouldReturnAsIncor calendar.add(Calendar.SECOND, 500); String validDateString = sdf.format(calendar.getTime()); - Map sessionInformationWithTokenMismatch = getSessionInformationAsAMap(authManager, validUserId, + Map sessionInformationWithTokenMismatch = getSessionInformationAsAMap(validUserId, validDateString, incorrectSessionToken); replay(dummySession); @@ -670,7 +669,7 @@ public final void validateUsersSession_incorrectSessionToken_shouldReturnAsIncor replay(dummyQuestionDatabase); // Act - boolean valid = Whitebox. invokeMethod(authManager, "isValidUsersSession", sessionInformationWithTokenMismatch, mappedUser); + boolean valid = authManager.isValidUsersSession(sessionInformationWithTokenMismatch, mappedUser); // Assert verify(dummyQuestionDatabase, dummySession, request); @@ -802,9 +801,8 @@ private UserAuthenticationManager buildTestAuthenticationManager(AuthenticationP return new UserAuthenticationManager(dummyDatabase, dummyDeletionTokenManager, dummyPropertiesLoader, providerMap, dummyQueue); } - private Map getSessionInformationAsAMap(UserAuthenticationManager userAuthManager, String userId, String dateExpires, Integer sessionToken) - throws Exception { - String validHMAC = Whitebox. invokeMethod(userAuthManager, "calculateSessionHMAC", dummyHMACSalt, userId, + private Map getSessionInformationAsAMap(String userId, String dateExpires, Integer sessionToken) { + String validHMAC = UserAuthenticationManager.calculateSessionHMAC(dummyHMACSalt, userId, dateExpires, sessionToken.toString(), null); return ImmutableMap.of(Constants.SESSION_USER_ID, userId, Constants.DATE_EXPIRES, dateExpires, Constants.HMAC, validHMAC, Constants.SESSION_TOKEN, sessionToken.toString()); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/api/monitors/MisuseMonitorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/api/monitors/MisuseMonitorTest.java index 08a18eca30..51461972f5 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/api/monitors/MisuseMonitorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/api/monitors/MisuseMonitorTest.java @@ -16,9 +16,8 @@ package uk.ac.cam.cl.dtg.segue.api.monitors; import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.api.managers.SegueResourceMisuseException; import uk.ac.cam.cl.dtg.segue.comm.EmailCommunicationMessage; @@ -28,13 +27,12 @@ import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import static org.easymock.EasyMock.*; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the user manager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class MisuseMonitorTest { private AbstractConfigLoader dummyPropertiesLoader; private EmailManager dummyCommunicator; @@ -45,7 +43,7 @@ public class MisuseMonitorTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.dummyCommunicator = createMock(EmailManager.class); this.dummyPropertiesLoader = createMock(AbstractConfigLoader.class); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/GoogleAuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/GoogleAuthenticatorTest.java index e593fb7f66..99fee5d410 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/GoogleAuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/GoogleAuthenticatorTest.java @@ -15,10 +15,10 @@ */ package uk.ac.cam.cl.dtg.segue.auth; -import org.junit.Before; import com.google.api.client.googleapis.auth.oauth2.GoogleClientSecrets; import com.google.api.client.googleapis.auth.oauth2.GoogleClientSecrets.Details; +import org.junit.jupiter.api.BeforeEach; public class GoogleAuthenticatorTest extends IOAuth2AuthenticatorTest { @@ -28,7 +28,7 @@ public class GoogleAuthenticatorTest extends IOAuth2AuthenticatorTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { GoogleClientSecrets clientSecrets = new GoogleClientSecrets(); Details details = new Details(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth1AuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth1AuthenticatorTest.java index 94bf4364cd..2698392b7d 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth1AuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth1AuthenticatorTest.java @@ -15,12 +15,12 @@ */ package uk.ac.cam.cl.dtg.segue.auth; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.io.IOException; import java.net.URL; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the OAuth authenticator class. diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth2AuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth2AuthenticatorTest.java index 1660e977ae..2e578a0ae0 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth2AuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuth2AuthenticatorTest.java @@ -16,13 +16,13 @@ package uk.ac.cam.cl.dtg.segue.auth; import com.google.api.client.http.GenericUrl; -import org.junit.Test; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.auth.exceptions.AuthenticationCodeException; import java.io.IOException; import java.net.URL; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Test class for the OAuth authenticator class. diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuthAuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuthAuthenticatorTest.java index 534997ca3d..55eb458647 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuthAuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/IOAuthAuthenticatorTest.java @@ -15,15 +15,15 @@ */ package uk.ac.cam.cl.dtg.segue.auth; -import org.junit.Test; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.auth.exceptions.AuthenticatorSecurityException; import uk.ac.cam.cl.dtg.segue.auth.exceptions.CodeExchangeException; import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserException; import java.io.IOException; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the OAuth authenticator class. diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/MicrosoftAuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/MicrosoftAuthenticatorTest.java index 8bbc702405..70a1d5b79a 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/MicrosoftAuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/MicrosoftAuthenticatorTest.java @@ -17,14 +17,15 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; - -import org.junit.Before; -import org.junit.experimental.runners.Enclosed; +import org.apache.commons.lang3.tuple.Pair; +import org.eclipse.jetty.server.Server; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; -import org.junit.runner.RunWith; -import org.junit.Test; - -import org.junit.runners.Parameterized; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import uk.ac.cam.cl.dtg.isaac.dos.users.EmailVerificationStatus; import uk.ac.cam.cl.dtg.isaac.dos.users.UserFromAuthProvider; import uk.ac.cam.cl.dtg.segue.auth.exceptions.AuthenticationCodeException; @@ -36,21 +37,21 @@ import uk.ac.cam.cl.dtg.segue.auth.microsoft.Token; import java.util.Arrays; -import java.util.Collection; import java.util.Date; import java.util.List; import java.util.UUID; +import java.util.stream.Stream; -import static org.junit.jupiter.api.Assertions.*; import static java.lang.String.format; +import static org.junit.jupiter.api.Assertions.*; -@RunWith(Enclosed.class) -public class MicrosoftAuthenticatorTest { - @RunWith(Parameterized.class) - public static class TestInitialisation extends Helpers { - @Parameterized.Parameters(name="{0}") - public static Collection data() { - return Arrays.asList(new Object[][] { +class MicrosoftAuthenticatorTest { + + @Nested + class TestInitialisation extends Helpers { + + static Stream data() { + return Stream.of(new Object[][] { { "client_id", (C) v -> new MicrosoftAuthenticator(v, tenantId, clientSecret, jwksUrl, redirectUrl) }, { "tenant_id", (C) v -> new MicrosoftAuthenticator(clientId, v, clientSecret, jwksUrl, redirectUrl) }, { "client_secret", (C) v -> new MicrosoftAuthenticator(clientId, tenantId, v, jwksUrl, redirectUrl) }, @@ -59,20 +60,16 @@ public static Collection data() { }); } - @Parameterized.Parameter() - public String field; - - @Parameterized.Parameter(1) - public C consumer; - - @Test - public void microsoftAuthenticator_nullValue_throwsError() { + @ParameterizedTest + @MethodSource("data") + void microsoftAuthenticator_nullValue_throwsError(final String field, final C consumer) { Executable act = () -> consumer.consume(null); assertError(act, NullPointerException.class, format("Missing %s, can't be \"null\".", field)); } - @Test - public void microsoftAuthenticator_emptyValue_throwsError() { + @ParameterizedTest + @MethodSource("data") + void microsoftAuthenticator_emptyValue_throwsError(final String field, final C consumer) { Executable act = () -> consumer.consume(""); assertError(act, IllegalArgumentException.class, format("Missing %s, can't be \"\".", field)); } @@ -86,57 +83,62 @@ public interface C { // Contains tests for: // 1) getAuthorizationUrl, 2) extractAuthCode, 3) getAntiForgeryStateToken, 4) getUserInfo, // 5) getAuthenticationProvider, 6) exchangeCode - public static class TestShared extends IOAuth2AuthenticatorTest { - @Before - public final void setUp() throws Exception { + @Nested + class TestShared extends IOAuth2AuthenticatorTest { + @BeforeEach + final void setUp() throws Exception { this.oauth2Authenticator = Helpers.subject(Helpers.getStore()); this.authenticator = this.oauth2Authenticator; } } - public static class TestExtractAuthCode extends Helpers { - /** happy path: {@link IOAuth2AuthenticatorTest#extractAuthCode_givenValidUrl_returnsCorrectCode} */ + @Nested + class TestExtractAuthCode extends Helpers { + /** happy path: {@link IOAuth2AuthenticatorTest#extractAuthCode_givenValidUrl_returnsCorrectCode}. */ @Test - public void extractAuthCode_givenInvalidUrl_throwsError() { + void extractAuthCode_givenInvalidUrl_throwsError() { Executable act = () -> subject(getStore()).extractAuthCode("https://example.com"); assertError(act, AuthenticationCodeException.class, "Error extracting authentication code."); } } - public static class TestGetAuthorizationUrl extends Helpers { + @Nested + class TestGetAuthorizationUrl extends Helpers { @Test - public void getAuthorizationUrl_validAntiForgeryToken_returnsUrl() { + void getAuthorizationUrl_validAntiForgeryToken_returnsUrl() { var csrfToken = "the_csrf_token"; var url = subject(getStore()).getAuthorizationUrl(csrfToken); - var expectedUrl = "https://login.microsoftonline.com/common/oauth2/v2.0/authorize" + - format("?client_id=%s", clientId) + - format("&redirect_uri=%s", redirectUrl) + - format("&response_type=%s", "code") + - format("&scope=%s", "openid%20profile%20email") + - format("&state=%s", csrfToken) + - format("&prompt=%s", "select_account") + - format("&response_mode=%s", "query"); + var expectedUrl = "https://login.microsoftonline.com/common/oauth2/v2.0/authorize" + + format("?client_id=%s", clientId) + + format("&redirect_uri=%s", redirectUrl) + + format("&response_type=%s", "code") + + format("&scope=%s", "openid%20profile%20email") + + format("&state=%s", csrfToken) + + format("&prompt=%s", "select_account") + + format("&response_mode=%s", "query"); assertEquals(expectedUrl, url); } } - public static class TestExchangeCode extends Helpers { + @Nested + class TestExchangeCode extends Helpers { @Test - public void exchangeCode_invalidCode_throwsCodeExchangeExceptionWithoutAnyDetails() { + void exchangeCode_invalidCode_throwsCodeExchangeExceptionWithoutAnyDetails() { Executable act = () -> subject(getStore()).exchangeCode(null); assertError(act, CodeExchangeException.class, "There was an error exchanging the code."); } } - @RunWith(Enclosed.class) - public static class TestGetUserInfo extends GetUserInfoHelpers { - @RunWith(Enclosed.class) - public static class TestUserClaims { - public static class TestValid { + @Nested + class TestGetUserInfo extends GetUserInfoHelpers { + @Nested + class TestUserClaims { + @Nested + class TestValid { @Test - public void getUserInfo_validToken_returnsUserInformation() throws Throwable { - var oid = UUID.randomUUID().toString(); - var t = token.valid(s -> s, u -> { + void getUserInfo_validToken_returnsUserInformation() throws Throwable { + String oid = UUID.randomUUID().toString(); + String t = token.valid(s -> s, u -> { u.put("oid", oid); u.put("email", "test@example.org"); u.put("family_name", "Doe"); @@ -144,7 +146,7 @@ public void getUserInfo_validToken_returnsUserInformation() throws Throwable { return null; }); - var userInfo = testGetUserInfo(t); + UserFromAuthProvider userInfo = testGetUserInfo(t); assertEquals(oid, userInfo.getProviderUserId()); assertEquals("test@example.org", userInfo.getEmail()); @@ -155,63 +157,64 @@ public void getUserInfo_validToken_returnsUserInformation() throws Throwable { } } - public static class TestOidClaim extends TestUUIDClaim { + @Nested + class TestOidClaim extends TestUUIDClaim { @Override String claim() { return "oid"; } } - @RunWith(Parameterized.class) - public static class TestValidNameClaims { - @Parameterized.Parameters(name="test case {index}") - public static Collection data() { - return Arrays.asList(new String[][][] { - { {"John", "Doe", "JohnDoe" }, {"John", "Doe"} }, - { {"John", "Doe", null }, {"John", "Doe"} }, - { {"John", "Doe", "" }, {"John", "Doe"} }, - { {"John", null, "John Doe" }, {"John", null} }, - { {"John", "", "John Doe" }, {"John", null} }, - { { null, "Doe", "John Doe" }, {null, "Doe"} }, - { { "", "Doe", "John Doe" }, {null, "Doe"} }, - { { null, null, "John Doe" }, {"John", "Doe"} }, - { { "", "", "John Doe" }, {"John", "Doe"} }, - { { null, null, "Doe" }, {null, "Doe"} }, - { { null, null, "John" }, {null, "John"} }, - { { null, null, " John " }, {null, "John"} }, - { { null, null, "John " }, {null, "John"} }, - { { null, null, "John " }, {null, "John"} }, - { { null, null, "John Joanne Doe" }, {"John Joanne", "Doe"} }, - { { null, null, "John Joanne Josephine Doe" }, {"John Joanne Josephine", "Doe"} }, - { { null, null, "John Joanne Josephine Doe " }, {"John Joanne Josephine", "Doe"} }, - }); + @Nested + class TestValidNameClaims { + static Stream data() { + return Stream.of( + Arguments.of(new String[]{"John", "Doe", "JohnDoe"}, new String[]{"John", "Doe"}), + Arguments.of(new String[]{"John", "Doe", null}, new String[]{"John", "Doe"}), + Arguments.of(new String[]{"John", "Doe", ""}, new String[]{"John", "Doe"}), + Arguments.of(new String[]{"John", null, "John Doe"}, new String[]{"John", null}), + Arguments.of(new String[]{"John", "", "John Doe"}, new String[]{"John", null}), + Arguments.of(new String[]{null, "Doe", "John Doe"}, new String[]{null, "Doe"}), + Arguments.of(new String[]{"", "Doe", "John Doe"}, new String[]{null, "Doe"}), + Arguments.of(new String[]{null, null, "John Doe"}, new String[]{"John", "Doe"}), + Arguments.of(new String[]{"", "", "John Doe"}, new String[]{"John", "Doe"}), + Arguments.of(new String[]{null, null, "Doe"}, new String[]{null, "Doe"}), + Arguments.of(new String[]{null, null, "John"}, new String[]{null, "John"}), + Arguments.of(new String[]{null, null, " John "}, new String[]{null, "John"}), + Arguments.of(new String[]{null, null, "John "}, new String[]{null, "John"}), + Arguments.of(new String[]{null, null, "John "}, new String[]{null, "John"}), + Arguments.of(new String[]{null, null, "John Joanne Doe"}, + new String[]{"John Joanne", "Doe"}), + Arguments.of(new String[]{null, null, "John Joanne Josephine Doe"}, + new String[]{"John Joanne Josephine", "Doe"}), + Arguments.of(new String[]{null, null, "John Joanne Josephine Doe "}, + new String[]{"John Joanne Josephine", "Doe"}) + ); } - @Parameterized.Parameter() - public String[] input; - - @Parameterized.Parameter(1) - public String[] expectedOutput; - - @Test - public void getUserInfo_validNameClaims_accepted() throws Throwable { - var t = token.valid(s -> s, u -> u.setName(input[0], input[1], input[2])); - var userInfo = testGetUserInfo(t); - assertEquals(expectedOutput[0], userInfo.getGivenName(), String.format("given name failed for %s", Arrays.toString(input))); - assertEquals(expectedOutput[1], userInfo.getFamilyName(), String.format("last name failed for %s", Arrays.toString(input))); + @ParameterizedTest + @MethodSource("data") + void getUserInfo_validNameClaims_accepted(final String[] input, final String[] expectedOutput) throws Throwable { + String t = token.valid(s -> s, u -> u.setName(input[0], input[1], input[2])); + UserFromAuthProvider userInfo = testGetUserInfo(t); + assertEquals(expectedOutput[0], userInfo.getGivenName(), + String.format("given name failed for %s", Arrays.toString(input))); + assertEquals(expectedOutput[1], userInfo.getFamilyName(), + String.format("last name failed for %s", Arrays.toString(input))); } } - public static class TestInvalidNameClaims { + @Nested + class TestInvalidNameClaims { @Test - public void getUserInfo_MissingNameClaims_rejected() { - var t = token.valid(s -> s, u -> u.setName(null, null, null)); + void getUserInfo_MissingNameClaims_rejected() { + String t = token.valid(s -> s, u -> u.setName(null, null, null)); testGetUserInfo(t, NoUserException.class, "Could not determine name"); } @Test - public void getUserInfo_nullNameClaims_rejected() { - var t = token.valid(s -> s, u -> { + void getUserInfo_nullNameClaims_rejected() { + String t = token.valid(s -> s, u -> { u.put("given_name", null); u.put("family_name", null); u.put("name", null); @@ -221,146 +224,155 @@ public void getUserInfo_nullNameClaims_rejected() { } @Test - public void getUserInfo_EmptyNameClaims_rejected() { - var t = token.valid(s -> s, u -> u.setName("", "", "")); + void getUserInfo_EmptyNameClaims_rejected() { + String t = token.valid(s -> s, u -> u.setName("", "", "")); testGetUserInfo(t, NoUserException.class, "Could not determine name"); } } - public static class TestTidClaim extends TestUUIDClaim { + @Nested + class TestTidClaim extends TestUUIDClaim { @Override String claim() { return "tid"; } } - public static class TestEmailClaim extends TestNonEmptyClaim { + @Nested + class TestEmailClaim extends TestNonEmptyClaim { @Override String claim() { return "email"; } @Test - public void getUserInfo_emailInvalid_throwsError() { - var t = token.valid(s -> s, u -> u.put("email", "some_bad_email")); + void getUserInfo_emailInvalid_throwsError() { + String t = token.valid(s -> s, u -> u.put("email", "some_bad_email")); testGetUserInfo(t, NoUserException.class, "User verification: BAD_CLAIM (email)"); } } } - @RunWith(Enclosed.class) - public static class TestSystemClaims { - public static class TestTokenMissing { + @Nested + class TestSystemClaims { + @Nested + class TestTokenMissing { @Test - public void getUserInfo_tryingToStoreNull_throwsError() { + void getUserInfo_tryingToStoreNull_throwsError() { assertThrows(NullPointerException.class, () -> testGetUserInfo(null)); } @Test - public void getUserInfo_noSuchToken_throwsError() { + void getUserInfo_noSuchToken_throwsError() { Executable act = () -> subject(getStore()).getUserInfo("no_token_for_id"); assertError(act, AuthenticatorSecurityException.class, "Token verification: TOKEN_MISSING"); } } - public static class TestSignatureVerification { + @Nested + class TestSignatureVerification { @Test - public void getUserInfo_tokenSignatureNoKeyId_throwsError() { - var t = token.valid(s -> s.withKeyId(null), u -> u); + void getUserInfo_tokenSignatureNoKeyId_throwsError() { + String t = token.valid(s -> s.withKeyId(null), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: KEY_NOT_FOUND"); } @Test - public void getUserInfo_tokenSignatureKeyNotFound_throwsError() { - var t = token.valid(s -> s.withKeyId("no-such-key"), u -> u); + void getUserInfo_tokenSignatureKeyNotFound_throwsError() { + String t = token.valid(s -> s.withKeyId("no-such-key"), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: KEY_NOT_FOUND"); } @Test - public void getUserInfo_tokenSignatureMismatch_throwsError() { - var t = Token.signed(invalidSigningKey, s -> s.withKeyId(validSigningKey.getPublic().id())); + void getUserInfo_tokenSignatureMismatch_throwsError() { + String t = Token.signed(invalidSigningKey, s -> s.withKeyId(validSigningKey.getPublic().id())); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_SIGNATURE"); } } - public static class TestExpirationClaim { + @Nested + class TestExpirationClaim { @Test - public void getUserInfo_tokenWithoutExp_accepted() { + void getUserInfo_tokenWithoutExp_accepted() { assertDoesNotThrow(() -> testGetUserInfo(token.valid(s -> s.withExpiresAt((Date) null), u -> u))); } @Test - public void getUserInfo_tokenExpired_throwsError() { - var t = token.valid(s -> s.withExpiresAt(Token.oneHourAgo), u -> u); + void getUserInfo_tokenExpired_throwsError() { + String t = token.valid(s -> s.withExpiresAt(Token.oneHourAgo), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: TOKEN_EXPIRED"); } } - public static class TestIssuedAtClaim { + @Nested + class TestIssuedAtClaim { @Test - public void getUserInfo_tokenWithoutIat_accepted() { + void getUserInfo_tokenWithoutIat_accepted() { assertDoesNotThrow(() -> testGetUserInfo(token.valid(s -> s.withIssuedAt((Date) null), u -> u))); } @Test - public void getUserInfo_tokenIatFuture_throwsError() { + void getUserInfo_tokenIatFuture_throwsError() { var t = token.valid(s -> s.withIssuedAt(Token.inOneHour), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (iat)"); } } - public static class TestNotBeforeClaim { + @Nested + class TestNotBeforeClaim { @Test - public void getUserInfo_tokenWithoutNbf_accepted() { + void getUserInfo_tokenWithoutNbf_accepted() { assertDoesNotThrow(() -> testGetUserInfo(token.valid(s -> s.withNotBefore((Date) null), u -> u))); } @Test - public void getUserInfo_tokenNbfFuture_throwsError() { - var t = token.valid(s -> s.withNotBefore(Token.inOneHour), u -> u); + void getUserInfo_tokenNbfFuture_throwsError() { + String t = token.valid(s -> s.withNotBefore(Token.inOneHour), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (nbf)"); } } - public static class TestAudienceClaim { + @Nested + class TestAudienceClaim { @Test - public void getUserInfo_tokenMissingAud_throwsError() { - var t = token.valid(s -> s.withAudience((String) null), u -> u); + void getUserInfo_tokenMissingAud_throwsError() { + String t = token.valid(s -> s.withAudience((String) null), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (aud)"); } @Test - public void getUserInfo_tokenAudIncorrect_throwsError() { - var t = token.valid(s -> s.withAudience("intended_for_somebody_else"), u -> u); + void getUserInfo_tokenAudIncorrect_throwsError() { + String t = token.valid(s -> s.withAudience("intended_for_somebody_else"), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (aud)"); } } - public static class TestIssuerClaim { + @Nested + class TestIssuerClaim { @Test - public void getUserInfo_tokenMissingIssuer_throwsError() { - var t = token.valid(s -> s.withIssuer(null), u -> u); + void getUserInfo_tokenMissingIssuer_throwsError() { + String t = token.valid(s -> s.withIssuer(null), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (iss)"); } @Test - public void getUserInfo_tokenIncorrectIssuer_throwsError() { - var t = token.valid(s -> s.withIssuer("some_bad_issuer"), u -> u); + void getUserInfo_tokenIncorrectIssuer_throwsError() { + String t = token.valid(s -> s.withIssuer("some_bad_issuer"), u -> u); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (iss)"); } @Test - public void getUserInfo_tokenTenantIssuerMismatch_throwsError() { - var tid = UUID.randomUUID().toString(); - var t = token.valid(s -> s.withIssuer(Token.expectedIssuer(Token.msTenantId)), + void getUserInfo_tokenTenantIssuerMismatch_throwsError() { + String tid = UUID.randomUUID().toString(); + String t = token.valid(s -> s.withIssuer(Token.expectedIssuer(Token.msTenantId)), u -> u.put("tid", tid)); testGetUserInfo(t, AuthenticatorSecurityException.class, "Token verification: BAD_CLAIM (iss)"); } @Test - public void getUserInfo_tokenTenantSpecificIssuer_accepted() { - var tid = UUID.randomUUID().toString(); - var t = token.valid(s -> s.withIssuer(Token.expectedIssuer(tid)), u -> u.put("tid", tid)); + void getUserInfo_tokenTenantSpecificIssuer_accepted() { + String tid = UUID.randomUUID().toString(); + String t = token.valid(s -> s.withIssuer(Token.expectedIssuer(tid)), u -> u.put("tid", tid)); assertDoesNotThrow(() -> testGetUserInfo(t)); } } @@ -381,8 +393,8 @@ static Cache getStore() { return CacheBuilder.newBuilder().build(); } - static void assertError(Executable action, Class errorClass, String errorMessage) { - var error = assertThrows(errorClass, action); + static void assertError(final Executable action, final Class errorClass, final String errorMessage) { + Exception error = assertThrows(errorClass, action); assertEquals(errorMessage, error.getMessage()); } @@ -394,30 +406,30 @@ static void assertError(Executable action, Class errorC } class GetUserInfoHelpers extends Helpers { - static abstract class TestNonEmptyClaim { + abstract static class TestNonEmptyClaim { abstract String claim(); @Test - public void getUserInfo_missing_throwsError() { - var t = token.valid(s -> s, u -> u.remove(claim())); + void getUserInfo_missing_throwsError() { + String t = token.valid(s -> s, u -> u.remove(claim())); testGetUserInfo(t, NoUserException.class, expectedMessage()); } @Test - public void getUserInfo_null_throwsError() { - var t = token.valid(s -> s, u -> u.put(claim(), null)); + void getUserInfo_null_throwsError() { + String t = token.valid(s -> s, u -> u.put(claim(), null)); testGetUserInfo(t, NoUserException.class, expectedMessage()); } @Test - public void getUserInfo_empty_throwsError() { - var t = token.valid(s -> s, u -> u.put(claim(), "")); + void getUserInfo_empty_throwsError() { + String t = token.valid(s -> s, u -> u.put(claim(), "")); testGetUserInfo(t, NoUserException.class, expectedMessage()); } @Test - public void getUserInfo_blank_throwsError() { - var t = token.valid(s -> s, u -> u.put(claim(), " ")); + void getUserInfo_blank_throwsError() { + String t = token.valid(s -> s, u -> u.put(claim(), " ")); testGetUserInfo(t, NoUserException.class, expectedMessage()); } @@ -426,25 +438,26 @@ String expectedMessage() { } } - static abstract class TestUUIDClaim extends TestNonEmptyClaim { + abstract static class TestUUIDClaim extends TestNonEmptyClaim { @Test - public void getUserInfo_invalid_throwsError() { - var t = token.valid(s -> s, u -> u.put(claim(), "some_bad_uuid")); + void getUserInfo_invalid_throwsError() { + String t = token.valid(s -> s, u -> u.put(claim(), "some_bad_uuid")); testGetUserInfo(t, NoUserException.class, expectedMessage()); } @Test - public void getUserInfo_prefixValidButOverallInvalid_throwsError() { - var t = token.valid(s -> s, u -> u.put(claim(), format("%s/hello/", UUID.randomUUID()))); + void getUserInfo_prefixValidButOverallInvalid_throwsError() { + String t = token.valid(s -> s, u -> u.put(claim(), format("%s/hello/", UUID.randomUUID()))); testGetUserInfo(t, NoUserException.class, expectedMessage()); } } - static UserFromAuthProvider testGetUserInfo(String token) throws Throwable { - var store = getStore(); - var subject = subject(store); + static UserFromAuthProvider testGetUserInfo(final String token) throws Throwable { + Cache store = getStore(); + MicrosoftAuthenticator subject = subject(store); store.put("the_internal_id", token); - var keySetServer = KeySetServlet.startServer(8888, List.of(validSigningKey, anotherValidSigningKey)); + Pair keySetServer = + KeySetServlet.startServer(8888, List.of(validSigningKey, anotherValidSigningKey)); try { return subject.getUserInfo("the_internal_id"); } finally { @@ -452,7 +465,8 @@ static UserFromAuthProvider testGetUserInfo(String token) throws Throwable { } } - static void testGetUserInfo(String token, Class errorClass, String errorMessage) { + static void testGetUserInfo(final String token, final Class errorClass, + final String errorMessage) { assertError(() -> testGetUserInfo(token), errorClass, errorMessage); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/RaspberryPiOidcAuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/RaspberryPiOidcAuthenticatorTest.java index 4696a82579..5f766c133b 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/RaspberryPiOidcAuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/RaspberryPiOidcAuthenticatorTest.java @@ -16,22 +16,22 @@ package uk.ac.cam.cl.dtg.segue.auth; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserException; import java.net.URL; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.List; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class RaspberryPiOidcAuthenticatorTest { RaspberryPiOidcAuthenticator authenticator; - @Before + @BeforeEach public void setUp() throws Exception{ // Set up an authenticator with local OIDC IdP metadata URL res = getClass().getClassLoader().getResource("test-rpf-idp-metadata.json"); @@ -76,17 +76,19 @@ public void getGivenNameFamilyName_emptyTokenisedNameProvided_returnsSensibleNam assertEquals("John", givenNameFamilyName.get(1)); } - @Test(expected = NoUserException.class) - public void getGivenNameFamilyName_invalidNicknameProvided_throwsException() throws Exception{ - // Arrange - String idpNickname = "*"; - String idpFullName = "John Smith"; - - // Act - authenticator.getGivenNameFamilyName(idpNickname, idpFullName); - - // Assert - // See signature + @Test + public void getGivenNameFamilyName_invalidNicknameProvided_throwsException() { + assertThrows(NoUserException.class, () -> { + // Arrange + String idpNickname = "*"; + String idpFullName = "John Smith"; + + // Act + authenticator.getGivenNameFamilyName(idpNickname, idpFullName); + + // Assert + // See signature + }); } @Test diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/SegueLocalAuthenticatorTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/SegueLocalAuthenticatorTest.java index d391602ef7..76bad5c346 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/auth/SegueLocalAuthenticatorTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/auth/SegueLocalAuthenticatorTest.java @@ -16,8 +16,8 @@ package uk.ac.cam.cl.dtg.segue.auth; import com.google.common.collect.ImmutableMap; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.users.LocalUserCredential; import uk.ac.cam.cl.dtg.isaac.dos.users.RegisteredUser; import uk.ac.cam.cl.dtg.segue.auth.exceptions.IncorrectCredentialsProvidedException; @@ -38,7 +38,7 @@ import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the SegueLocalAuthenticator class. @@ -66,7 +66,7 @@ public class SegueLocalAuthenticatorTest { * * @throws Exception - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.userDataManager = createMock(IUserDataManager.class); this.passwordDataManager = createMock(IPasswordDataManager.class); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/comm/EmailManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/comm/EmailManagerTest.java index a82d4b3d15..729d459355 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/comm/EmailManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/comm/EmailManagerTest.java @@ -15,9 +15,11 @@ */ package uk.ac.cam.cl.dtg.segue.comm; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; import java.util.Date; import java.util.List; @@ -27,14 +29,8 @@ import org.easymock.Capture; import org.easymock.EasyMock; import org.easymock.IAnswer; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; -import org.powermock.api.easymock.PowerMock; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,9 +57,6 @@ * Test class for the EmailManager class. * */ -@RunWith(PowerMockRunner.class) -@PrepareForTest(GitContentManager.class) -@PowerMockIgnore("javax.management.*") public class EmailManagerTest { private final String CONTENT_VERSION = "liveVersion"; @@ -88,7 +81,7 @@ public class EmailManagerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { // Create dummy user @@ -137,7 +130,7 @@ public final void setUp() throws Exception { // Create content manager - mockContentManager = PowerMock.createMock(GitContentManager.class); + mockContentManager = EasyMock.createMock(GitContentManager.class); // Create log manager logManager = EasyMock.createMock(ILogManager.class); @@ -155,7 +148,7 @@ public final void setUp() throws Exception { EasyMock.isA(EmailCommunicationMessage.class))); } catch (CommunicationException e1) { e1.printStackTrace(); - Assert.fail(); + fail(); } EasyMock.replay(emailCommunicator); @@ -244,7 +237,7 @@ public final void sendTemplatedEmailToUser_checkForTemplateCompletion_emailShoul } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, @@ -256,10 +249,10 @@ public final void sendTemplatedEmailToUser_checkForTemplateCompletion_emailShoul emailTokens, EmailType.SYSTEM); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } catch (SegueDatabaseException e) { e.printStackTrace(); - Assert.fail(); + fail(); } final String expectedMessagePlainText = "Hi, tester." @@ -280,7 +273,7 @@ public final void sendTemplatedEmailToUser_checkForTemplateCompletion_emailShoul i++; } catch (InterruptedException e) { e.printStackTrace(); - Assert.fail(); + fail(); } } email = capturedArgument.getValue(); @@ -319,7 +312,7 @@ public final void sendFederatedPasswordReset_checkForTemplateCompletion_emailSho EasyMock.replay(mockContentManager); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, @@ -332,12 +325,12 @@ public final void sendFederatedPasswordReset_checkForTemplateCompletion_emailSho } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); log.debug(e.getMessage()); } catch (SegueDatabaseException e) { e.printStackTrace(); log.debug(e.getMessage()); - Assert.fail(); + fail(); } final String expectedMessage = "Hello, tester.\n\nYou requested a password reset. " @@ -352,7 +345,7 @@ public final void sendFederatedPasswordReset_checkForTemplateCompletion_emailSho i++; } catch (InterruptedException e) { e.printStackTrace(); - Assert.fail(); + fail(); } } email = capturedArgument.getValue(); @@ -392,7 +385,7 @@ public final void sendPasswordReset_checkForTemplateCompletion_emailShouldBeSent } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, @@ -407,10 +400,10 @@ public final void sendPasswordReset_checkForTemplateCompletion_emailShouldBeSent } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } catch (SegueDatabaseException e) { e.printStackTrace(); - Assert.fail(); + fail(); } final String expectedMessage = "Hello, tester.\n\nA request has been " @@ -426,7 +419,7 @@ public final void sendPasswordReset_checkForTemplateCompletion_emailShouldBeSent i++; } catch (InterruptedException e) { e.printStackTrace(); - Assert.fail(); + fail(); } } email = capturedArgument.getValue(); @@ -441,47 +434,49 @@ public final void sendPasswordReset_checkForTemplateCompletion_emailShouldBeSent * * @throws CommunicationException */ - @Test(expected = IllegalArgumentException.class) + @Test public final void sendRegistrationConfirmation_checkForInvalidTemplateTags_throwIllegalArgumentException() { - EmailTemplateDTO template = createDummyEmailTemplate("Hi, {{givenName}} {{surname}}.\n" - + "Thanks for registering!\nYour Isaac email address is: " - + "{{email}}.\naddress\n{{sig}}"); + assertThrows(IllegalArgumentException.class, () -> { + EmailTemplateDTO template = createDummyEmailTemplate("Hi, {{givenName}} {{surname}}.\n" + + "Thanks for registering!\nYour Isaac email address is: " + + "{{email}}.\naddress\n{{sig}}"); - ContentDTO htmlTemplate = createDummyContentTemplate("{{content}}"); - // Create content manager - try { - EasyMock.expect( - mockContentManager.getContentById("email-template-registration-confirmation")) - .andReturn(template).once(); + ContentDTO htmlTemplate = createDummyContentTemplate("{{content}}"); + // Create content manager + try { + EasyMock.expect( + mockContentManager.getContentById("email-template-registration-confirmation")) + .andReturn(template).once(); - EasyMock.expect(mockContentManager.getContentById("email-template-html")) - .andReturn(htmlTemplate).once(); + EasyMock.expect(mockContentManager.getContentById("email-template-html")) + .andReturn(htmlTemplate).once(); - EasyMock.expect(mockContentManager.getCurrentContentSHA()).andReturn(CONTENT_VERSION).atLeastOnce(); + EasyMock.expect(mockContentManager.getCurrentContentSHA()).andReturn(CONTENT_VERSION).atLeastOnce(); - EasyMock.replay(mockContentManager); + EasyMock.replay(mockContentManager); - } catch (ContentManagerException e) { - e.printStackTrace(); - Assert.fail(); - } + } catch (ContentManagerException e) { + e.printStackTrace(); + fail(); + } - EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, - mockContentManager, logManager, generateGlobalTokenMap()); - try { - ImmutableMap emailTokens = ImmutableMap.of("verificationURL", "https://testUrl.com"); + EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, + mockContentManager, logManager, generateGlobalTokenMap()); + try { + ImmutableMap emailTokens = ImmutableMap.of("verificationURL", "https://testUrl.com"); - manager.sendTemplatedEmailToUser(userDTO, - template, - emailTokens, EmailType.SYSTEM); + manager.sendTemplatedEmailToUser(userDTO, + template, + emailTokens, EmailType.SYSTEM); - } catch (ContentManagerException e) { - e.printStackTrace(); - Assert.fail(); - } catch (SegueDatabaseException e) { - e.printStackTrace(); - Assert.fail(); - } + } catch (ContentManagerException e) { + e.printStackTrace(); + fail(); + } catch (SegueDatabaseException e) { + e.printStackTrace(); + fail(); + } + }); } /** @@ -492,7 +487,7 @@ public final void sendRegistrationConfirmation_checkTemplatesWithNoTagsWorks_ema EmailTemplateDTO template = createDummyEmailTemplate("this is a template with no tags"); // Create content manager - GitContentManager mockContentManager = PowerMock.createMock(GitContentManager.class); + GitContentManager mockContentManager = EasyMock.createMock(GitContentManager.class); ContentDTO htmlTemplate = createDummyContentTemplate("{{content}}"); try { @@ -508,10 +503,10 @@ public final void sendRegistrationConfirmation_checkTemplatesWithNoTagsWorks_ema EasyMock.expect(mockContentManager.getCurrentContentSHA()).andReturn(CONTENT_VERSION).atLeastOnce(); - PowerMock.replay(mockContentManager); + EasyMock.replay(mockContentManager); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, @@ -524,10 +519,10 @@ public final void sendRegistrationConfirmation_checkTemplatesWithNoTagsWorks_ema emailTokens, EmailType.SYSTEM); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } catch (SegueDatabaseException e) { e.printStackTrace(); - Assert.fail(); + fail(); } // Wait for the emailQueue to spin up and send our message @@ -538,7 +533,7 @@ public final void sendRegistrationConfirmation_checkTemplatesWithNoTagsWorks_ema i++; } catch (InterruptedException e) { e.printStackTrace(); - Assert.fail(); + fail(); } } email = capturedArgument.getValue(); @@ -566,7 +561,7 @@ public void sendRegistrationConfirmation_checkNullContentDTO_exceptionThrownAndD EasyMock.replay(mockContentManager); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } EmailManager manager = new EmailManager(emailCommunicator, userPreferenceManager, mockPropertiesLoader, @@ -580,7 +575,7 @@ public void sendRegistrationConfirmation_checkNullContentDTO_exceptionThrownAndD } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } catch (SegueDatabaseException e) { e.printStackTrace(); log.info(e.getMessage()); @@ -594,7 +589,7 @@ public void sendRegistrationConfirmation_checkNullContentDTO_exceptionThrownAndD i++; } catch (InterruptedException e) { e.printStackTrace(); - Assert.fail(); + fail(); } } // We expect there to be nothing captured because the content was not returned @@ -620,7 +615,7 @@ public void sendCustomEmail_checkNullProperties_replacedWithEmptyString() { EasyMock.expect(userPreferenceManager.getUserPreference(SegueUserPreferences.EMAIL_PREFERENCE.name(), "ASSIGNMENTS", userDTOWithNulls.getId())).andReturn(userPreference); } catch (SegueDatabaseException e1) { e1.printStackTrace(); - Assert.fail(); + fail(); } EasyMock.replay(userPreferenceManager); @@ -644,15 +639,15 @@ public void sendCustomEmail_checkNullProperties_replacedWithEmptyString() { EasyMock.replay(mockContentManager); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } try { manager.sendCustomEmail(userDTOWithNulls, contentObjectId, allSelectedUsers, EmailType.ASSIGNMENTS); } catch (SegueDatabaseException e) { - Assert.fail(); + fail(); } catch (ContentManagerException e) { - Assert.fail(); + fail(); } } @@ -676,7 +671,7 @@ public void sendCustomContentEmail_checkNullProperties_replacedWithEmptyString() EasyMock.expect(userPreferenceManager.getUserPreference(SegueUserPreferences.EMAIL_PREFERENCE.name(), "ASSIGNMENTS", userDTOWithNulls.getId())).andReturn(userPreference); } catch (SegueDatabaseException e1) { e1.printStackTrace(); - Assert.fail(); + fail(); } EasyMock.replay(userPreferenceManager); @@ -703,15 +698,15 @@ public void sendCustomContentEmail_checkNullProperties_replacedWithEmptyString() EasyMock.replay(mockContentManager); } catch (ContentManagerException e) { e.printStackTrace(); - Assert.fail(); + fail(); } try { manager.sendCustomContentEmail(userDTOWithNulls, emailTemplate, allSelectedUsers, EmailType.ASSIGNMENTS); } catch (SegueDatabaseException e) { - Assert.fail(); + fail(); } catch (ContentManagerException e) { - Assert.fail(); + fail(); } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/comm/MailGunEmailManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/comm/MailGunEmailManagerTest.java index ee6f4ec76f..fbf7be05e1 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/comm/MailGunEmailManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/comm/MailGunEmailManagerTest.java @@ -19,18 +19,13 @@ import com.google.api.client.util.Maps; import com.mailgun.model.message.MessageResponse; import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.AbstractUserPreferenceManager; import uk.ac.cam.cl.dtg.isaac.dos.PgUserPreferenceManager; import uk.ac.cam.cl.dtg.isaac.dto.content.EmailTemplateDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; import uk.ac.cam.cl.dtg.segue.dao.ILogManager; -import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.util.AbstractConfigLoader; import java.util.Collection; @@ -41,9 +36,6 @@ * Test class for the EmailManager class. * */ -@RunWith(PowerMockRunner.class) -@PrepareForTest(GitContentManager.class) -@PowerMockIgnore("javax.management.*") public class MailGunEmailManagerTest { private AbstractConfigLoader mockPropertiesLoader; private AbstractUserPreferenceManager userPreferenceManager; @@ -55,7 +47,7 @@ public class MailGunEmailManagerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { // Create dummy email preferences userPreferenceManager = EasyMock.createMock(PgUserPreferenceManager.class); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/ContentSubclassMapperTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/ContentSubclassMapperTest.java index a27123e593..dfb7fb715d 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/ContentSubclassMapperTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/ContentSubclassMapperTest.java @@ -1,20 +1,20 @@ package uk.ac.cam.cl.dtg.segue.dao; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.reflections.Reflections; import uk.ac.cam.cl.dtg.isaac.dos.content.CodeSnippet; import uk.ac.cam.cl.dtg.isaac.dos.content.Content; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertTrue; public class ContentSubclassMapperTest { private ContentSubclassMapper contentSubclassMapper; - @Before + @BeforeEach public void setUp() { this.contentSubclassMapper = new ContentSubclassMapper(new Reflections("uk.ac.cam.cl.dtg.isaac")); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/GitContentManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/GitContentManagerTest.java index 8e161f7484..e685609df9 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/dao/GitContentManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/dao/GitContentManagerTest.java @@ -15,9 +15,8 @@ */ package uk.ac.cam.cl.dtg.segue.dao; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; @@ -30,14 +29,13 @@ import java.util.*; import static org.easymock.EasyMock.*; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * Test class for the GitContentManager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class GitContentManagerTest { private GitDb database; private ISearchProvider searchProvider; @@ -54,7 +52,7 @@ public class GitContentManagerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.database = createMock(GitDb.class); this.searchProvider = createMock(ISearchProvider.class); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/database/GitDbTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/database/GitDbTest.java index 7e735e933a..d81c0ea5bc 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/database/GitDbTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/database/GitDbTest.java @@ -15,16 +15,15 @@ */ package uk.ac.cam.cl.dtg.segue.database; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.fail; import java.io.IOException; import org.easymock.EasyMock; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.Repository; -import org.junit.Test; - -import org.powermock.api.easymock.PowerMock; +import org.junit.jupiter.api.Test; public class GitDbTest { @@ -32,8 +31,6 @@ public class GitDbTest { public void gitDbOtherConstructor_checkForBadParameters_exceptionsShouldBeThrown() { // Test that if you provide an empty string or null, an IllegalArgumentException gets thrown and git.open never gets called. - PowerMock.replay(Git.class); - try { new GitDb("", null, null); fail("GitDb constructor was given an empty string, but didn't throw an exception"); diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java index e7554bc716..3a2050954b 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexerTest.java @@ -16,7 +16,8 @@ * limitations under the License. */ import static org.easymock.EasyMock.*; -import static org.junit.Assert.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static uk.ac.cam.cl.dtg.segue.etl.ContentIndexer.FEEDBACK_QUESTION_UNUSED_DZ; import java.util.*; @@ -24,15 +25,12 @@ import com.google.api.client.util.Maps; import com.google.api.client.util.Sets; import com.google.common.collect.ImmutableMap; -import org.junit.Before; -import org.junit.Test; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.reflect.Whitebox; import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.IsaacNumericQuestion; -import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidator; import uk.ac.cam.cl.dtg.isaac.quiz.IsaacDndValidatorTest; import uk.ac.cam.cl.dtg.segue.api.Constants; import uk.ac.cam.cl.dtg.segue.dao.content.ContentSubclassMapper; @@ -46,7 +44,6 @@ * Test class for the GitContentManager class. * */ -@PowerMockIgnore({"jakarta.ws.*"}) public class ContentIndexerTest { private GitDb database; private ElasticSearchIndexer searchProvider; @@ -63,7 +60,7 @@ public class ContentIndexerTest { * @throws Exception * - test exception */ - @Before + @BeforeEach public final void setUp() throws Exception { this.database = createMock(GitDb.class); this.searchProvider = createMock(ElasticSearchIndexer.class); @@ -160,9 +157,8 @@ public void buildSearchIndexes_sendContentToSearchProvider_checkSearchProviderIs ContentIndexer contentIndexer = new ContentIndexer(database, searchProvider, contentSubclassMapper); // Method under test - Whitebox.invokeMethod(contentIndexer, - "buildElasticSearchIndex", - INITIAL_VERSION, contents, someTagsList, someUnitsMap, publishedUnitsMap, someContentProblemsMap); + contentIndexer.buildElasticSearchIndex(INITIAL_VERSION, contents, someTagsList, someUnitsMap, publishedUnitsMap, + someContentProblemsMap); verify(searchProvider, contentMapper, objectMapper); } @@ -182,8 +178,7 @@ public void flattenContentObjects_flattenMultiTierObject_checkCorrectObjectRetur Set elements = new HashSet<>(); Content rootNode = createContentHierarchy(numChildLevels, elements); - Set contents = Whitebox.invokeMethod( - defaultContentIndexer, "flattenContentObjects", rootNode); + Set contents = ContentIndexer.flattenContentObjects(rootNode); assertEquals(numNodes, contents.size()); @@ -214,13 +209,7 @@ public void recordContentTypeSpecificError_noSigFigSet_checkNoError() // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -246,13 +235,7 @@ public void recordContentTypeSpecificError_onlyOneSigFigSet_checkErrorIsCorrect( // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -281,13 +264,7 @@ public void recordContentTypeSpecificError_bothSigFigSetLessThan1_checkErrorIsCo // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -314,13 +291,7 @@ public void recordContentTypeSpecificError_maxLessThanMin_checkErrorIsCorrect() // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -348,13 +319,7 @@ public void recordContentTypeSpecificError_minLessThanMax_checkNoError() // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -386,13 +351,7 @@ public void recordContentTypeSpecificError_disregardSigFigsSet_checkNoError() // ACT for (Content content : contents) { - Whitebox.invokeMethod( - defaultContentIndexer, - "recordContentTypeSpecificError", - "", - content, - indexProblemCache - ); + defaultContentIndexer.recordContentTypeSpecificError("", content, indexProblemCache); } // ASSERT @@ -414,7 +373,7 @@ public void recordContentTypeSpecificError_dndQuestionCorrect_checkNoError() thr // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT @@ -435,7 +394,7 @@ public void recordContentTypeSpecificError_dndQuestionNoDropZones_checkErrorIsCo // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT @@ -459,7 +418,7 @@ public void recordContentTypeSpecificError_dndQuestionDuplicateDropZones_checkEr // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT @@ -483,7 +442,7 @@ public void recordContentTypeSpecificError_dndQuestionUnusedDropZones_checkError // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT @@ -507,7 +466,7 @@ public void recordContentTypeSpecificError_dndQuestionUnrecognisedDropZones_chec // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT @@ -533,7 +492,7 @@ public void recordContentTypeSpecificError_answerDuplicateDropZones_checkErrorIs // ACT for (Content content : contents) { - Whitebox.invokeMethod(defaultContentIndexer, "recordContentTypeSpecificError", "", content, problemCache); + defaultContentIndexer.recordContentTypeSpecificError("", content, problemCache); } // ASSERT diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ETLInMemorySshConfigStoreTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ETLInMemorySshConfigStoreTest.java index 94097a83f0..00057aca0b 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ETLInMemorySshConfigStoreTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/etl/ETLInMemorySshConfigStoreTest.java @@ -1,7 +1,6 @@ package uk.ac.cam.cl.dtg.segue.etl; -import com.google.common.collect.Sets; -import org.junit.Test; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.segue.etl.ETLInMemorySshConfigStore.InMemoryHostConfig; import java.util.Arrays; @@ -10,8 +9,8 @@ import java.util.List; import java.util.Map; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; public class ETLInMemorySshConfigStoreTest { diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java index 80beb3b3b9..c9357ed3a8 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/search/ElasticSearchProviderTest.java @@ -17,12 +17,12 @@ import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery; import co.elastic.clients.elasticsearch._types.query_dsl.Query; -import org.junit.Test; +import org.junit.jupiter.api.Test; import java.util.HashMap; import java.util.Map; -import static org.junit.Assert.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; public class ElasticSearchProviderTest { diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/util/NameFormatterTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/util/NameFormatterTest.java index 6d024a171e..c39e20b907 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/util/NameFormatterTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/util/NameFormatterTest.java @@ -1,6 +1,6 @@ package uk.ac.cam.cl.dtg.segue.util; -import org.junit.Test; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dto.UserGroupDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO; @@ -8,8 +8,8 @@ import java.util.Date; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; public class NameFormatterTest { diff --git a/src/test/java/uk/ac/cam/cl/dtg/segue/util/PostCodeLocationResolverTest.java b/src/test/java/uk/ac/cam/cl/dtg/segue/util/PostCodeLocationResolverTest.java index bdbc7875d9..8e2e2365f4 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/segue/util/PostCodeLocationResolverTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/segue/util/PostCodeLocationResolverTest.java @@ -23,14 +23,13 @@ import java.util.List; import java.util.Map; -import junit.framework.Assert; - import org.easymock.EasyMock; -import org.junit.Before; -import org.junit.Test; import com.google.api.client.util.Maps; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.database.PostgresSqlDb; import uk.ac.cam.cl.dtg.isaac.dos.LocationHistory; @@ -39,6 +38,10 @@ import uk.ac.cam.cl.dtg.util.locations.PostCodeIOLocationResolver; import uk.ac.cam.cl.dtg.util.locations.PostCodeRadius; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + /** * Test suite to check that the use of the 3rd party service postcodes.io * @@ -51,7 +54,7 @@ public class PostCodeLocationResolverTest { private PostCodeIOLocationResolver resolver; private PostgresSqlDb mockDatabase; - @Before + @BeforeEach public final void setUp() throws Exception { mockDatabase = EasyMock.createMock(PostgresSqlDb.class); locationHistory = new PgLocationHistory(mockDatabase); @@ -99,11 +102,11 @@ public void filterPostcodesWithinProximityOfPostcode_passingCorrectPostCodesOfKn List ids = resolver.filterPostcodesWithinProximityOfPostcode(map, "BD175TT", PostCodeRadius.TWENTY_FIVE_MILES); System.out.println(ids.toString()); - Assert.assertTrue(ids.contains(5l)); + assertTrue(ids.contains(5l)); } catch (LocationServerException e) { - Assert.fail(); + fail(); } catch (SegueDatabaseException e) { - Assert.fail(); + fail(); } } @@ -126,9 +129,9 @@ public void filterPostcodesWithinProximityOfPostcode_passingGoodPostCodesOutOfPr List ids = resolver.filterPostcodesWithinProximityOfPostcode(map, "CB237AN", PostCodeRadius.FIFTY_MILES); System.out.println(ids.toString()); - Assert.assertTrue(ids.size() == 1); + assertEquals(1, ids.size()); } catch (LocationServerException | SegueDatabaseException e) { - Assert.fail(); + fail(); } } @@ -150,10 +153,10 @@ public void filterPostcodesWithinProximityOfPostcode_passingBadPostCodes_ExpectE try { List ids = resolver.filterPostcodesWithinProximityOfPostcode(map, "BD175TT", PostCodeRadius.TWENTY_FIVE_MILES); - Assert.assertTrue(ids.isEmpty()); + assertTrue(ids.isEmpty()); System.out.println(ids.toString()); } catch (LocationServerException | SegueDatabaseException e) { - Assert.fail(); + fail(); } } @@ -164,10 +167,10 @@ public void filterPostcodesWithinProximityOfPostcode_passingEmptyMap_ExpectEmpty try { List ids = resolver.filterPostcodesWithinProximityOfPostcode(map, "BD175TT", PostCodeRadius.TWENTY_FIVE_MILES); - Assert.assertTrue(ids.isEmpty()); + assertTrue(ids.isEmpty()); System.out.println(ids.toString()); } catch (LocationServerException | SegueDatabaseException e) { - Assert.fail(); + fail(); } } @@ -188,7 +191,7 @@ public void filterPostcodesWithinProximityOfPostcode_passingBadTargetPostCode_Ex try { resolver.filterPostcodesWithinProximityOfPostcode(map, "46346364", PostCodeRadius.TWENTY_FIVE_MILES); - Assert.fail(); + fail(); } catch (LocationServerException | SegueDatabaseException e) { System.out.println(e.getMessage()); } @@ -199,7 +202,7 @@ public void filterPostcodesWithinProximityOfPostcode_passingBadArguments_ExpectE try { resolver.filterPostcodesWithinProximityOfPostcode(null, "", null); - Assert.fail(); + fail(); } catch (LocationServerException | SegueDatabaseException e) { System.out.println(e.getMessage()); } @@ -222,10 +225,10 @@ public void filterPostcodesWithinProximityOfPostcode_passingPostCodesWithRandomS try { List ids = resolver.filterPostcodesWithinProximityOfPostcode(map, "CB237AN", PostCodeRadius.TWENTY_FIVE_MILES); - Assert.assertTrue(ids.contains(2l)); + assertTrue(ids.contains(2l)); } catch (LocationServerException | SegueDatabaseException e) { System.out.println(e.getMessage()); - Assert.fail(); + fail(); } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GroupManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/util/NameOrdererTest.java similarity index 83% rename from src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GroupManagerTest.java rename to src/test/java/uk/ac/cam/cl/dtg/util/NameOrdererTest.java index 3b7c3b09e0..e3a1ee2eed 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/GroupManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/util/NameOrdererTest.java @@ -1,31 +1,22 @@ -package uk.ac.cam.cl.dtg.isaac.api.managers; +package uk.ac.cam.cl.dtg.util; -import org.junit.Before; -import org.junit.Test; -import uk.ac.cam.cl.dtg.segue.api.managers.GroupManager; +import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.dos.users.EmailVerificationStatus; import uk.ac.cam.cl.dtg.isaac.dos.users.Gender; import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO; import java.util.ArrayList; import java.util.Collections; +import java.util.Date; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.powermock.reflect.Whitebox; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.easymock.EasyMock.createMock; -import static org.junit.Assert.*; - -public class GroupManagerTest extends AbstractManagerTest { - - private GroupManager groupManager; - - @Before - public final void setUp() { - this.groupManager = createMock(GroupManager.class); - } +public class NameOrdererTest { + Date somePastDate = new Date(System.currentTimeMillis() - 7 * 24 * 60 * 60 * 1000); @Test public void orderUsersByName_ordersBySurnamePrimarily() throws Exception { @@ -46,10 +37,11 @@ public void orderUsersByName_ordersBySurnamePrimarily() throws Exception { ).peek(user -> user.setId((long) ("" + user.getGivenName() + user.getFamilyName()).hashCode())).collect(Collectors.toList()); List shuffledUsers = new ArrayList<>(users); - Collections.shuffle(shuffledUsers); + Collections.shuffle(shuffledUsers); assertNotEquals(users, shuffledUsers); - Whitebox.invokeMethod(groupManager, "orderUsersByName", shuffledUsers); + + NameOrderer.orderUsersByName(shuffledUsers); assertEquals(users, shuffledUsers); } } diff --git a/src/test/java/uk/ac/cam/cl/dtg/util/YamlLoaderTest.java b/src/test/java/uk/ac/cam/cl/dtg/util/YamlLoaderTest.java index df023f64cf..41b6616c4b 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/util/YamlLoaderTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/util/YamlLoaderTest.java @@ -1,24 +1,17 @@ package uk.ac.cam.cl.dtg.util; import org.easymock.EasyMock; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PowerMockIgnore; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.junit.jupiter.api.Test; import org.yaml.snakeyaml.Yaml; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import static junit.framework.TestCase.assertEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; -@RunWith(PowerMockRunner.class) -@PrepareForTest({YamlLoader.class, FileInputStream.class, Yaml.class}) -@PowerMockIgnore({"javax.xml.datatype.*", "javax.management.*", "javax.crypto.*", "javax.net.ssl.*", "javax.net.*"}) public class YamlLoaderTest { @Test public void yamlLoader_usingSingleResourceConfigFile_loadsSuccessfully() throws IOException { @@ -42,62 +35,56 @@ public void yamlLoader_usingMultipleResourceConfigFiles_firstFileValuesAreOverri @Test public void yamlLoader_usingSingleConfigFile_loadsSuccessfully() throws Exception { - // Arrange - setUpMockConfigs(); - - // Act - YamlLoader loader = new YamlLoader("/some/external/file/path.properties"); - - // Assert + YamlLoader loader = getTestableYamlLoader("/some/external/file/path.properties", false); assertEquals("src/test/resources/content_indices.test.properties", loader.getProperty("CONTENT_INDICES_LOCATION")); } @Test public void yamlLoader_usingMultipleConfigFiles_firstFileValuesAreOverriddenBySecond() throws Exception { - // Arrange - setUpMockConfigs(); - - // Act - YamlLoader loader = new YamlLoader( - "/some/external/file/path.properties,/another/external/file/path.properties" - ); - - // Assert + YamlLoader loader = getTestableYamlLoader("/some/external/file/path.properties," + + "/another/external/file/path.properties", true); assertEquals("somewhere/on/my/machine.properties", loader.getProperty("CONTENT_INDICES_LOCATION")); } - public void setUpMockConfigs() throws Exception { + private YamlLoader getTestableYamlLoader(final String paths, final boolean includeOverride) throws Exception { // Mock on-disk config files // main config file - String configPath = "/some/external/file/path.properties"; - Map mockConfig = new ConcurrentHashMap<>(); mockConfig.put("CONTENT_INDICES_LOCATION", "src/test/resources/content_indices.test.properties"); FileInputStream mockConfigFileInputStream = EasyMock.createNiceMock(FileInputStream.class); - EasyMock.replay(mockConfigFileInputStream); - - PowerMock.expectNew(FileInputStream.class, configPath).andReturn(mockConfigFileInputStream); // override config file - String overridePath = "/another/external/file/path.properties"; - Map mockOverrideConfig = new ConcurrentHashMap<>(); mockOverrideConfig.put("CONTENT_INDICES_LOCATION", "somewhere/on/my/machine.properties"); FileInputStream mockOverrideFileInputStream = EasyMock.createNiceMock(FileInputStream.class); - EasyMock.replay(mockOverrideFileInputStream); - PowerMock.expectNew(FileInputStream.class, overridePath).andReturn(mockOverrideFileInputStream); + Yaml mockYaml = EasyMock.createMock(Yaml.class); // configure mock YAML parser to return mock configs - PowerMock.replay(FileInputStream.class); - - Yaml mockYaml = EasyMock.createMock(Yaml.class); - EasyMock.expect(mockYaml.load(mockConfigFileInputStream)).andStubReturn(mockConfig); - EasyMock.expect(mockYaml.load(mockOverrideFileInputStream)).andStubReturn(mockOverrideConfig); - EasyMock.replay(mockYaml); - PowerMock.expectNew(Yaml.class).andReturn(mockYaml); - PowerMock.replay(Yaml.class); + if (includeOverride) { + EasyMock.expect(mockYaml.load(EasyMock.anyObject(InputStream.class))) + .andReturn(mockConfig) + .andReturn(mockOverrideConfig); + } else { + EasyMock.expect(mockYaml.load(EasyMock.anyObject(InputStream.class))) + .andReturn(mockConfig); + } + + EasyMock.replay(mockConfigFileInputStream, mockOverrideFileInputStream, mockYaml); + + // This cannot be a (partial) mock because we need to set configPath + return new YamlLoader(paths) { + @Override + protected Yaml createYaml() { + return mockYaml; + } + + @Override + protected FileInputStream createFileInputStream(final String path) { + return includeOverride ? mockOverrideFileInputStream : mockConfigFileInputStream; + } + }; } -} +} \ No newline at end of file diff --git a/src/test/java/uk/ac/cam/cl/dtg/util/mappers/ContentMapperTest.java b/src/test/java/uk/ac/cam/cl/dtg/util/mappers/ContentMapperTest.java index d3f5679e00..fa5bfc1910 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/util/mappers/ContentMapperTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/util/mappers/ContentMapperTest.java @@ -1,45 +1,39 @@ package uk.ac.cam.cl.dtg.util.mappers; -import org.junit.Test; import org.junit.jupiter.api.Assertions; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.reflections.Reflections; import uk.ac.cam.cl.dtg.isaac.dos.content.Content; import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentDTO; import java.lang.reflect.Modifier; -import java.util.Collection; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Parameterised unit tests verifying mapping from subclasses of {@code Content} to their corresponding * {@code ContentDTO} subclasses. */ -@RunWith(Parameterized.class) -public class ContentMapperTest { +class ContentMapperTest { private final MainMapper mapper = MainMapper.INSTANCE; - Class sourceDOClass; - Class expectedDTOClass; - /** * Builds parameterised test cases mapping each concrete {@code Content} subclass to its corresponding * {@code ContentDTO} class. * - * @return collection of {@code Content}/{@code ContentDTO} class pairings + * @return a stream of {@code Content}/{@code ContentDTO} class pairings */ - @SuppressWarnings("unchecked") - @Parameterized.Parameters - public static Collection testCasesDOtoDTO() { + static Stream testCasesDOtoDTO() { Reflections reflections = new Reflections("uk.ac.cam.cl.dtg.isaac.dos"); Set> contentSubclasses = reflections.getSubTypesOf(Content.class); contentSubclasses.add(Content.class); return contentSubclasses.stream() + .filter(subclass -> !Modifier.isAbstract(subclass.getModifiers())) .map(subclass -> { if (Modifier.isAbstract(subclass.getModifiers())) { return null; @@ -48,21 +42,15 @@ public static Collection testCasesDOtoDTO() { if (mapping == null) { return null; } - Class dtoClass = (Class) mapping.value(); - return new Object[]{subclass, dtoClass}; + return Arguments.of(subclass, mapping.value()); }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - } - - public ContentMapperTest(final Class sourceDOClass, - final Class expectedDTOClass) { - this.sourceDOClass = sourceDOClass; - this.expectedDTOClass = expectedDTOClass; + .filter(Objects::nonNull); } - @Test - public void testDOtoDTOMapping() { + @ParameterizedTest + @MethodSource("testCasesDOtoDTO") + void testDOtoDTOMapping(final Class sourceDOClass, + final Class expectedDTOClass) { Content source; try { source = sourceDOClass.getConstructor().newInstance(); diff --git a/src/test/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapperTest.java b/src/test/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapperTest.java index 67dc65d346..4b7b16addd 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapperTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/util/mappers/QuestionValidationMapperTest.java @@ -1,40 +1,32 @@ package uk.ac.cam.cl.dtg.util.mappers; -import org.junit.Test; import org.junit.jupiter.api.Assertions; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.reflections.Reflections; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.content.DTOMapping; import uk.ac.cam.cl.dtg.isaac.dto.QuestionValidationResponseDTO; -import java.lang.reflect.Modifier; -import java.util.Collection; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Parameterised unit tests verifying mapping from subclasses of {@code QuestionValidationResponse} to their * corresponding {@code QuestionValidationResponseDTO} subclasses. */ -@RunWith(Parameterized.class) public class QuestionValidationMapperTest { private final MainMapper mapper = MainMapper.INSTANCE; - Class sourceDOClass; - Class expectedDTOClass; - /** * Builds parameterised test cases mapping each {@code QuestionValidationResponse} subclass to its * corresponding {@code QuestionValidationResponseDTO} class. * - * @return collection of {@code QuestionValidationResponse}/{@code QuestionValidationResponseDTO} class pairings + * @return a stream of {@code QuestionValidationResponse}/{@code QuestionValidationResponseDTO} class pairings */ - @SuppressWarnings("unchecked") - @Parameterized.Parameters - public static Collection testCasesDOtoDTO() { + public static Stream testCasesDOtoDTO() { Reflections reflections = new Reflections("uk.ac.cam.cl.dtg.isaac.dos"); Set> subclasses = reflections.getSubTypesOf(QuestionValidationResponse.class); @@ -44,21 +36,15 @@ public static Collection testCasesDOtoDTO() { if (mapping == null) { return null; } - Class dtoClass = (Class) mapping.value(); - return new Object[]{subclass, dtoClass}; + return Arguments.of(subclass, mapping.value()); }) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - } - - public QuestionValidationMapperTest(final Class sourceDOClass, - final Class expectedDTOClass) { - this.sourceDOClass = sourceDOClass; - this.expectedDTOClass = expectedDTOClass; + .filter(Objects::nonNull); } - @Test - public void testDOtoDTOMapping() { + @ParameterizedTest + @MethodSource("testCasesDOtoDTO") + public void testDOtoDTOMapping(final Class sourceDOClass, + final Class expectedDTOClass) { QuestionValidationResponse source; try { source = sourceDOClass.getConstructor().newInstance();