Migrate to junit 5

trinodb · Sep 15, 2023 · 1e31ca4 · 1e31ca4
1 parent 7900850
commit 1e31ca4
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 51 deletions.
diff --git a/gateway-ha/pom.xml b/gateway-ha/pom.xml
@@ -148,6 +148,12 @@
             <version>42.6.0</version>
         </dependency>
         <!-- Test deps -->
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.testng</groupId>
             <artifactId>testng</artifactId>

diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/LbLdapClientTest.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/LbLdapClientTest.java
@@ -1,5 +1,7 @@
 package io.trino.gateway.ha.security;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.eq;
 
@@ -8,17 +10,16 @@
 import org.apache.directory.api.ldap.model.message.SearchScope;
 import org.apache.directory.ldap.client.template.LdapConnectionTemplate;
 import org.apache.directory.ldap.client.template.exception.PasswordException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
-import org.testng.Assert;
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
 
 @Slf4j
-public class LbLdapClientTest extends junit.framework.TestCase {
+public class LbLdapClientTest {
   class DummyPasswordWarning implements org.apache.directory.ldap.client.template.PasswordWarning {
     @Override
     public int getTimeBeforeExpiration() {
@@ -48,21 +49,21 @@ public boolean isChangeAfterReset() {
 
   }
 
-  @BeforeMethod
+  @BeforeEach
   public void initMocks() {
     log.info("initializing test");
     ldapConfig = LdapConfiguration.load("src/test/resources/auth/ldapTestConfig.yml");
     org.mockito.MockitoAnnotations.initMocks(this);
   }
 
-  @AfterMethod
+  @AfterEach
   public void resetMocks() {
     log.info("resetting mocks");
     Mockito.reset(ldapConnectionTemplate);
     Mockito.reset(ldapConfig);
   }
 
-  @Test(enabled = false)
+  @Test
   public void testAuthenticate() {
     String user = "user1";
     String password = "pass1";
@@ -78,7 +79,7 @@ public void testAuthenticate() {
           .thenReturn(null);
 
       //Success case
-      Assert.assertTrue(lbLdapClient.authenticate(user, password));
+      assertTrue(lbLdapClient.authenticate(user, password));
 
       Mockito
           .when(ldapConnectionTemplate.authenticate(ldapConfig.getLdapUserBaseDn(),
@@ -88,7 +89,7 @@ public void testAuthenticate() {
           .thenReturn(new LbLdapClientTest.DummyPasswordWarning());
 
       //Warning case
-      Assert.assertTrue(lbLdapClient.authenticate(user, password));
+      assertTrue(lbLdapClient.authenticate(user, password));
 
 
       Mockito
@@ -99,7 +100,7 @@ public void testAuthenticate() {
           .thenThrow(PasswordException.class);
 
       //failure case
-      Assert.assertFalse(lbLdapClient.authenticate(user, password));
+      assertFalse(lbLdapClient.authenticate(user, password));
 
       Mockito
           .when(ldapConnectionTemplate.authenticate(ldapConfig.getLdapUserBaseDn(),
@@ -112,11 +113,11 @@ public void testAuthenticate() {
     } catch (PasswordException ex) {
       log.error("This should not fail");
       //Force the test to fail
-      Assert.assertFalse(false);
+      assertFalse(false);
     }
   }
 
-  @Test(enabled = false)
+  @Test
   public void testMemberof() {
     String user = "user1";
     String[] attributes = new String[]{"memberOf"};
@@ -137,7 +138,7 @@ public void testMemberof() {
     String ret = lbLdapClient.getMemberOf(user);
 
     log.info("ret is {}", ret);
-    Assert.assertTrue(ret.equals("Admin,User"));
+    assertTrue(ret.equals("Admin,User"));
 
     org.mockito.Mockito
         .when(ldapConnectionTemplate.search(eq(ldapConfig.getLdapUserBaseDn()),
@@ -148,7 +149,7 @@ public void testMemberof() {
         .thenReturn(null);
 
     //failure case
-    Assert.assertFalse(lbLdapClient.getMemberOf(user).equals("Admin,User"));
+    assertFalse(lbLdapClient.getMemberOf(user).equals("Admin,User"));
 
   }
 }
diff --git a/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java b/gateway-ha/src/test/java/io/trino/gateway/ha/security/TestLbFilter.java
@@ -1,5 +1,9 @@
 package io.trino.gateway.ha.security;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 import com.auth0.jwt.interfaces.Claim;
 import io.trino.gateway.ha.config.AuthorizationConfiguration;
 import java.util.Map;
@@ -12,16 +16,14 @@
 import javax.ws.rs.core.SecurityContext;
 
 import lombok.extern.slf4j.Slf4j;
-import org.junit.Assert;
-import org.junit.Ignore;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
-import org.testng.annotations.BeforeClass;
-
 
 @Slf4j
-@Ignore
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 public class TestLbFilter {
 
   private static final String USER = "username";
@@ -32,7 +34,7 @@ public class TestLbFilter {
   private AuthorizationManager authorizationManager;
   private ContainerRequestContext requestContext;
 
-  @BeforeClass(alwaysRun = true)
+  @BeforeAll
   public void setup() throws Exception {
 
     // Set authentication manager mock with 'sub' claim
@@ -44,12 +46,23 @@ public void setup() throws Exception {
     Mockito
         .when(oauthManager.getClaimsFromIdToken(ID_TOKEN))
         .thenReturn(Optional.of(Map.of("sub", claim)));
+    Mockito.when(oauthManager.getUserIdField()).thenReturn("sub");
 
     // Set authorization manager with membership
     authorizationManager = Mockito.mock(AuthorizationManager.class);
     Mockito
         .when(authorizationManager.searchMemberOf(USER))
         .thenReturn(MEMBER_OF);
+    Mockito
+            .when(authorizationManager.getPrivileges(USER))
+            .thenReturn(MEMBER_OF);
+    /*
+    This is required if LbAuthenticator.authenticate uses
+    `.map(sub -> new LbPrincipal(sub, authorizationManager.getPrivileges(sub)));`
+    If it should actually be using
+    LbPrincipal(sub, authorizationManager.getMemberOf(sub)))
+    then this should be removed
+    */
 
     // Request context for the auth filter
     requestContext = Mockito.mock(ContainerRequestContext.class);
@@ -71,9 +84,11 @@ public void testSuccessfulCookieAuthentication() throws Exception {
     Mockito
         .when(requestContext.getHeaders())
         .thenReturn(new MultivaluedHashMap());
+
     LbAuthenticator authenticator = new LbAuthenticator(
         oauthManager,
         authorizationManager);
+
     LbAuthorizer authorizer = new LbAuthorizer(configuration);
     LbFilter<LbPrincipal> lbFilter = new LbFilter.Builder<LbPrincipal>()
         .setAuthenticator(authenticator)
@@ -91,8 +106,8 @@ public void testSuccessfulCookieAuthentication() throws Exception {
         .setSecurityContext(secContextCaptor.capture());
 
     // Checks authorization for authenticated principal
-    Assert.assertTrue(secContextCaptor.getValue().isUserInRole("USER"));
-    Assert.assertFalse(secContextCaptor.getValue().isUserInRole("ADMIN"));
+    assertTrue(secContextCaptor.getValue().isUserInRole("USER"));
+    assertFalse(secContextCaptor.getValue().isUserInRole("ADMIN"));
   }
 
   @Test
@@ -126,38 +141,41 @@ public void testSuccessfulHeaderAuthentication() throws Exception {
     lbFilter.filter(requestContext);
 
     // SecurityContext must be set with the right authorizer at authentication
-    Mockito.verify(requestContext, Mockito.times(1)).setSecurityContext(secContextCaptor.capture());
+    Mockito.verify(requestContext, Mockito.atLeast(1))
+            .setSecurityContext(secContextCaptor.capture());
 
     // Checks authorization for authenticated principal
-    Assert.assertTrue(secContextCaptor.getValue().isUserInRole("USER"));
-    Assert.assertTrue(secContextCaptor.getValue().isUserInRole("ADMIN"));
+    assertTrue(secContextCaptor.getValue().isUserInRole("USER"));
+    assertTrue(secContextCaptor.getValue().isUserInRole("ADMIN"));
 
   }
 
-  @Test(expected = WebApplicationException.class)
+  @Test
   public void testMissingAuthenticationToken() throws WebApplicationException {
-    AuthorizationConfiguration configuration = new AuthorizationConfiguration();
-
-    MultivaluedHashMap<String, String> headers = new MultivaluedHashMap<>();
-
-    Mockito
-        .when(requestContext.getCookies())
-        .thenReturn(Map.of());
-    Mockito
-        .when(requestContext.getHeaders())
-        .thenReturn(headers);
-    LbAuthenticator authenticator = new LbAuthenticator(
-        oauthManager,
-        authorizationManager);
-    LbAuthorizer authorizer = new LbAuthorizer(configuration);
-    LbFilter<LbPrincipal> lbFilter = new LbFilter.Builder<LbPrincipal>()
-        .setAuthenticator(authenticator)
-        .setAuthorizer(authorizer)
-        .setPrefix("Bearer")
-        .buildAuthFilter();
-
-    // Exception is thrown when the authentication fails
-    lbFilter.filter(requestContext);
-
+    assertThrows(WebApplicationException.class, () -> {
+
+      AuthorizationConfiguration configuration = new AuthorizationConfiguration();
+
+      MultivaluedHashMap<String, String> headers = new MultivaluedHashMap<>();
+
+      Mockito
+              .when(requestContext.getCookies())
+              .thenReturn(Map.of());
+      Mockito
+              .when(requestContext.getHeaders())
+              .thenReturn(headers);
+      LbAuthenticator authenticator = new LbAuthenticator(
+              oauthManager,
+              authorizationManager);
+      LbAuthorizer authorizer = new LbAuthorizer(configuration);
+      LbFilter<LbPrincipal> lbFilter = new LbFilter.Builder<LbPrincipal>()
+              .setAuthenticator(authenticator)
+              .setAuthorizer(authorizer)
+              .setPrefix("Bearer")
+              .buildAuthFilter();
+
+      // Exception is thrown when the authentication fails
+      lbFilter.filter(requestContext);
+    });
   }
 }
diff --git a/pom.xml b/pom.xml
@@ -80,11 +80,20 @@
             </dependency>
 
             <!-- Test deps -->
+            <!-- https://mvnrepository.com/artifact/org.junit.jupiter/junit-jupiter-api -->
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-api</artifactId>
+                <version>5.3.2</version>
+                <scope>test</scope>
+            </dependency>
+
             <dependency>
                 <groupId>org.testng</groupId>
                 <artifactId>testng</artifactId>
                 <version>${testng.version}</version>
             </dependency>
+
             <dependency>
                 <groupId>com.squareup.okhttp</groupId>
                 <artifactId>mockwebserver</artifactId>