Skip to content

Commit

Permalink
Merge pull request #1452 from lprimak/assert-j-best-practices
Browse files Browse the repository at this point in the history
Refactor: AssertJ best practices by @timtebeek
  • Loading branch information
lprimak committed May 3, 2024
2 parents e05ce03 + 27fc6a3 commit bb0078f
Show file tree
Hide file tree
Showing 104 changed files with 1,551 additions and 1,624 deletions.
27 changes: 14 additions & 13 deletions core/src/test/java/org/apache/shiro/SecurityUtilsUnwrapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@
import static org.apache.shiro.SecurityUtils.getSecurityManager;
import static org.apache.shiro.SecurityUtils.isSecurityManagerTypeOf;
import static org.apache.shiro.SecurityUtils.unwrapSecurityManager;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;

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.Mockito.mockStatic;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
Expand Down Expand Up @@ -78,73 +79,73 @@ public <SM extends SecurityManager> SM unwrap() {
@Test
void basicUnwrap() {
SecurityManager sm = unwrapSecurityManager(securityManager, SecurityManager.class);
assertEquals(securityManager, sm);
assertThat(sm).isEqualTo(securityManager);
}

@Test
void basicTypeCheck() {
assertTrue(isSecurityManagerTypeOf(securityManager, SecurityManager.class));
assertThat(isSecurityManagerTypeOf(securityManager, SecurityManager.class)).isTrue();
}

@Test
void securityManager() {
try (var threadContext = mockStatic(ThreadContext.class)) {
threadContext.when(ThreadContext::getSecurityManager).thenReturn(defaultSecurityManager);
DefaultSecurityManager dsm = getSecurityManager(DefaultSecurityManager.class);
assertEquals(defaultSecurityManager, dsm);
assertThat(dsm).isEqualTo(defaultSecurityManager);
}
}

@Test
void failedTypeUnwrap() {
assertThrows(ClassCastException.class, () -> {
assertThatExceptionOfType(ClassCastException.class).isThrownBy(() -> {
SessionsSecurityManager ssm = unwrapSecurityManager(securityManager, SessionsSecurityManager.class);
});
}

@Test
void defaultSecurityManager() {
var dsm = unwrapSecurityManager(defaultSecurityManager, DefaultSecurityManager.class);
assertEquals(defaultSecurityManager, dsm);
assertThat(dsm).isEqualTo(defaultSecurityManager);
when(defaultSecurityManager.createSubject(subjectContext)).thenReturn(subject);
Subject subject = dsm.createSubject(subjectContext);
assertEquals(this.subject, subject);
assertThat(subject).isEqualTo(this.subject);
verify(defaultSecurityManager).createSubject(subjectContext);
verifyNoMoreInteractions(defaultSecurityManager, this.subject, subjectContext);
}

@Test
void invalidCast() {
SecurityManager wrapped = new Wrapped(defaultSecurityManager);
assertThrows(ClassCastException.class, () -> {
assertThatExceptionOfType(ClassCastException.class).isThrownBy(() -> {
DefaultSecurityManager sm = (DefaultSecurityManager) wrapped;
});
}

@Test
void unwrapOne() {
SecurityManager wrapped = new Wrapped(defaultSecurityManager);
assertEquals(defaultSecurityManager, unwrapSecurityManager(wrapped, DefaultSecurityManager.class));
assertThat(unwrapSecurityManager(wrapped, DefaultSecurityManager.class)).isEqualTo(defaultSecurityManager);
}

@Test
void unwrapTwo() {
SecurityManager wrapped = new Wrapped(new Wrapped(defaultSecurityManager));
assertEquals(defaultSecurityManager, unwrapSecurityManager(wrapped, DefaultSecurityManager.class));
assertThat(unwrapSecurityManager(wrapped, DefaultSecurityManager.class)).isEqualTo(defaultSecurityManager);
}

@Test
void invalidWrap() {
SecurityManager wrapped = new Wrapped(new InvalidWrapped(defaultSecurityManager));
assertThrows(IllegalStateException.class, () -> {
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> {
assertEquals(defaultSecurityManager, unwrapSecurityManager(wrapped, DefaultSecurityManager.class));
});
}

@Test
void invalidWrapInverted() {
SecurityManager wrapped = new InvalidWrapped(new Wrapped(defaultSecurityManager));
assertThrows(IllegalStateException.class, () -> {
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> {
assertEquals(defaultSecurityManager, unwrapSecurityManager(wrapped, DefaultSecurityManager.class));
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
*/
package org.apache.shiro.aop;

import static org.assertj.core.api.Assertions.assertThat;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import java.lang.reflect.Method;

Expand Down Expand Up @@ -53,7 +52,7 @@ void testAnnotationFoundFromClass() throws SecurityException, NoSuchMethodExcept
expect(methodInvocation.getMethod()).andReturn(method);
expect(methodInvocation.getThis()).andReturn(myFixture);
replay(methodInvocation);
assertNotNull(annotationResolver.getAnnotation(methodInvocation, RequiresRoles.class));
assertThat(annotationResolver.getAnnotation(methodInvocation, RequiresRoles.class)).isNotNull();
}

@Test
Expand All @@ -62,7 +61,7 @@ void testAnnotationFoundFromMethod() throws SecurityException, NoSuchMethodExcep
Method method = MyFixture.class.getDeclaredMethod("operateThat");
expect(methodInvocation.getMethod()).andReturn(method);
replay(methodInvocation);
assertNotNull(annotationResolver.getAnnotation(methodInvocation, RequiresUser.class));
assertThat(annotationResolver.getAnnotation(methodInvocation, RequiresUser.class)).isNotNull();
}

@Test
Expand All @@ -72,7 +71,7 @@ void testNullMethodInvocation() throws SecurityException, NoSuchMethodException
expect(methodInvocation.getMethod()).andReturn(method);
expect(methodInvocation.getThis()).andReturn(null);
replay(methodInvocation);
assertNull(annotationResolver.getAnnotation(methodInvocation, RequiresUser.class));
assertThat(annotationResolver.getAnnotation(methodInvocation, RequiresUser.class)).isNull();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,12 @@

import java.net.URI;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* @since 0.1
Expand Down Expand Up @@ -98,7 +96,7 @@ protected AuthenticationInfo doAuthenticate(AuthenticationToken token) throws Au
*/
@Test
void authenticateWithNullArgument() {
assertThrows(IllegalArgumentException.class, () -> {
assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> {
abstractAuthenticator.authenticate(null);
});
}
Expand All @@ -109,7 +107,7 @@ void authenticateWithNullArgument() {
*/
@Test
void throwAuthenticationExceptionIfDoAuthenticateReturnsNull() {
assertThrows(AuthenticationException.class, () -> {
assertThatExceptionOfType(AuthenticationException.class).isThrownBy(() -> {
abstractAuthenticator = createAuthcReturnNull();
abstractAuthenticator.authenticate(newToken());
});
Expand All @@ -123,7 +121,7 @@ void throwAuthenticationExceptionIfDoAuthenticateReturnsNull() {
@Test
void nonNullAuthenticationInfoAfterAuthenticate() {
AuthenticationInfo authcInfo = abstractAuthenticator.authenticate(newToken());
assertNotNull(authcInfo);
assertThat(authcInfo).isNotNull();
}

@Test
Expand Down Expand Up @@ -160,7 +158,7 @@ protected AuthenticationInfo doAuthenticate(AuthenticationToken token) throws Au
abstractAuthenticator.authenticate(token);
} catch (AuthenticationException e) {
exceptionThrown = true;
assertEquals(e, ae);
assertThat(ae).isEqualTo(e);
}
verify(mockListener);

Expand All @@ -171,7 +169,7 @@ protected AuthenticationInfo doAuthenticate(AuthenticationToken token) throws Au

@Test
void notifyFailureAfterDoAuthenticateThrowsNonAuthenticationException() {
assertThrows(AuthenticationException.class, () -> {
assertThatExceptionOfType(AuthenticationException.class).isThrownBy(() -> {
abstractAuthenticator = new AbstractAuthenticator() {
protected AuthenticationInfo doAuthenticate(AuthenticationToken token) throws AuthenticationException {
throw new IllegalArgumentException("not an AuthenticationException subclass");
Expand Down Expand Up @@ -202,8 +200,8 @@ protected AuthenticationInfo doAuthenticate(AuthenticationToken token) throws Au
}

String logMsg = String.join("\n", listAppender.getMessages());
assertTrue(logMsg.contains("WARN"));
assertTrue(logMsg.contains("java.lang.IllegalArgumentException: " + expectedExceptionMessage));
assertThat(logMsg).contains("WARN");
assertThat(logMsg).contains("java.lang.IllegalArgumentException: " + expectedExceptionMessage);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import java.util.Set;

import org.apache.shiro.subject.PrincipalCollection;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.jupiter.api.Test;


/**
Expand Down Expand Up @@ -106,7 +106,7 @@ public Iterator iterator() {
aggregate.setPrincipals(principalCollection);
SimpleAuthenticationInfo local = new SimpleAuthenticationInfo("username", "password", "testRealm");
aggregate.merge(local);
assertEquals(2, aggregate.getPrincipals().asList().size());
assertThat(aggregate.getPrincipals().asList()).hasSize(2);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@

import org.apache.shiro.authc.AuthenticationInfo;

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

import org.apache.shiro.authc.AuthenticationToken;
import org.apache.shiro.authc.SimpleAuthenticationInfo;
import org.apache.shiro.authc.UsernamePasswordToken;
import org.apache.shiro.crypto.hash.SimpleHash;
import org.apache.shiro.lang.util.ClassUtils;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @since Jun 10, 2008 4:47:09 PM
*/
Expand All @@ -44,6 +44,6 @@ void testBasic() {
byte[] hashed = hash("password").getBytes();
AuthenticationInfo account = new SimpleAuthenticationInfo("username", hashed, "realmName");
AuthenticationToken token = new UsernamePasswordToken("username", "password");
assertTrue(matcher.doCredentialsMatch(token, account));
assertThat(matcher.doCredentialsMatch(token, account)).isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.assertj.core.api.Assertions.assertThat;


/**
Expand All @@ -30,7 +30,7 @@ public class AllowAllCredentialsMatcherTest {

@Test
void testBasic() {
assertTrue(new AllowAllCredentialsMatcher().doCredentialsMatch(null, null));
assertThat(new AllowAllCredentialsMatcher().doCredentialsMatch(null, null)).isTrue();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import org.apache.shiro.subject.SimplePrincipalCollection;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.assertj.core.api.Assertions.assertThat;

/**
* Tests for the {@link org.apache.shiro.authc.credential.HashedCredentialsMatcher} class.
Expand All @@ -54,7 +54,7 @@ void testSaltedAuthenticationInfo() {
AuthenticationToken token = new UsernamePasswordToken("username", "password");

//verify the hashed token matches what is in the account:
assertTrue(matcher.doCredentialsMatch(token, account));
assertThat(matcher.doCredentialsMatch(token, account)).isTrue();
}

/**
Expand Down Expand Up @@ -87,7 +87,7 @@ public Object getCredentials() {
AuthenticationToken token = new UsernamePasswordToken("username", "password");

//verify the hashed token matches what is in the account:
assertTrue(matcher.doCredentialsMatch(token, account));
assertThat(matcher.doCredentialsMatch(token, account)).isTrue();
}

/**
Expand Down Expand Up @@ -124,6 +124,6 @@ public Object getCredentials() {
AuthenticationToken token = new UsernamePasswordToken("username", "password");

//verify the hashed token matches what is in the account:
assertTrue(matcher.doCredentialsMatch(token, account));
assertThat(matcher.doCredentialsMatch(token, account)).isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import org.junit.jupiter.api.Test;
import org.apache.shiro.authc.AuthenticationException;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;

import org.apache.shiro.authc.AuthenticationInfo;
import org.apache.shiro.authc.AuthenticationToken;
Expand All @@ -46,7 +46,7 @@ public void setUp() {
@Test
void beforeAllAttempts() {
AuthenticationInfo info = strategy.beforeAllAttempts(null, null);
assertNotNull(info);
assertThat(info).isNotNull();
}

@Test
Expand All @@ -56,7 +56,7 @@ void beforeAttemptSupportingToken() {

@Test
void beforeAttemptRealmDoesntSupportToken() {
assertThrows(UnsupportedTokenException.class, () -> {
assertThatExceptionOfType(UnsupportedTokenException.class).isThrownBy(() -> {
Realm notSupportingRealm = new AuthorizingRealm() {

public boolean supports(AuthenticationToken token) {
Expand Down

0 comments on commit bb0078f

Please sign in to comment.