Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-06-01 Thread via GitHub


chia7712 merged PR #15862:
URL: https://github.com/apache/kafka/pull/15862


-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-06-01 Thread via GitHub


TaiJuWu commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1623180599


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -17,29 +17,64 @@
 
 package kafka.test.junit;
 
+import kafka.test.ClusterConfig;
 import kafka.test.annotation.ClusterTemplate;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static List cfgEmpty() {
+return Collections.emptyList();
+}
+
+@SuppressWarnings({"unchecked", "rawtypes"})
+private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {
+ExtensionContext extensionContext = mock(ExtensionContext.class);
+Class clazz = ClusterTestExtensionsUnitTest.class;
+Method method = clazz.getDeclaredMethod(methodName);
+when(extensionContext.getRequiredTestClass()).thenReturn(clazz);
+when(extensionContext.getRequiredTestMethod()).thenReturn(method);
+return extensionContext;
+}
+
 @Test
-void testProcessClusterTemplate() {
+void testProcessClusterTemplate() throws Exception {
 ClusterTestExtensions ext = new ClusterTestExtensions();
-ExtensionContext context = mock(ExtensionContext.class);
+ExtensionContext context = buildExtensionContext("cfgEmpty");
 
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("cfgEmpty");
+
+Assertions.assertEquals(

Review Comment:
   Yes, but the first one is for `@clusterTemplate("")`, the other is 
`@ClusterTemplate(" ")`.



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-31 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1622894813


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -17,29 +17,64 @@
 
 package kafka.test.junit;
 
+import kafka.test.ClusterConfig;
 import kafka.test.annotation.ClusterTemplate;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
+
+import java.lang.reflect.Method;
+import java.util.Collections;
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static List cfgEmpty() {
+return Collections.emptyList();
+}
+
+@SuppressWarnings({"unchecked", "rawtypes"})
+private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {
+ExtensionContext extensionContext = mock(ExtensionContext.class);
+Class clazz = ClusterTestExtensionsUnitTest.class;
+Method method = clazz.getDeclaredMethod(methodName);
+when(extensionContext.getRequiredTestClass()).thenReturn(clazz);
+when(extensionContext.getRequiredTestMethod()).thenReturn(method);
+return extensionContext;
+}
+
 @Test
-void testProcessClusterTemplate() {
+void testProcessClusterTemplate() throws Exception {
 ClusterTestExtensions ext = new ClusterTestExtensions();
-ExtensionContext context = mock(ExtensionContext.class);
+ExtensionContext context = buildExtensionContext("cfgEmpty");
 
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("cfgEmpty");
+
+Assertions.assertEquals(

Review Comment:
   This test is equal to next one, right?



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-29 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1619265047


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -17,29 +17,64 @@
 
 package kafka.test.junit;
 
+import kafka.test.ClusterConfig;
 import kafka.test.annotation.ClusterTemplate;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static List cfgEmpty() {
+return new ArrayList();

Review Comment:
   `Collections.emptyList()`



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-28 Thread via GitHub


chia7712 commented on PR #15862:
URL: https://github.com/apache/kafka/pull/15862#issuecomment-2134588650

   @TaiJuWu Could you please rebase code?


-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-27 Thread via GitHub


TaiJuWu commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1616328324


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -17,29 +17,66 @@
 
 package kafka.test.junit;
 
+import kafka.test.ClusterConfig;
 import kafka.test.annotation.ClusterTemplate;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static class StubTest {

Review Comment:
   In my mind, keeping the argument will make this function reusable if needed.



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-27 Thread via GitHub


soarez commented on PR #15862:
URL: https://github.com/apache/kafka/pull/15862#issuecomment-2133610600

   @TaiJuWu sorry it has taken me a while to get back to this.
   
   I agree with the simplification @chia7712 I suggesting. 


-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-27 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1615991000


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -17,29 +17,66 @@
 
 package kafka.test.junit;
 
+import kafka.test.ClusterConfig;
 import kafka.test.annotation.ClusterTemplate;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
+
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static class StubTest {

Review Comment:
   It seems this nested class is unnecessary. For example:
   
   ```java
   static List cfgEmpty() {
   return new ArrayList();
   }
   
   @SuppressWarnings({"unchecked", "rawtypes"})
   private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {
   ExtensionContext extensionContext = mock(ExtensionContext.class);
   Class clazz = ClusterTestExtensionsUnitTest.class;
   Method method = clazz.getDeclaredMethod(methodName);
   when(extensionContext.getRequiredTestClass()).thenReturn(clazz);
   when(extensionContext.getRequiredTestMethod()).thenReturn(method);
   return extensionContext;
   }
   ```
   
   BTW, `methodName` is always "cfgEmpty", so maybe we can remove it from input 
argument.



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-23 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1611501044


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -33,16 +31,22 @@ public class ClusterTestExtensionsUnitTest {
 void testProcessClusterTemplate() {
 ClusterTestExtensions ext = new ClusterTestExtensions();
 ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("test_empty_config");

Review Comment:
   ok, we have many checks to make the invalid annotation failed. Maybe we go 
back to test each helper methods instead? 



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-23 Thread via GitHub


TaiJuWu commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1611087227


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -33,16 +31,22 @@ public class ClusterTestExtensionsUnitTest {
 void testProcessClusterTemplate() {
 ClusterTestExtensions ext = new ClusterTestExtensions();
 ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("test_empty_config");

Review Comment:
   @soarez It seems not work because tester will test 
`@ClusterTemplate("cfgFoo")` and `@ClusterTemplate("")` and these method will 
make test fail.
   Please look at https://ge.apache.org/s/yyivyrph2oas6, thanks.



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-22 Thread via GitHub


TaiJuWu commented on PR #15862:
URL: https://github.com/apache/kafka/pull/15862#issuecomment-2124566645

   @soarez @chia7712 Please review, thanks a lot.


-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-09 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1596214008


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -18,31 +18,53 @@
 package kafka.test.junit;
 
 import kafka.test.annotation.ClusterTemplate;
+import kafka.test.ClusterGenerator;
 
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.lang.reflect.Method;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static class StubTest {
+@ClusterTemplate("cfgFoo")
+void testFoo() {}
+static void cfgFoo(ClusterGenerator gen) { /* ... */ }
+
+@ClusterTemplate("")
+void testBar() {}
+
+};
+
+private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {

Review Comment:
   please add `@SuppressWarnings({"unchecked", "rawtypes"})`



##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -18,31 +18,53 @@
 package kafka.test.junit;
 
 import kafka.test.annotation.ClusterTemplate;
+import kafka.test.ClusterGenerator;
 
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.lang.reflect.Method;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static class StubTest {
+@ClusterTemplate("cfgFoo")
+void testFoo() {}
+static void cfgFoo(ClusterGenerator gen) { /* ... */ }
+
+@ClusterTemplate("")
+void testBar() {}
+
+};
+
+private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {
+ExtensionContext extensionContext = mock(ExtensionContext.class);
+Class clazz = StubTest.class;
+Method method = clazz.getDeclaredMethod(methodName);
+when(extensionContext.getRequiredTestClass()).thenReturn(clazz);
+when(extensionContext.getRequiredTestMethod()).thenReturn(method);
+return extensionContext;
+}
 @Test
 @SuppressWarnings("unchecked")

Review Comment:
   this is unused now.



##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -18,31 +18,53 @@
 package kafka.test.junit;
 
 import kafka.test.annotation.ClusterTemplate;
+import kafka.test.ClusterGenerator;
 
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.lang.reflect.Method;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 public class ClusterTestExtensionsUnitTest {
+
+static class StubTest {
+@ClusterTemplate("cfgFoo")
+void testFoo() {}
+static void cfgFoo(ClusterGenerator gen) { /* ... */ }
+
+@ClusterTemplate("")
+void testBar() {}
+
+};
+
+private ExtensionContext buildExtensionContext(String methodName) throws 
Exception {
+ExtensionContext extensionContext = mock(ExtensionContext.class);
+Class clazz = StubTest.class;
+Method method = clazz.getDeclaredMethod(methodName);
+when(extensionContext.getRequiredTestClass()).thenReturn(clazz);
+when(extensionContext.getRequiredTestMethod()).thenReturn(method);
+return extensionContext;
+}
 @Test
 @SuppressWarnings("unchecked")
 void testProcessClusterTemplate() {
-ClusterTestExtensions ext = new ClusterTestExtensions();
-ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
-ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
-
-Assertions.assertThrows(IllegalStateException.class, () ->
-ext.processClusterTemplate(context, annot, testInvocations)
+ClusterTestExtensions clusterTestExtensions = new 
ClusterTestExtensions();
+
+assertEquals("ClusterConfig generator method should provide at least 
one config.",

Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-06 Thread via GitHub


TaiJuWu commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1591630803


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -33,16 +31,22 @@ public class ClusterTestExtensionsUnitTest {
 void testProcessClusterTemplate() {
 ClusterTestExtensions ext = new ClusterTestExtensions();
 ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("test_empty_config");

Review Comment:
   @soarez Thanks for your comments. All is vert excellent!!! Thanks!



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-05 Thread via GitHub


chia7712 commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1590454147


##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -33,16 +31,22 @@ public class ClusterTestExtensionsUnitTest {
 void testProcessClusterTemplate() {
 ClusterTestExtensions ext = new ClusterTestExtensions();
 ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("test_empty_config");

Review Comment:
   That is a good way. Also, we can do the refactor in 
https://issues.apache.org/jira/browse/KAFKA-16654 later.



-- 
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



Re: [PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-05 Thread via GitHub


soarez commented on code in PR #15862:
URL: https://github.com/apache/kafka/pull/15862#discussion_r1590420247


##
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java:
##
@@ -119,23 +116,34 @@ public Stream 
provideTestTemplateInvocationContex
 return generatedContexts.stream();
 }
 
-void processClusterTemplate(ExtensionContext context, ClusterTemplate 
annot,
-
Consumer testInvocations) {
+List 
processClusterTemplate(ExtensionContext context, ClusterTemplate annot) {
 // If specified, call cluster config generated method (must be static)
 List generatedClusterConfigs = new ArrayList<>();
+List testTemplateInvocationContexts = 
new ArrayList<>();
 if (annot.value().trim().isEmpty()) {
 throw new IllegalStateException("ClusterTemplate value can't be 
empty string.");
 }
 generateClusterConfigurations(context, annot.value(), 
generatedClusterConfigs::add);
 
-String baseDisplayName = context.getRequiredTestMethod().getName();
-generatedClusterConfigs.forEach(config -> 
config.clusterType().invocationContexts(baseDisplayName, config, 
testInvocations));
+if (context.getRequiredTestMethod() != null) {

Review Comment:
   We shouldn't need to check for null here. Can you remove the condition?
   
   
https://github.com/junit-team/junit5/blob/releases/5.10.x/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java#L236



##
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java:
##
@@ -119,23 +116,34 @@ public Stream 
provideTestTemplateInvocationContex
 return generatedContexts.stream();
 }
 
-void processClusterTemplate(ExtensionContext context, ClusterTemplate 
annot,
-
Consumer testInvocations) {
+List 
processClusterTemplate(ExtensionContext context, ClusterTemplate annot) {
 // If specified, call cluster config generated method (must be static)
 List generatedClusterConfigs = new ArrayList<>();
+List testTemplateInvocationContexts = 
new ArrayList<>();
 if (annot.value().trim().isEmpty()) {
 throw new IllegalStateException("ClusterTemplate value can't be 
empty string.");
 }
 generateClusterConfigurations(context, annot.value(), 
generatedClusterConfigs::add);
 
-String baseDisplayName = context.getRequiredTestMethod().getName();
-generatedClusterConfigs.forEach(config -> 
config.clusterType().invocationContexts(baseDisplayName, config, 
testInvocations));
+if (context.getRequiredTestMethod() != null) {
+String baseDisplayName = context.getRequiredTestMethod().getName();
+generatedClusterConfigs.forEach(config -> 
config.clusterType().invocationContexts(baseDisplayName, config, 
testTemplateInvocationContexts::add));
+}
+
+if (testTemplateInvocationContexts.isEmpty()) {
+throw new IllegalStateException("ClusterConfig generator method 
should provide at least one config.");
+}
+
+return testTemplateInvocationContexts;
 }
 
 private void generateClusterConfigurations(ExtensionContext context, 
String generateClustersMethods, ClusterGenerator generator) {
 Object testInstance = context.getTestInstance().orElse(null);
-Method method = 
ReflectionUtils.getRequiredMethod(context.getRequiredTestClass(), 
generateClustersMethods, ClusterGenerator.class);
-ReflectionUtils.invokeMethod(method, testInstance, generator);
+Class testClass = context.getRequiredTestClass();
+if (context.getRequiredTestClass() != null) {

Review Comment:
   Same here. 
   
https://github.com/junit-team/junit5/blob/releases/5.10.x/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java#L137
   
   Can you remove the `if`?



##
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java:
##
@@ -33,16 +31,22 @@ public class ClusterTestExtensionsUnitTest {
 void testProcessClusterTemplate() {
 ClusterTestExtensions ext = new ClusterTestExtensions();
 ExtensionContext context = mock(ExtensionContext.class);
-Consumer testInvocations = 
mock(Consumer.class);
 ClusterTemplate annot = mock(ClusterTemplate.class);
-when(annot.value()).thenReturn("").thenReturn(" ");
+when(annot.value()).thenReturn("").thenReturn(" 
").thenReturn("test_empty_config");

Review Comment:
   I'm wondering if these tests would be simpler to write with actual methods. 
   
   We could define a static inner class:
   
   ```java
   static class StubTest {
   @ClusterTemplate("cfgFoo")
   void testFoo() {}
   static void cfgFoo(ClusterGenerator gen) { /* ... */ }
   
   @ClusterTemplate("")
   void testBar() {}
   }
   ```
   
   and a utility method:
   
   

[PR] KAFKA-16652: add unit test for ClusterTemplate offering zero ClusterConfig [kafka]

2024-05-05 Thread via GitHub


TaiJuWu opened a new pull request, #15862:
URL: https://github.com/apache/kafka/pull/15862

   *More detailed description of your change,
   refactor processClusterTemplate and add unit test for empty config
   
   *Summary of testing strategy (including rationale)
   
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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