[GitHub] [kafka] cadonna commented on a diff in pull request #12343: MINOR: Update unit/integration tests to work with the IBM Semeru JDK

[GitBox <gi...@apache.org>]

cadonna commented on code in PR #12343:
URL: https://github.com/apache/kafka/pull/12343#discussion_r1021245518


##########
clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java:
##########
@@ -2263,7 +2263,10 @@ public void configure(Map<String, ?> configs) {
         private Sender sender = mock(Sender.class);
         private TransactionManager transactionManager = mock(TransactionManager.class);
         private Partitioner partitioner = mock(Partitioner.class);
-        private KafkaThread ioThread = mock(KafkaThread.class);
+        private KafkaThread ioThread = new KafkaThread("Fake Kafka Producer I/O Thread", new Runnable() {
+            @Override
+            public void run() {}
+        }, true);

Review Comment:
   Is it possible to set up the mock in such a way, so that it can be used? 
   Are there native methods involved? That woulod be a reason to not use a mock since Mockito cannot mock native methods. 



##########
core/src/test/scala/kafka/security/minikdc/MiniKdc.scala:
##########
@@ -260,11 +260,17 @@ class MiniKdc(config: Properties, workDir: File) extends Logging {
   }
 
   private def refreshJvmKerberosConfig(): Unit = {
-    val klass =
-      if (Java.isIbmJdk)
-        Class.forName("com.ibm.security.krb5.internal.Config")
-      else
-        Class.forName("sun.security.krb5.Config")
+    // Newer IBM JDKs use the OpenJDK security providers so try that first
+    val klass = try {
+      Class.forName("sun.security.krb5.Config")
+    } catch {
+      case ex: Exception => {
+        if (Java.isIbmJdk)
+          Class.forName("com.ibm.security.krb5.internal.Config")
+        else
+          throw ex
+      }
+    }

Review Comment:
   Why do you not use a method `Java.isIbmJdkSemeru` here similar to what you did for the clients? I think that would improve readability and avoid the comment. 



##########
core/src/test/scala/unit/kafka/utils/JaasTestUtils.scala:
##########
@@ -119,6 +120,18 @@ object JaasTestUtils {
     }
   }
 
+  private val isIbmSecurity = try {
+      Class.forName("sun.security.krb5.Config"); false
+    } catch {
+      case ex: Exception => {
+        if (Java.isIbmJdk) {
+          Class.forName("com.ibm.security.krb5.internal.Config"); true
+        } else {
+          throw ex
+        }
+      }
+    }

Review Comment:
   See my comment above.



-- 
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-unsubscribe@kafka.apache.org

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