From cbadc562dc8cd3256b92bc394401b164f2bab34f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arnaud=20Cogolu=C3=A8gnes?= Date: Thu, 30 Jan 2025 15:39:56 +0100 Subject: [PATCH] Make sure wrong value for boolean option returns error --- .../com/rabbitmq/stream/perf/Converters.java | 14 ++ .../rabbitmq/stream/perf/StreamPerfTest.java | 138 +++++++++-- .../com/rabbitmq/stream/perf/PicoCliTest.java | 228 ++++++++++++++++++ 3 files changed, 362 insertions(+), 18 deletions(-) create mode 100644 src/test/java/com/rabbitmq/stream/perf/PicoCliTest.java diff --git a/src/main/java/com/rabbitmq/stream/perf/Converters.java b/src/main/java/com/rabbitmq/stream/perf/Converters.java index b6ef0db..a576cf6 100644 --- a/src/main/java/com/rabbitmq/stream/perf/Converters.java +++ b/src/main/java/com/rabbitmq/stream/perf/Converters.java @@ -43,6 +43,8 @@ final class Converters { private static final CommandLine.ITypeConverter DURATION_TYPE_CONVERTER = new DurationTypeConverter(); + static final CommandLine.ITypeConverter BOOLEAN_TYPE_CONVERTER = new BooleanConverter(); + private Converters() {} static void typeConversionException(String message) { @@ -434,6 +436,18 @@ public Integer convert(String input) { } } + static class BooleanConverter implements CommandLine.ITypeConverter { + + @Override + public Boolean convert(String value) { + if ("true".equalsIgnoreCase(value) || "false".equalsIgnoreCase(value)) { + return Boolean.parseBoolean(value); + } else { + throw new CommandLine.TypeConversionException("'" + value + "' is not a boolean"); + } + } + } + private static void throwConversionException(String format, String... arguments) { throw new CommandLine.TypeConversionException(String.format(format, (Object[]) arguments)); } diff --git a/src/main/java/com/rabbitmq/stream/perf/StreamPerfTest.java b/src/main/java/com/rabbitmq/stream/perf/StreamPerfTest.java index d9f78f5..04c648d 100644 --- a/src/main/java/com/rabbitmq/stream/perf/StreamPerfTest.java +++ b/src/main/java/com/rabbitmq/stream/perf/StreamPerfTest.java @@ -94,6 +94,7 @@ name = "stream-perf-test", mixinStandardHelpOptions = false, showDefaultValues = true, + separator = " ", description = "Tests the performance of stream queues in RabbitMQ.") public class StreamPerfTest implements Callable { @@ -169,8 +170,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--delete-streams", "-ds"}, description = "whether to delete stream(s) after the run or not", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean deleteStreams; + void setDeleteStreams(String input) throws Exception { + this.deleteStreams = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean deleteStreams; @CommandLine.Option( names = {"--offset", "-o"}, @@ -265,14 +272,26 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--version", "-v"}, description = "show version information", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean version; + void setVersion(String input) throws Exception { + this.version = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean version; @CommandLine.Option( names = {"--summary-file", "-sf"}, description = "generate a summary file with metrics", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean summaryFile; + void setSummaryFile(String input) throws Exception { + this.summaryFile = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean summaryFile; @CommandLine.Option( names = {"--producers-by-connection", "-pbc"}, @@ -309,8 +328,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--load-balancer", "-lb"}, description = "assume URIs point to a load balancer", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean loadBalancer; + void setLoadBalancer(String input) throws Exception { + this.loadBalancer = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean loadBalancer; @CommandLine.Option( names = {"--consumer-names", "-cn"}, @@ -324,14 +349,26 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--metrics-byte-rates", "-mbr"}, description = "include written and read byte rates in metrics", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean includeByteRates; + void setIncludeByteRates(String input) throws Exception { + this.includeByteRates = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean includeByteRates; @CommandLine.Option( names = {"--memory-report", "-mr"}, description = "report information on memory settings and usage", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean memoryReport; + void setMemoryReport(String input) throws Exception { + this.memoryReport = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean memoryReport; @CommandLine.Option( names = {"--server-name-indication", "-sni"}, @@ -349,8 +386,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--environment-variables", "-env"}, description = "show usage with environment variables", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean environmentVariables; + void setEnvironmentVariables(String input) throws Exception { + this.environmentVariables = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean environmentVariables; @CommandLine.Option( names = {"--rpc-timeout", "-rt"}, @@ -362,14 +405,26 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--confirm-latency", "-cl"}, description = "evaluate confirm latency", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean confirmLatency; + void setConfirmLatency(String input) throws Exception { + this.confirmLatency = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean confirmLatency; @CommandLine.Option( names = {"--super-streams", "-sst"}, description = "use super streams (RabbitMQ 3.13+)", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean superStreams; + void setSuperStreams(String input) throws Exception { + this.superStreams = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean superStreams; @CommandLine.Option( names = {"--super-stream-partitions", "-ssp"}, @@ -381,8 +436,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--single-active-consumer", "-sac"}, description = "use single active consumer", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean singleActiveConsumer; + void setSingleActiveConsumer(String input) throws Exception { + this.singleActiveConsumer = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean singleActiveConsumer; @CommandLine.Option( names = {"--amqp-uri", "-au"}, @@ -406,8 +467,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--metrics-command-line-arguments", "-mcla"}, description = "add fixed metrics with command line arguments label", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean metricsCommandLineArguments; + void setMetricsCommandLineArguments(String input) throws Exception { + this.metricsCommandLineArguments = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean metricsCommandLineArguments; @CommandLine.Option( names = {"--requested-max-frame-size", "-rmfs"}, @@ -419,8 +486,14 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--native-epoll", "-ne"}, description = "use Netty's native epoll transport (Linux x86-64 only)", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean nativeEpoll; + void setNativeEpoll(String input) throws Exception { + this.nativeEpoll = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean nativeEpoll; @ArgGroup(exclusive = false, multiplicity = "0..1") InstanceSyncOptions instanceSyncOptions; @@ -447,26 +520,50 @@ public class StreamPerfTest implements Callable { @CommandLine.Option( names = {"--force-replica-for-consumers", "-frfc"}, description = "force the connection to a replica for consumers", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean forceReplicaForConsumers; + void setForceReplicaForConsumers(String input) throws Exception { + this.forceReplicaForConsumers = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean forceReplicaForConsumers; @CommandLine.Option( names = {"--no-dev-mode", "-ndm"}, description = "do not use development mode (useful for local cluster)", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean noDevMode; + void setNoDevMode(String input) throws Exception { + this.noDevMode = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean noDevMode; @CommandLine.Option( names = {"--dynamic-batch-size", "-dbs"}, description = "use dynamic batch size for publishing", + arity = "0..1", + fallbackValue = "true", defaultValue = "true") - private boolean dynamicBatch; + void setDynamicBatch(String input) throws Exception { + this.dynamicBatch = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean dynamicBatch; @CommandLine.Option( names = {"--batch-size-metric", "-bsm"}, description = "display batch size", + arity = "0..1", + fallbackValue = "true", defaultValue = "false") - private boolean includeBatchSizeMetric; + void setIncludeByteSizeMetric(String input) throws Exception { + this.includeBatchSizeMetric = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean includeBatchSizeMetric; static class InstanceSyncOptions { @@ -553,9 +650,14 @@ static class InstanceSyncOptions { @CommandLine.Option( names = {"--tcp-no-delay", "-tnd"}, description = "TCP NODELAY", - arity = "1", + arity = "0..1", + fallbackValue = "true", defaultValue = "true") - private boolean tcpNoDelay; + void setTcpNoDelay(String input) throws Exception { + this.tcpNoDelay = Converters.BOOLEAN_TYPE_CONVERTER.convert(input); + } + + boolean tcpNoDelay; @CommandLine.Option( names = {"--consumer-latency", "-L"}, diff --git a/src/test/java/com/rabbitmq/stream/perf/PicoCliTest.java b/src/test/java/com/rabbitmq/stream/perf/PicoCliTest.java new file mode 100644 index 0000000..6a5c5f5 --- /dev/null +++ b/src/test/java/com/rabbitmq/stream/perf/PicoCliTest.java @@ -0,0 +1,228 @@ +// Copyright (c) 2025 Broadcom. All Rights Reserved. +// The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. +// +// This software, the RabbitMQ Stream Performance Testing Tool, is dual-licensed under the +// Mozilla Public License 2.0 ("MPL"), and the Apache License version 2 ("ASL"). +// For the MPL, please see LICENSE-MPL-RabbitMQ. For the ASL, +// please see LICENSE-APACHE2. +// +// This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY KIND, +// either express or implied. See the LICENSE file for specific language governing +// rights and limitations of this software. +// +// If you have any questions regarding licensing, please contact us at +// info@rabbitmq.com. +package com.rabbitmq.stream.perf; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.concurrent.Callable; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +public class PicoCliTest { + + @Test + void parseConfirmLatency() { + assertThat(exec("").confirmLatency).isFalse(); + assertThat(exec("--confirm-latency").confirmLatency).isTrue(); + assertThat(exec("--confirm-latency true").confirmLatency).isTrue(); + assertThat(exec("--confirm-latency false").confirmLatency).isFalse(); + assertThatThrownBy(() -> exec("--confirm-latency 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseDeleteStreams() { + assertThat(exec("").deleteStreams).isFalse(); + assertThat(exec("--delete-streams").deleteStreams).isTrue(); + assertThat(exec("--delete-streams true").deleteStreams).isTrue(); + assertThat(exec("--delete-streams false").deleteStreams).isFalse(); + assertThatThrownBy(() -> exec("--delete-streams 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseDynamicBatchSize() { + assertThat(exec("").dynamicBatch).isTrue(); + assertThat(exec("--dynamic-batch-size").dynamicBatch).isTrue(); + assertThat(exec("--dynamic-batch-size true").dynamicBatch).isTrue(); + assertThat(exec("--dynamic-batch-size false").dynamicBatch).isFalse(); + assertThatThrownBy(() -> exec("--dynamic-batch-size 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseEnvironmentVariables() { + assertThat(exec("").environmentVariables).isFalse(); + assertThat(exec("--environment-variables").environmentVariables).isTrue(); + assertThat(exec("--environment-variables true").environmentVariables).isTrue(); + assertThat(exec("--environment-variables false").environmentVariables).isFalse(); + assertThatThrownBy(() -> exec("--environment-variables 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseForceReplicaForConsumers() { + assertThat(exec("").forceReplicaForConsumers).isFalse(); + assertThat(exec("--force-replica-for-consumers").forceReplicaForConsumers).isTrue(); + assertThat(exec("--force-replica-for-consumers true").forceReplicaForConsumers).isTrue(); + assertThat(exec("--force-replica-for-consumers false").forceReplicaForConsumers).isFalse(); + assertThatThrownBy(() -> exec("--force-replica-for-consumers 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseNoDevMode() { + assertThat(exec("").noDevMode).isFalse(); + assertThat(exec("--no-dev-mode").noDevMode).isTrue(); + assertThat(exec("--no-dev-mode true").noDevMode).isTrue(); + assertThat(exec("--no-dev-mode false").noDevMode).isFalse(); + assertThatThrownBy(() -> exec("--no-dev-mode 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseBatchSizeMetric() { + assertThat(exec("").includeBatchSizeMetric).isFalse(); + assertThat(exec("--batch-size-metric").includeBatchSizeMetric).isTrue(); + assertThat(exec("--batch-size-metric true").includeBatchSizeMetric).isTrue(); + assertThat(exec("--batch-size-metric false").includeBatchSizeMetric).isFalse(); + assertThatThrownBy(() -> exec("--batch-size-metric 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseMetricsByteRates() { + assertThat(exec("").includeByteRates).isFalse(); + assertThat(exec("--metrics-byte-rates").includeByteRates).isTrue(); + assertThat(exec("--metrics-byte-rates true").includeByteRates).isTrue(); + assertThat(exec("--metrics-byte-rates false").includeByteRates).isFalse(); + assertThatThrownBy(() -> exec("--metrics-byte-rates 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseLoadBalancer() { + assertThat(exec("").loadBalancer).isFalse(); + assertThat(exec("--load-balancer").loadBalancer).isTrue(); + assertThat(exec("--load-balancer true").loadBalancer).isTrue(); + assertThat(exec("--load-balancer false").loadBalancer).isFalse(); + assertThatThrownBy(() -> exec("--load-balancer 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseMemoryReport() { + assertThat(exec("").memoryReport).isFalse(); + assertThat(exec("--memory-report").memoryReport).isTrue(); + assertThat(exec("--memory-report true").memoryReport).isTrue(); + assertThat(exec("--memory-report false").memoryReport).isFalse(); + assertThatThrownBy(() -> exec("--memory-report 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseMetricsCommandLineArguments() { + assertThat(exec("").metricsCommandLineArguments).isFalse(); + assertThat(exec("--metrics-command-line-arguments").metricsCommandLineArguments).isTrue(); + assertThat(exec("--metrics-command-line-arguments true").metricsCommandLineArguments).isTrue(); + assertThat(exec("--metrics-command-line-arguments false").metricsCommandLineArguments) + .isFalse(); + assertThatThrownBy(() -> exec("--metrics-command-line-arguments 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseNativeEpoll() { + assertThat(exec("").nativeEpoll).isFalse(); + assertThat(exec("--native-epoll").nativeEpoll).isTrue(); + assertThat(exec("--native-epoll true").nativeEpoll).isTrue(); + assertThat(exec("--native-epoll false").nativeEpoll).isFalse(); + assertThatThrownBy(() -> exec("--native-epoll 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseSingleActiveConsumer() { + assertThat(exec("").singleActiveConsumer).isFalse(); + assertThat(exec("--single-active-consumer").singleActiveConsumer).isTrue(); + assertThat(exec("--single-active-consumer true").singleActiveConsumer).isTrue(); + assertThat(exec("--single-active-consumer false").singleActiveConsumer).isFalse(); + assertThatThrownBy(() -> exec("--single-active-consumer 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseSummaryFile() { + assertThat(exec("").summaryFile).isFalse(); + assertThat(exec("--summary-file").summaryFile).isTrue(); + assertThat(exec("--summary-file true").summaryFile).isTrue(); + assertThat(exec("--summary-file false").summaryFile).isFalse(); + assertThatThrownBy(() -> exec("--summary-file 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseSuperStreams() { + assertThat(exec("").superStreams).isFalse(); + assertThat(exec("--super-streams").superStreams).isTrue(); + assertThat(exec("--super-streams true").superStreams).isTrue(); + assertThat(exec("--super-streams false").superStreams).isFalse(); + assertThatThrownBy(() -> exec("--super-streams 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseTcpNoDelay() { + assertThat(exec("").tcpNoDelay).isTrue(); + assertThat(exec("--tcp-no-delay").tcpNoDelay).isTrue(); + assertThat(exec("--tcp-no-delay true").tcpNoDelay).isTrue(); + assertThat(exec("--tcp-no-delay false").tcpNoDelay).isFalse(); + assertThatThrownBy(() -> exec("--tcp-no-delay 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + @Test + void parseVersion() { + assertThat(exec("").version).isFalse(); + assertThat(exec("--version").version).isTrue(); + assertThat(exec("--version true").version).isTrue(); + assertThat(exec("--version false").version).isFalse(); + assertThatThrownBy(() -> exec("--version 10ms")) + .isInstanceOf(CommandLine.ParameterException.class); + } + + StreamPerfTest exec(String line) { + StreamPerfTest app = new StreamPerfTest(); + CommandLine commandLine = new CommandLine(app); + commandLine.parseArgs(line.isBlank() ? new String[0] : args(line)); + return app; + } + + private static String[] args(String line) { + return line.split(" "); + } + + @CommandLine.Command( + name = "stream-perf-test", + mixinStandardHelpOptions = false, + showDefaultValues = true, + description = "Tests the performance of stream queues in RabbitMQ.") + private static class AppWithBooleanOption implements Callable { + + @CommandLine.Option( + names = {"--confirm-latency", "-cl"}, + description = "evaluate confirm latency", + arity = "0..1", + fallbackValue = "true", + defaultValue = "false") + private boolean confirmLatency; + + @Override + public Integer call() { + return 0; + } + } +}