From 26274afd05bc54bf8b3f8439b6918047a9c9fc23 Mon Sep 17 00:00:00 2001 From: David Jacot Date: Tue, 5 Dec 2023 08:58:48 +0100 Subject: [PATCH] MINOR: Ensure that DisplayName is set in all parameterized tests (#14850) This is a follow-up to https://github.com/apache/kafka/pull/14687 as we found out that some parameterized tests do not include the test method name in their name. For the context, the JUnit XML report does not include the name of the method by default but only rely on the display name provided. Reviewers: David Arthur --- .../test/java/kafka/test/annotation/Type.java | 24 +++++++++---------- .../test/junit/ClusterTestExtensions.java | 7 +++--- .../junit/RaftClusterInvocationContext.java | 6 +++-- .../junit/ZkClusterInvocationContext.java | 6 +++-- .../scala/kafka/utils/TestInfoUtils.scala | 2 +- .../apache/kafka/tools/ToolsTestUtils.java | 2 +- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/core/src/test/java/kafka/test/annotation/Type.java b/core/src/test/java/kafka/test/annotation/Type.java index 933ca501134..feb9a278489 100644 --- a/core/src/test/java/kafka/test/annotation/Type.java +++ b/core/src/test/java/kafka/test/annotation/Type.java @@ -30,36 +30,36 @@ import java.util.function.Consumer; public enum Type { KRAFT { @Override - public void invocationContexts(ClusterConfig config, Consumer invocationConsumer) { - invocationConsumer.accept(new RaftClusterInvocationContext(config.copyOf(), false)); + public void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer) { + invocationConsumer.accept(new RaftClusterInvocationContext(baseDisplayName, config.copyOf(), false)); } }, CO_KRAFT { @Override - public void invocationContexts(ClusterConfig config, Consumer invocationConsumer) { - invocationConsumer.accept(new RaftClusterInvocationContext(config.copyOf(), true)); + public void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer) { + invocationConsumer.accept(new RaftClusterInvocationContext(baseDisplayName, config.copyOf(), true)); } }, ZK { @Override - public void invocationContexts(ClusterConfig config, Consumer invocationConsumer) { - invocationConsumer.accept(new ZkClusterInvocationContext(config.copyOf())); + public void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer) { + invocationConsumer.accept(new ZkClusterInvocationContext(baseDisplayName, config.copyOf())); } }, ALL { @Override - public void invocationContexts(ClusterConfig config, Consumer invocationConsumer) { - invocationConsumer.accept(new RaftClusterInvocationContext(config.copyOf(), false)); - invocationConsumer.accept(new RaftClusterInvocationContext(config.copyOf(), true)); - invocationConsumer.accept(new ZkClusterInvocationContext(config.copyOf())); + public void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer) { + invocationConsumer.accept(new RaftClusterInvocationContext(baseDisplayName, config.copyOf(), false)); + invocationConsumer.accept(new RaftClusterInvocationContext(baseDisplayName, config.copyOf(), true)); + invocationConsumer.accept(new ZkClusterInvocationContext(baseDisplayName, config.copyOf())); } }, DEFAULT { @Override - public void invocationContexts(ClusterConfig config, Consumer invocationConsumer) { + public void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer) { throw new UnsupportedOperationException("Cannot create invocation contexts for DEFAULT type"); } }; - public abstract void invocationContexts(ClusterConfig config, Consumer invocationConsumer); + public abstract void invocationContexts(String baseDisplayName, ClusterConfig config, Consumer invocationConsumer); } diff --git a/core/src/test/java/kafka/test/junit/ClusterTestExtensions.java b/core/src/test/java/kafka/test/junit/ClusterTestExtensions.java index 50e2a063649..8c838b279bd 100644 --- a/core/src/test/java/kafka/test/junit/ClusterTestExtensions.java +++ b/core/src/test/java/kafka/test/junit/ClusterTestExtensions.java @@ -128,7 +128,8 @@ public class ClusterTestExtensions implements TestTemplateInvocationContextProvi generatedClusterConfigs.add(ClusterConfig.defaultClusterBuilder().build()); } - generatedClusterConfigs.forEach(config -> config.clusterType().invocationContexts(config, testInvocations)); + String baseDisplayName = context.getRequiredTestMethod().getName(); + generatedClusterConfigs.forEach(config -> config.clusterType().invocationContexts(baseDisplayName, config, testInvocations)); } private void generateClusterConfigurations(ExtensionContext context, String generateClustersMethods, ClusterGenerator generator) { @@ -183,8 +184,6 @@ public class ClusterTestExtensions implements TestTemplateInvocationContextProvi annot.securityProtocol(), annot.metadataVersion()); if (!annot.name().isEmpty()) { builder.name(annot.name()); - } else { - builder.name(context.getRequiredTestMethod().getName()); } if (!annot.listener().isEmpty()) { builder.listenerName(annot.listener()); @@ -197,7 +196,7 @@ public class ClusterTestExtensions implements TestTemplateInvocationContextProvi ClusterConfig config = builder.build(); config.serverProperties().putAll(properties); - type.invocationContexts(config, testInvocations); + type.invocationContexts(context.getRequiredTestMethod().getName(), config, testInvocations); } private ClusterTestDefaults getClusterTestDefaults(Class testClass) { diff --git a/core/src/test/java/kafka/test/junit/RaftClusterInvocationContext.java b/core/src/test/java/kafka/test/junit/RaftClusterInvocationContext.java index 98f8ab55ce7..4aa152055af 100644 --- a/core/src/test/java/kafka/test/junit/RaftClusterInvocationContext.java +++ b/core/src/test/java/kafka/test/junit/RaftClusterInvocationContext.java @@ -64,12 +64,14 @@ import java.util.stream.Stream; */ public class RaftClusterInvocationContext implements TestTemplateInvocationContext { + private final String baseDisplayName; private final ClusterConfig clusterConfig; private final AtomicReference clusterReference; private final AtomicReference zkReference; private final boolean isCombined; - public RaftClusterInvocationContext(ClusterConfig clusterConfig, boolean isCombined) { + public RaftClusterInvocationContext(String baseDisplayName, ClusterConfig clusterConfig, boolean isCombined) { + this.baseDisplayName = baseDisplayName; this.clusterConfig = clusterConfig; this.clusterReference = new AtomicReference<>(); this.zkReference = new AtomicReference<>(); @@ -81,7 +83,7 @@ public class RaftClusterInvocationContext implements TestTemplateInvocationConte String clusterDesc = clusterConfig.nameTags().entrySet().stream() .map(Object::toString) .collect(Collectors.joining(", ")); - return String.format("[%d] Type=Raft-%s, %s", invocationIndex, isCombined ? "Combined" : "Isolated", clusterDesc); + return String.format("%s [%d] Type=Raft-%s, %s", baseDisplayName, invocationIndex, isCombined ? "Combined" : "Isolated", clusterDesc); } @Override diff --git a/core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java b/core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java index 5eb342d8a53..dd0098f7868 100644 --- a/core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java +++ b/core/src/test/java/kafka/test/junit/ZkClusterInvocationContext.java @@ -66,10 +66,12 @@ import java.util.stream.Stream; */ public class ZkClusterInvocationContext implements TestTemplateInvocationContext { + private final String baseDisplayName; private final ClusterConfig clusterConfig; private final AtomicReference clusterReference; - public ZkClusterInvocationContext(ClusterConfig clusterConfig) { + public ZkClusterInvocationContext(String baseDisplayName, ClusterConfig clusterConfig) { + this.baseDisplayName = baseDisplayName; this.clusterConfig = clusterConfig; this.clusterReference = new AtomicReference<>(); } @@ -79,7 +81,7 @@ public class ZkClusterInvocationContext implements TestTemplateInvocationContext String clusterDesc = clusterConfig.nameTags().entrySet().stream() .map(Object::toString) .collect(Collectors.joining(", ")); - return String.format("[%d] Type=ZK, %s", invocationIndex, clusterDesc); + return String.format("%s [%d] Type=ZK, %s", baseDisplayName, invocationIndex, clusterDesc); } @Override diff --git a/core/src/test/scala/kafka/utils/TestInfoUtils.scala b/core/src/test/scala/kafka/utils/TestInfoUtils.scala index b586be36933..c82a654b228 100644 --- a/core/src/test/scala/kafka/utils/TestInfoUtils.scala +++ b/core/src/test/scala/kafka/utils/TestInfoUtils.scala @@ -52,7 +52,7 @@ object TestInfoUtils { testInfo.getDisplayName().contains("quorum=zkMigration") } } - final val TestWithParameterizedQuorumName = "{displayName}.quorum={0}" + final val TestWithParameterizedQuorumName = "{displayName}.{argumentsWithNames}" final val TestWithParameterizedQuorumAndGroupProtocolNames = "{displayName}.quorum={0}.groupProtocol={1}" diff --git a/tools/src/test/java/org/apache/kafka/tools/ToolsTestUtils.java b/tools/src/test/java/org/apache/kafka/tools/ToolsTestUtils.java index ac379bcb49e..9b698b0cc5f 100644 --- a/tools/src/test/java/org/apache/kafka/tools/ToolsTestUtils.java +++ b/tools/src/test/java/org/apache/kafka/tools/ToolsTestUtils.java @@ -42,7 +42,7 @@ import java.util.stream.Collectors; public class ToolsTestUtils { /** @see TestInfoUtils#TestWithParameterizedQuorumName() */ - public static final String TEST_WITH_PARAMETERIZED_QUORUM_NAME = "{displayName}.quorum={0}"; + public static final String TEST_WITH_PARAMETERIZED_QUORUM_NAME = "{displayName}.{argumentsWithNames}"; private static int randomPort = 0;