acsaki commented on code in PR #12179:
URL: https://github.com/apache/kafka/pull/12179#discussion_r893299096


##########
clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticatorTest.java:
##########
@@ -126,40 +135,94 @@ public void testUnexpectedRequestType() throws 
IOException {
     @Test
     public void testOldestApiVersionsRequest() throws IOException {
         testApiVersionsRequest(ApiKeys.API_VERSIONS.oldestVersion(),
-            ClientInformation.UNKNOWN_NAME_OR_VERSION, 
ClientInformation.UNKNOWN_NAME_OR_VERSION);
+                ClientInformation.UNKNOWN_NAME_OR_VERSION, 
ClientInformation.UNKNOWN_NAME_OR_VERSION);
     }
 
     @Test
     public void testLatestApiVersionsRequest() throws IOException {
         testApiVersionsRequest(ApiKeys.API_VERSIONS.latestVersion(),
-            "apache-kafka-java", AppInfoParser.getVersion());
+                "apache-kafka-java", AppInfoParser.getVersion());
     }
 
     @Test
-    public void testExpiredCredentialLifetime() throws IOException {
+    public void 
testSessionExpirationLeftAsNullButLifetimeReturnedToTheClientWhenReauthDisabled()
 throws IOException {
         String mechanism = OAuthBearerLoginModule.OAUTHBEARER_MECHANISM;
+        Duration tokenExpirationDuration = Duration.ofSeconds(100);
         SaslServer saslServer = mock(SaslServer.class);
-        when(saslServer.getMechanismName()).thenReturn(mechanism);
-        when(saslServer.evaluateResponse(any())).thenReturn(new byte[]{});
-        
when(saslServer.getNegotiatedProperty(eq(SaslInternalConfigs.CREDENTIAL_LIFETIME_MS_SASL_NEGOTIATED_PROPERTY_KEY))).thenReturn(1L);
-        KafkaPrincipalBuilder kafkaPrincipalBuilder = 
mock(KafkaPrincipalBuilder.class);
-        when(kafkaPrincipalBuilder.build(any())).thenReturn(new 
KafkaPrincipal("[principal-type]", "[principal-name"));
+
+        MockTime time = new MockTime();

Review Comment:
   I've cleaned up tests a bit to make them more readable but leave them as 
explicit as they can be. I usually don't mind a bit relaxed DRY if tests can be 
understood as is. Also, `MockedStatic`s are supposed to be closed shorty after 
they did their thing so using them in a try-with-resources block is usually 
considered best. Any remaining redundancies in your opinion that should be 
dealt with? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to