chia7712 commented on code in PR #15916:
URL: https://github.com/apache/kafka/pull/15916#discussion_r1597475237
##########
core/src/test/java/kafka/test/annotation/Type.java:
##########
@@ -22,44 +22,53 @@
import kafka.test.junit.ZkClusterInvocationContext;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.ArrayList;
/**
* The type of cluster config being requested. Used by {@link
kafka.test.ClusterConfig} and the test annotations.
*/
public enum Type {
KRAFT {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
RaftClusterInvocationContext(baseDisplayName, config, false));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new RaftClusterInvocationContext(baseDisplayName, config,
false));
+ return ret;
}
},
CO_KRAFT {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
RaftClusterInvocationContext(baseDisplayName, config, true));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new RaftClusterInvocationContext(baseDisplayName, config,
true));
+ return ret;
Review Comment:
ditto
##########
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java:
##########
@@ -140,8 +141,38 @@ private void
generateClusterConfigurations(ExtensionContext context, String gene
ReflectionUtils.invokeMethod(method, testInstance, generator);
}
- private void processClusterTest(ExtensionContext context, ClusterTest
annot, ClusterTestDefaults defaults,
- Consumer<TestTemplateInvocationContext>
testInvocations) {
+ private List<TestTemplateInvocationContext>
processClusterTests(ExtensionContext context,
+
ClusterTestDefaults defaults) {
+
+ ClusterTests clusterTestsAnnot =
context.getRequiredTestMethod().getDeclaredAnnotation(ClusterTests.class);
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+
+ if (clusterTestsAnnot != null) {
+ for (ClusterTest annot : clusterTestsAnnot.value()) {
+ ret.addAll(processClusterTestInternal(context, annot,
defaults));
+ }
+ }
+
+ if (ret.isEmpty()) {
+ throw new IllegalStateException("processClusterTests method should
provide at least one config");
+ }
+
+ return ret;
+ }
+
+ private List<TestTemplateInvocationContext>
processClusterTest(ExtensionContext context, ClusterTest annot,
+
ClusterTestDefaults defaults) {
+ List<TestTemplateInvocationContext> ret =
processClusterTestInternal(context, annot, defaults);
+
+ if (ret.isEmpty()) {
+ throw new IllegalStateException("processClusterTest method should
provide at least one config");
+ }
+
+ return ret;
+ }
+
+ private List<TestTemplateInvocationContext>
processClusterTestInternal(ExtensionContext context, ClusterTest annot,
Review Comment:
It seems we don't need `processClusterTestInternal` as it just has a check.
We can move the check to `processClusterTest`
##########
core/src/test/java/kafka/test/annotation/Type.java:
##########
@@ -22,44 +22,53 @@
import kafka.test.junit.ZkClusterInvocationContext;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.ArrayList;
/**
* The type of cluster config being requested. Used by {@link
kafka.test.ClusterConfig} and the test annotations.
*/
public enum Type {
KRAFT {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
RaftClusterInvocationContext(baseDisplayName, config, false));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new RaftClusterInvocationContext(baseDisplayName, config,
false));
+ return ret;
Review Comment:
How about `return Collections.singletonList(new
RaftClusterInvocationContext(baseDisplayName, config, false));`?
##########
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java:
##########
@@ -93,24 +92,19 @@ public Stream<TestTemplateInvocationContext>
provideTestTemplateInvocationContex
// Process the @ClusterTemplate annotation
ClusterTemplate clusterTemplateAnnot =
context.getRequiredTestMethod().getDeclaredAnnotation(ClusterTemplate.class);
if (clusterTemplateAnnot != null) {
- processClusterTemplate(context, clusterTemplateAnnot,
generatedContexts::add);
- if (generatedContexts.isEmpty()) {
- throw new IllegalStateException("ClusterConfig generator
method should provide at least one config");
- }
+ generatedContexts.addAll(processClusterTemplate(context,
clusterTemplateAnnot));
}
// Process single @ClusterTest annotation
ClusterTest clusterTestAnnot =
context.getRequiredTestMethod().getDeclaredAnnotation(ClusterTest.class);
if (clusterTestAnnot != null) {
- processClusterTest(context, clusterTestAnnot, defaults,
generatedContexts::add);
+ generatedContexts.addAll(processClusterTest(context,
clusterTestAnnot, defaults));
}
// Process multiple @ClusterTest annotation within @ClusterTests
ClusterTests clusterTestsAnnot =
context.getRequiredTestMethod().getDeclaredAnnotation(ClusterTests.class);
if (clusterTestsAnnot != null) {
- for (ClusterTest annot : clusterTestsAnnot.value()) {
- processClusterTest(context, annot, defaults,
generatedContexts::add);
- }
+ generatedContexts.addAll(processClusterTests(context, defaults));
Review Comment:
`processClusterTests` should accept `clusterTestsAnnot` as we have checked it
##########
core/src/test/java/kafka/test/annotation/Type.java:
##########
@@ -22,44 +22,53 @@
import kafka.test.junit.ZkClusterInvocationContext;
import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
-import java.util.function.Consumer;
+import java.util.List;
+import java.util.ArrayList;
/**
* The type of cluster config being requested. Used by {@link
kafka.test.ClusterConfig} and the test annotations.
*/
public enum Type {
KRAFT {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
RaftClusterInvocationContext(baseDisplayName, config, false));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new RaftClusterInvocationContext(baseDisplayName, config,
false));
+ return ret;
}
},
CO_KRAFT {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
RaftClusterInvocationContext(baseDisplayName, config, true));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new RaftClusterInvocationContext(baseDisplayName, config,
true));
+ return ret;
}
},
ZK {
@Override
- public void invocationContexts(String baseDisplayName, ClusterConfig
config, Consumer<TestTemplateInvocationContext> invocationConsumer) {
- invocationConsumer.accept(new
ZkClusterInvocationContext(baseDisplayName, config));
+ public List<TestTemplateInvocationContext> invocationContexts(String
baseDisplayName, ClusterConfig config) {
+ List<TestTemplateInvocationContext> ret = new ArrayList<>();
+ ret.add(new ZkClusterInvocationContext(baseDisplayName, config));
+ return ret;
Review Comment:
ditto
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
