Merge pull request #1338 from timtebeek/refactor/j-unit-5-best-practices

[SHIRO-776] refactor: JUnit5 Best Practices
apache · Mar 2, 2024 · d8014ef · d8014ef
2 parents 1739f6f + 1818385
commit d8014ef
Show file tree

Hide file tree

Showing 20 changed files with 152 additions and 138 deletions.
diff --git a/core/src/test/java/org/apache/shiro/authz/permission/DomainPermissionTest.java b/core/src/test/java/org/apache/shiro/authz/permission/DomainPermissionTest.java
@@ -25,7 +25,6 @@
 import java.util.Set;
 
 import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
@@ -44,7 +43,7 @@ void testDefaultConstructor() {
         p = new DomainPermission();
 
         // Verify domain
-        assertTrue("domain".equals(p.getDomain()));
+        assertEquals("domain", p.getDomain());
 
         // Verify actions
         set = p.getActions();

diff --git a/core/src/test/java/org/apache/shiro/concurrent/SubjectAwareExecutorServiceTest.java b/core/src/test/java/org/apache/shiro/concurrent/SubjectAwareExecutorServiceTest.java
@@ -20,7 +20,6 @@
 
 import org.apache.shiro.subject.support.SubjectRunnable;
 import org.apache.shiro.test.SecurityManagerTestSupport;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 
@@ -33,6 +32,8 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
 /**
  * Test cases for the {@link SubjectAwareExecutorService} implementation.
  */
@@ -52,7 +53,7 @@ public void testSubmitRunnable() {
 
             executor.submit(testRunnable);
             SubjectRunnable subjectRunnable = captor.getValue();
-            Assertions.assertNotNull(subjectRunnable);
+            assertNotNull(subjectRunnable);
         });
     }
 

