From 048097d6ead819ddd303f2af0722d5a19a9104be Mon Sep 17 00:00:00 2001 From: SunilKumarPradhan Date: Thu, 15 Oct 2026 20:51:19 +0530 Subject: [PATCH 1/2] fix(cli): use correct consumer kind wording in consumer-offset set explain(), the error context and the success message in set_consumer_offset.rs always said "consumer", even for -k consumer-group or -k external-group. Add get_consumer_info(), mirroring get_consumer_offset.rs, and use it in all three places. Add unit tests for the explain message of each consumer kind and assert the wording in the external-group CLI integration test. Closes #4459 --- .../set_consumer_offset.rs | 66 +++++++++++++++++-- ..._consumer_offset_external_group_command.rs | 9 ++- 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs b/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs index 6a8b4518a1..06456e6860 100644 --- a/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs +++ b/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs @@ -51,14 +51,28 @@ impl SetConsumerOffsetCmd { offset, } } + + pub fn get_consumer_info(&self) -> String { + match self.consumer.kind { + ConsumerKind::Consumer => { + format!("consumer with ID: {}", self.consumer.id) + } + ConsumerKind::ConsumerGroup => format!("consumer group with ID: {}", self.consumer.id), + ConsumerKind::ExternalGroup => format!("external group with ID: {}", self.consumer.id), + } + } } #[async_trait] impl CliCommand for SetConsumerOffsetCmd { fn explain(&self) -> String { format!( - "set consumer offset for consumer with ID: {} for stream with ID: {} and topic with ID: {} and partition with ID: {} to {}", - self.consumer.id, self.stream_id, self.topic_id, self.partition_id, self.offset, + "set consumer offset for {} for stream with ID: {} and topic with ID: {} and partition with ID: {} to {}", + self.get_consumer_info(), + self.stream_id, + self.topic_id, + self.partition_id, + self.offset, ) } @@ -68,14 +82,14 @@ impl CliCommand for SetConsumerOffsetCmd { .await .with_context(|| { format!( - "Problem setting consumer offset for consumer with ID: {} for stream with ID: {} and topic with ID: {} and partition with ID: {}", - self.consumer.id, self.stream_id, self.topic_id, self.partition_id + "Problem setting consumer offset for {} for stream with ID: {} and topic with ID: {} and partition with ID: {}", + self.get_consumer_info(), self.stream_id, self.topic_id, self.partition_id ) })?; event!(target: PRINT_TARGET, Level::INFO, - "Consumer offset for consumer with ID: {} for stream with ID: {} and topic with ID: {} and partition with ID: {} set to {}", - self.consumer.id, + "Consumer offset for {} for stream with ID: {} and topic with ID: {} and partition with ID: {} set to {}", + self.get_consumer_info(), self.stream_id, self.topic_id, self.partition_id, @@ -85,3 +99,43 @@ impl CliCommand for SetConsumerOffsetCmd { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn cmd(kind: ConsumerKind) -> SetConsumerOffsetCmd { + SetConsumerOffsetCmd::new( + Identifier::numeric(1).unwrap(), + Identifier::numeric(2).unwrap(), + Identifier::numeric(3).unwrap(), + 0, + 100, + kind, + ) + } + + #[test] + fn should_name_consumer_kind_in_explain_message() { + assert_eq!( + cmd(ConsumerKind::Consumer).explain(), + "set consumer offset for consumer with ID: 1 for stream with ID: 2 and topic with ID: 3 and partition with ID: 0 to 100" + ); + } + + #[test] + fn should_name_consumer_group_kind_in_explain_message() { + assert_eq!( + cmd(ConsumerKind::ConsumerGroup).explain(), + "set consumer offset for consumer group with ID: 1 for stream with ID: 2 and topic with ID: 3 and partition with ID: 0 to 100" + ); + } + + #[test] + fn should_name_external_group_kind_in_explain_message() { + assert_eq!( + cmd(ConsumerKind::ExternalGroup).explain(), + "set consumer offset for external group with ID: 1 for stream with ID: 2 and topic with ID: 3 and partition with ID: 0 to 100" + ); + } +} diff --git a/core/integration/tests/cli/consumer_offset/test_consumer_offset_external_group_command.rs b/core/integration/tests/cli/consumer_offset/test_consumer_offset_external_group_command.rs index 669e5b3d38..8dda006728 100644 --- a/core/integration/tests/cli/consumer_offset/test_consumer_offset_external_group_command.rs +++ b/core/integration/tests/cli/consumer_offset/test_consumer_offset_external_group_command.rs @@ -99,7 +99,14 @@ impl IggyCmdTestCase for TestExternalGroupOffsetCmd { fn verify_command(&self, command_state: Assert) { match self.step { Step::Set => { - command_state.success(); + command_state + .success() + .stdout(starts_with(format!( + "Executing set consumer offset for external group with ID: {GROUP_NAME}" + ))) + .stdout(contains(format!( + "Consumer offset for external group with ID: {GROUP_NAME}" + ))); } Step::Get => { command_state From c3aab3473edf8a4f4e112dc66154b702c54bd176 Mon Sep 17 00:00:00 2001 From: SunilKumarPradhan Date: Mon, 26 Oct 2026 18:34:21 +0530 Subject: [PATCH 2/2] refactor(cli): share consumer-kind wording between get/set commands GetConsumerOffsetCmd and SetConsumerOffsetCmd each had their own get_consumer_info() with an identical three-arm match on ConsumerKind. Move the match into a consumer_info() helper in binary_consumer_offsets/mod.rs and have both commands call it. Addresses review feedback on #4620. --- .../binary_consumer_offsets/get_consumer_offset.rs | 9 ++------- core/cli/src/commands/binary_consumer_offsets/mod.rs | 10 ++++++++++ .../binary_consumer_offsets/set_consumer_offset.rs | 9 ++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/cli/src/commands/binary_consumer_offsets/get_consumer_offset.rs b/core/cli/src/commands/binary_consumer_offsets/get_consumer_offset.rs index f76f455c59..fef680549b 100644 --- a/core/cli/src/commands/binary_consumer_offsets/get_consumer_offset.rs +++ b/core/cli/src/commands/binary_consumer_offsets/get_consumer_offset.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use crate::commands::binary_consumer_offsets::consumer_info; use crate::commands::cli_command::{CliCommand, PRINT_TARGET}; use anyhow::Context; use async_trait::async_trait; @@ -50,13 +51,7 @@ impl GetConsumerOffsetCmd { } pub fn get_consumer_info(&self) -> String { - match self.consumer.kind { - ConsumerKind::Consumer => { - format!("consumer with ID: {}", self.consumer.id) - } - ConsumerKind::ConsumerGroup => format!("consumer group with ID: {}", self.consumer.id), - ConsumerKind::ExternalGroup => format!("external group with ID: {}", self.consumer.id), - } + consumer_info(&self.consumer) } } diff --git a/core/cli/src/commands/binary_consumer_offsets/mod.rs b/core/cli/src/commands/binary_consumer_offsets/mod.rs index d8ec0264e5..059f893a1c 100644 --- a/core/cli/src/commands/binary_consumer_offsets/mod.rs +++ b/core/cli/src/commands/binary_consumer_offsets/mod.rs @@ -17,3 +17,13 @@ pub mod get_consumer_offset; pub mod set_consumer_offset; + +use iggy_common::{Consumer, ConsumerKind}; + +pub(crate) fn consumer_info(consumer: &Consumer) -> String { + match consumer.kind { + ConsumerKind::Consumer => format!("consumer with ID: {}", consumer.id), + ConsumerKind::ConsumerGroup => format!("consumer group with ID: {}", consumer.id), + ConsumerKind::ExternalGroup => format!("external group with ID: {}", consumer.id), + } +} diff --git a/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs b/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs index 06456e6860..2bde8c9a19 100644 --- a/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs +++ b/core/cli/src/commands/binary_consumer_offsets/set_consumer_offset.rs @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +use crate::commands::binary_consumer_offsets::consumer_info; use crate::commands::cli_command::{CliCommand, PRINT_TARGET}; use anyhow::Context; use async_trait::async_trait; @@ -53,13 +54,7 @@ impl SetConsumerOffsetCmd { } pub fn get_consumer_info(&self) -> String { - match self.consumer.kind { - ConsumerKind::Consumer => { - format!("consumer with ID: {}", self.consumer.id) - } - ConsumerKind::ConsumerGroup => format!("consumer group with ID: {}", self.consumer.id), - ConsumerKind::ExternalGroup => format!("external group with ID: {}", self.consumer.id), - } + consumer_info(&self.consumer) } }