diff --git a/core/src/test/java/org/apache/shiro/mgt/DefaultSecurityManagerTest.java b/core/src/test/java/org/apache/shiro/mgt/DefaultSecurityManagerTest.java
@@ -80,7 +80,7 @@ void testDefaultConfig() {
 
         Session session = subject.getSession();
         session.setAttribute("key", "value");
-        assertEquals(session.getAttribute("key"), "value");
+        assertEquals("value", session.getAttribute("key"));
 
         subject.logout();
 
@@ -137,7 +137,7 @@ void testSubjectReuseAfterLogout() {
         Serializable firstSessionId = session.getId();
 
         session.setAttribute("key", "value");
-        assertEquals(session.getAttribute("key"), "value");
+        assertEquals("value", session.getAttribute("key"));
 
         subject.logout();
 

diff --git a/core/src/test/java/org/apache/shiro/mgt/VMSingletonDefaultSecurityManagerTest.java b/core/src/test/java/org/apache/shiro/mgt/VMSingletonDefaultSecurityManagerTest.java
@@ -29,6 +29,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 
@@ -62,12 +63,12 @@ void testVMSingleton() {
             AuthenticationToken token = new UsernamePasswordToken("guest", "guest");
             subject.login(token);
             subject.getSession().setAttribute("key", "value");
-            assertTrue(subject.getSession().getAttribute("key").equals("value"));
+            assertEquals("value", subject.getSession().getAttribute("key"));
 
             subject = SecurityUtils.getSubject();
 
             assertTrue(subject.isAuthenticated());
-            assertTrue(subject.getSession().getAttribute("key").equals("value"));
+            assertEquals("value", subject.getSession().getAttribute("key"));
         } finally {
             sm.destroy();
             //SHIRO-270:

diff --git a/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java b/core/src/test/java/org/apache/shiro/realm/AuthorizingRealmTest.java
@@ -254,7 +254,7 @@ public Collection<Permission> resolvePermissionsInRole(String roleString) {
         authorizationInfo.addStringPermission("\t");
         authorizationInfo.addStringPermission(null);
         Collection<Permission> permissions = realm.getPermissions(authorizationInfo);
-        assertEquals(permissions.size(), 4);
+        assertEquals(4, permissions.size());
     }
 
     private void assertArrayEquals(boolean[] expected, boolean[] actual) {

diff --git a/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java b/core/src/test/java/org/apache/shiro/realm/activedirectory/ActiveDirectoryRealmTest.java
@@ -41,7 +41,6 @@
 import org.easymock.Capture;
 import org.easymock.CaptureType;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -63,7 +62,10 @@
 
 import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.is;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * Simple test case for ActiveDirectoryRealm.
@@ -108,10 +110,10 @@ void testDefaultConfig() {
 
 
         UsernamePrincipal usernamePrincipal = subject.getPrincipals().oneByType(UsernamePrincipal.class);
-        assertTrue(usernamePrincipal.getUsername().equals(USERNAME));
+        assertEquals(USERNAME, usernamePrincipal.getUsername());
 
         UserIdPrincipal userIdPrincipal = subject.getPrincipals().oneByType(UserIdPrincipal.class);
-        assertTrue(userIdPrincipal.getUserId() == USER_ID);
+        assertEquals(USER_ID, userIdPrincipal.getUserId());
 
         assertTrue(realm.hasRole(subject.getPrincipals(), ROLE));
 
@@ -148,7 +150,7 @@ public void assertExistingUserSuffix(String username, String expectedPrincipalNa
             try {
                 activeDirectoryRealm.getRoleNamesForUser(username, ldapContext);
             } catch (NamingException e) {
-                Assertions.fail("Unexpected NamingException thrown during test");
+                fail("Unexpected NamingException thrown during test");
             }
         });
 

diff --git a/core/src/test/java/org/apache/shiro/realm/jdbc/JDBCRealmTest.java b/core/src/test/java/org/apache/shiro/realm/jdbc/JDBCRealmTest.java
@@ -36,7 +36,6 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.TestInfo;
 
 import javax.sql.DataSource;
@@ -48,6 +47,10 @@
 import java.util.HashMap;
 import java.util.Optional;
 
+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 case for JDBCRealm.
@@ -259,7 +262,7 @@ void testRolePresent() throws Exception {
         Subject currentUser = builder.buildSubject();
         UsernamePasswordToken token = new UsernamePasswordToken(username, plainTextPassword);
         currentUser.login(token);
-        Assertions.assertTrue(currentUser.hasRole(testRole));
+        assertTrue(currentUser.hasRole(testRole));
     }
 
     @Test
@@ -273,7 +276,7 @@ void testRoleNotPresent() throws Exception {
         Subject currentUser = builder.buildSubject();
         UsernamePasswordToken token = new UsernamePasswordToken(username, plainTextPassword);
         currentUser.login(token);
-        Assertions.assertFalse(currentUser.hasRole("Game Overall Director"));
+        assertFalse(currentUser.hasRole("Game Overall Director"));
     }
 
     @Test
@@ -288,7 +291,7 @@ void testPermissionPresent() throws Exception {
         Subject currentUser = builder.buildSubject();
         UsernamePasswordToken token = new UsernamePasswordToken(username, plainTextPassword);
         currentUser.login(token);
-        Assertions.assertTrue(currentUser.isPermitted(testPermissionString));
+        assertTrue(currentUser.isPermitted(testPermissionString));
     }
 
     @Test
@@ -303,7 +306,7 @@ void testPermissionNotPresent() throws Exception {
         Subject currentUser = builder.buildSubject();
         UsernamePasswordToken token = new UsernamePasswordToken(username, plainTextPassword);
         currentUser.login(token);
-        Assertions.assertFalse(currentUser.isPermitted("testDomain:testTarget:specialAction"));
+        assertFalse(currentUser.isPermitted("testDomain:testTarget:specialAction"));
     }
 
     /**
@@ -357,7 +360,7 @@ protected void createDefaultSchema(String testName, boolean salted) {
             String password = sha256Hash.toHex();
             sql.executeUpdate("insert into users values ('" + username + "', '" + password + "')");
         } catch (SQLException ex) {
-            Assertions.fail("Exception creating test database");
+            fail("Exception creating test database");
         } finally {
             JdbcUtils.closeStatement(sql);
             JdbcUtils.closeConnection(conn);
@@ -393,7 +396,7 @@ protected void createSaltColumnSchema(String testName, boolean base64EncodeSalt)
                     "insert into users values ('" + username + "', '" + password + "', '"
                             + maybeBase64EncodedSalt + "')");
         } catch (SQLException ex) {
-            Assertions.fail("Exception creating test database");
+            fail("Exception creating test database");
         } finally {
             JdbcUtils.closeStatement(sql);
             JdbcUtils.closeConnection(conn);
@@ -418,7 +421,7 @@ protected void createRolesAndPermissions(DataSource ds) {
             sql.executeUpdate(
                     "insert into roles_permissions values ('" + testRole + "', '" + testPermissionString + "')");
         } catch (SQLException ex) {
-            Assertions.fail("Exception adding test role and permission");
+            fail("Exception adding test role and permission");
         } finally {
             JdbcUtils.closeStatement(sql);
             JdbcUtils.closeConnection(conn);

diff --git a/core/src/test/java/org/apache/shiro/session/mgt/DelegatingSessionTest.java b/core/src/test/java/org/apache/shiro/session/mgt/DelegatingSessionTest.java
@@ -62,7 +62,7 @@ public void sleep(long millis) {
     @Test
     void testTimeout() {
         Serializable origId = session.getId();
-        assertEquals(session.getTimeout(), AbstractSessionManager.DEFAULT_GLOBAL_SESSION_TIMEOUT);
+        assertEquals(AbstractSessionManager.DEFAULT_GLOBAL_SESSION_TIMEOUT, session.getTimeout());
         session.touch();
         session.setTimeout(100);
         assertEquals(100, session.getTimeout());

diff --git a/...test/java/org/apache/shiro/session/mgt/ExecutorServiceSessionValidationSchedulerTest.java b/...test/java/org/apache/shiro/session/mgt/ExecutorServiceSessionValidationSchedulerTest.java
@@ -20,10 +20,12 @@
 
 import org.apache.shiro.session.Session;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 @SuppressWarnings("checkstyle:MagicNumber")
 public class ExecutorServiceSessionValidationSchedulerTest {
 
@@ -47,8 +49,8 @@ void timeoutSessionValidate() throws InterruptedException {
         session.setTimeout(2000L);
         defaultSessionManager.create(session);
         Thread.sleep(5000L);
-        Assertions.assertTrue(defaultSessionManager.getActiveSessions().isEmpty());
-        Assertions.assertTrue(executorServiceSessionValidationScheduler.isEnabled());
+        assertTrue(defaultSessionManager.getActiveSessions().isEmpty());
+        assertTrue(executorServiceSessionValidationScheduler.isEnabled());
     }
 
     @Test
@@ -59,19 +61,19 @@ void stopSessionValidate() throws InterruptedException {
         Thread.sleep(1000L);
         session.stop();
         Thread.sleep(3000L);
-        Assertions.assertTrue(defaultSessionManager.getActiveSessions().isEmpty());
-        Assertions.assertTrue(executorServiceSessionValidationScheduler.isEnabled());
+        assertTrue(defaultSessionManager.getActiveSessions().isEmpty());
+        assertTrue(executorServiceSessionValidationScheduler.isEnabled());
     }
 
     @Test
     void enableSessionValidation() throws InterruptedException {
-        Assertions.assertTrue(executorServiceSessionValidationScheduler.isEnabled());
+        assertTrue(executorServiceSessionValidationScheduler.isEnabled());
         executorServiceSessionValidationScheduler.disableSessionValidation();
         Thread.sleep(2000L);
-        Assertions.assertFalse(executorServiceSessionValidationScheduler.isEnabled());
+        assertFalse(executorServiceSessionValidationScheduler.isEnabled());
         executorServiceSessionValidationScheduler.enableSessionValidation();
         Thread.sleep(2000L);
-        Assertions.assertTrue(executorServiceSessionValidationScheduler.isEnabled());
+        assertTrue(executorServiceSessionValidationScheduler.isEnabled());
     }
 
     @Test
@@ -88,8 +90,8 @@ void threadException() throws InterruptedException {
         Thread.sleep(2000L);
         session.stop();
         Thread.sleep(2000L);
-        Assertions.assertFalse(defaultSessionManager.getActiveSessions().isEmpty());
-        Assertions.assertTrue(executorServiceSessionValidationScheduler.isEnabled());
+        assertFalse(defaultSessionManager.getActiveSessions().isEmpty());
+        assertTrue(executorServiceSessionValidationScheduler.isEnabled());
     }
 
     @AfterEach

diff --git a/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java b/core/src/test/java/org/apache/shiro/subject/DelegatingSubjectTest.java
@@ -39,6 +39,7 @@
 import static org.easymock.EasyMock.createNiceMock;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -68,7 +69,7 @@ void testSessionStopThenStart() {
 
         Session session = subject.getSession();
         session.setAttribute(key, value);
-        assertTrue(session.getAttribute(key).equals(value));
+        assertEquals(session.getAttribute(key), value);
         Serializable firstSessionId = session.getId();
         assertNotNull(firstSessionId);
 
@@ -79,7 +80,7 @@ void testSessionStopThenStart() {
         assertNull(session.getAttribute(key));
         Serializable secondSessionId = session.getId();
         assertNotNull(secondSessionId);
-        assertFalse(firstSessionId.equals(secondSessionId));
+        assertNotEquals(firstSessionId, secondSessionId);
 
         subject.logout();
 
@@ -175,7 +176,7 @@ void testRunAs() {
         //assert we still have the previous (user1) principals:
         PrincipalCollection previous = subject.getPreviousPrincipals();
         assertFalse(previous == null || previous.isEmpty());
-        assertTrue(previous.getPrimaryPrincipal().equals("user1"));
+        assertEquals("user1", previous.getPrimaryPrincipal());
 
         //test the stack functionality:  While as user2, run as user3:
         subject.runAs(new SimplePrincipalCollection("user3", INI_REALM_NAME));
@@ -188,7 +189,7 @@ void testRunAs() {
         //assert we still have the previous (user2) principals in the stack:
         previous = subject.getPreviousPrincipals();
         assertFalse(previous == null || previous.isEmpty());
-        assertTrue(previous.getPrimaryPrincipal().equals("user2"));
+        assertEquals("user2", previous.getPrimaryPrincipal());
 
         //drop down to user2:
         subject.releaseRunAs();
@@ -203,7 +204,7 @@ void testRunAs() {
         //assert we still have the previous (user1) principals:
         previous = subject.getPreviousPrincipals();
         assertFalse(previous == null || previous.isEmpty());
-        assertTrue(previous.getPrimaryPrincipal().equals("user1"));
+        assertEquals("user1", previous.getPrimaryPrincipal());
 
         //drop down to original user1:
         subject.releaseRunAs();

diff --git a/core/src/test/java/org/apache/shiro/util/AntPathMatcherTests.java b/core/src/test/java/org/apache/shiro/util/AntPathMatcherTests.java
@@ -283,27 +283,27 @@ void uniqueDelimiter() {
 
     @Test
     void extractPathWithinPattern() throws Exception {
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/commit.html", "/docs/commit.html"), "");
-
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/*", "/docs/cvs/commit"), "cvs/commit");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/cvs/*.html", "/docs/cvs/commit.html"), "commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/**", "/docs/cvs/commit"), "cvs/commit");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/cvs/commit.html"), "cvs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/commit.html"), "commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/*.html", "/commit.html"), "commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/*.html", "/docs/commit.html"), "docs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("*.html", "/commit.html"), "/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("*.html", "/docs/commit.html"), "/docs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("**/*.*", "/docs/commit.html"), "/docs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("*", "/docs/commit.html"), "/docs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("**/commit.html", "/docs/cvs/other/commit.html"), "/docs/cvs/other/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/commit.html", "/docs/cvs/other/commit.html"), "cvs/other/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/**/**/**/**", "/docs/cvs/other/commit.html"), "cvs/other/commit.html");
-
-        assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/*", "/docs/cvs/commit"), "docs/cvs/commit");
-        assertEquals(pathMatcher.extractPathWithinPattern("/docs/c?s/*.html", "/docs/cvs/commit.html"), "cvs/commit.html");
-        assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/**", "/docs/cvs/commit"), "docs/cvs/commit");
-        assertEquals(pathMatcher.extractPathWithinPattern("/d?cs/**/*.html", "/docs/cvs/commit.html"), "docs/cvs/commit.html");
+        assertEquals("", pathMatcher.extractPathWithinPattern("/docs/commit.html", "/docs/commit.html"));
+
+        assertEquals("cvs/commit", pathMatcher.extractPathWithinPattern("/docs/*", "/docs/cvs/commit"));
+        assertEquals("commit.html", pathMatcher.extractPathWithinPattern("/docs/cvs/*.html", "/docs/cvs/commit.html"));
+        assertEquals("cvs/commit", pathMatcher.extractPathWithinPattern("/docs/**", "/docs/cvs/commit"));
+        assertEquals("cvs/commit.html", pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/cvs/commit.html"));
+        assertEquals("commit.html", pathMatcher.extractPathWithinPattern("/docs/**/*.html", "/docs/commit.html"));
+        assertEquals("commit.html", pathMatcher.extractPathWithinPattern("/*.html", "/commit.html"));
+        assertEquals("docs/commit.html", pathMatcher.extractPathWithinPattern("/*.html", "/docs/commit.html"));
+        assertEquals("/commit.html", pathMatcher.extractPathWithinPattern("*.html", "/commit.html"));
+        assertEquals("/docs/commit.html", pathMatcher.extractPathWithinPattern("*.html", "/docs/commit.html"));
+        assertEquals("/docs/commit.html", pathMatcher.extractPathWithinPattern("**/*.*", "/docs/commit.html"));
+        assertEquals("/docs/commit.html", pathMatcher.extractPathWithinPattern("*", "/docs/commit.html"));
+        assertEquals("/docs/cvs/other/commit.html", pathMatcher.extractPathWithinPattern("**/commit.html", "/docs/cvs/other/commit.html"));
+        assertEquals("cvs/other/commit.html", pathMatcher.extractPathWithinPattern("/docs/**/commit.html", "/docs/cvs/other/commit.html"));
+        assertEquals("cvs/other/commit.html", pathMatcher.extractPathWithinPattern("/docs/**/**/**/**", "/docs/cvs/other/commit.html"));
+
+        assertEquals("docs/cvs/commit", pathMatcher.extractPathWithinPattern("/d?cs/*", "/docs/cvs/commit"));
+        assertEquals("cvs/commit.html", pathMatcher.extractPathWithinPattern("/docs/c?s/*.html", "/docs/cvs/commit.html"));
+        assertEquals("docs/cvs/commit", pathMatcher.extractPathWithinPattern("/d?cs/**", "/docs/cvs/commit"));
+        assertEquals("docs/cvs/commit.html", pathMatcher.extractPathWithinPattern("/d?cs/**/*.html", "/docs/cvs/commit.html"));
     }
 
     @Test