Skip to content

Commit 3404c33

Browse files
test: replace superficial tests with real behavioral assertions
- Rewrite 28 outbound handler tests using wiremock to verify actual Discord HTTP calls (method, path, body content) instead of just checking subscribe completes without panic - Add behavioral tests for DiscordAgent verifying channel_id, content, reply_to_message_id, ack_emoji, and publisher independence - Make DiscordAgent and OutboundProcessor generic over NATS client and publisher types (default params preserve production behavior) - Add publisher as explicit constructor param to both structs - Add wiremock dev-dependency to discord-bot Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ce5d6f5 commit 3404c33

16 files changed

Lines changed: 2383 additions & 420 deletions

File tree

rsworkspace/Cargo.lock

Lines changed: 70 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rsworkspace/crates/discord-agent/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,4 @@ reqwest = { version = "0.12", features = ["json", "stream"] }
2626

2727
[dev-dependencies]
2828
uuid = { version = "1", features = ["v4"] }
29+
discord-nats = { path = "../discord-nats", features = ["test-support"] }

rsworkspace/crates/discord-agent/src/agent.rs

Lines changed: 137 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
//! Main agent implementation
22
33
use anyhow::Result;
4-
use async_nats::Client;
5-
use discord_nats::{MessagePublisher, MessageSubscriber};
4+
use discord_nats::{MessagePublisher, MessageSubscriber, Publish, QueueSubscribeClient};
65
use tracing::{error, info};
76

87
use crate::health::AgentMetrics;
@@ -11,19 +10,23 @@ use crate::processor::{MessageProcessor, WelcomeConfig};
1110
use tokio::time::Duration;
1211

1312
/// Discord agent that processes messages
14-
pub struct DiscordAgent {
15-
subscriber: MessageSubscriber,
16-
publisher: MessagePublisher,
13+
pub struct DiscordAgent<
14+
N: QueueSubscribeClient + Clone = async_nats::Client,
15+
P: Publish = MessagePublisher,
16+
> {
17+
subscriber: MessageSubscriber<N>,
18+
publisher: P,
1719
processor: MessageProcessor,
1820
agent_name: String,
1921
}
2022

21-
impl DiscordAgent {
23+
impl<N: QueueSubscribeClient + Clone, P: Publish> DiscordAgent<N, P> {
2224
/// Create a new Discord agent
2325
#[allow(clippy::too_many_arguments)]
2426
pub fn new(
25-
client: Client,
27+
client: N,
2628
prefix: String,
29+
publisher: P,
2730
agent_name: String,
2831
llm_config: Option<ClaudeConfig>,
2932
conversation_kv: Option<async_nats::jetstream::kv::Store>,
@@ -36,8 +39,7 @@ impl DiscordAgent {
3639
stream_timeout_secs: u64,
3740
ack_emoji: Option<String>,
3841
) -> Self {
39-
let subscriber = MessageSubscriber::new(client.clone(), prefix.clone());
40-
let publisher = MessagePublisher::new(client, prefix);
42+
let subscriber = MessageSubscriber::new(client, prefix);
4143
let processor = MessageProcessor::new(
4244
llm_config,
4345
conversation_kv,
@@ -820,3 +822,129 @@ impl DiscordAgent {
820822
Ok(())
821823
}
822824
}
825+
826+
#[cfg(test)]
827+
mod tests {
828+
use super::*;
829+
use discord_nats::{MockNatsClient, MockPublisher};
830+
831+
fn make_agent() -> DiscordAgent<MockNatsClient, MockPublisher> {
832+
let client = MockNatsClient::new();
833+
let publisher = MockPublisher::new("test");
834+
DiscordAgent::new(
835+
client, "test".to_string(), publisher, "test-agent".to_string(),
836+
None, None, None, None, None, None, None, 20, 120, None,
837+
)
838+
}
839+
840+
fn make_message_event(
841+
session_id: &str,
842+
content: &str,
843+
channel_id: u64,
844+
message_id: u64,
845+
) -> discord_types::events::MessageCreatedEvent {
846+
use discord_types::events::{EventMetadata, MessageCreatedEvent};
847+
use discord_types::types::{DiscordMessage, DiscordUser};
848+
MessageCreatedEvent {
849+
metadata: EventMetadata::new(session_id, 1),
850+
message: DiscordMessage {
851+
id: message_id,
852+
channel_id,
853+
guild_id: None,
854+
author: DiscordUser { id: 42, username: "tester".to_string(), global_name: None, bot: false },
855+
content: content.to_string(),
856+
timestamp: "2024-01-01T00:00:00Z".to_string(),
857+
edited_timestamp: None,
858+
attachments: vec![],
859+
embeds: vec![],
860+
referenced_message_id: None,
861+
referenced_message_content: None,
862+
},
863+
pluralkit_member_id: None,
864+
pluralkit_member_name: None,
865+
}
866+
}
867+
868+
/// Verify the agent forwards prefix and name correctly.
869+
#[test]
870+
fn test_agent_constructs_with_mock() {
871+
let agent = make_agent();
872+
assert_eq!(agent.agent_name, "test-agent");
873+
assert_eq!(agent.subscriber.prefix(), "test");
874+
}
875+
876+
/// run() must propagate the subscribe error immediately when the NATS client is a mock.
877+
#[tokio::test]
878+
async fn test_agent_run_returns_err_when_subscribe_fails() {
879+
let agent = make_agent();
880+
let result = agent.run().await;
881+
assert!(result.is_err(), "run() must return Err when subscribe fails");
882+
let msg = result.unwrap_err().to_string();
883+
assert!(!msg.is_empty(), "error must have a message");
884+
}
885+
886+
/// Without ack_emoji: process_message publishes exactly typing + send_message (2 commands)
887+
/// and the send_message carries the original content and channel_id.
888+
#[tokio::test]
889+
async fn test_agent_echo_mode_publishes_typing_and_send() {
890+
let agent = make_agent();
891+
let pub_ref = MockPublisher::new("test");
892+
let event = make_message_event("sess-1", "hello world", 100, 50);
893+
894+
agent.processor.process_message(&event, &pub_ref).await.unwrap();
895+
896+
let msgs = pub_ref.published_messages();
897+
assert_eq!(msgs.len(), 2, "echo mode must publish typing + send_message");
898+
let cmd: discord_types::SendMessageCommand =
899+
serde_json::from_value(msgs[1].1.clone()).unwrap();
900+
assert_eq!(cmd.channel_id, 100, "send must target the right channel");
901+
assert!(cmd.content.contains("hello world"), "send must echo the content");
902+
assert_eq!(cmd.reply_to_message_id, Some(50), "send must reply to the original message");
903+
}
904+
905+
/// With ack_emoji: process_message publishes ack_reaction first, then typing, then send_message.
906+
/// The ack reaction must carry the configured emoji and target the original message.
907+
#[tokio::test]
908+
async fn test_agent_with_ack_emoji_publishes_ack_reaction_first() {
909+
let client = MockNatsClient::new();
910+
let publisher = MockPublisher::new("test");
911+
let agent = DiscordAgent::new(
912+
client, "test".to_string(), publisher, "ack-agent".to_string(),
913+
None, None, None, None, None, None, None, 20, 120, Some("⏳".to_string()),
914+
);
915+
let pub_ref = MockPublisher::new("test");
916+
let event = make_message_event("sess-2", "help me", 200, 99);
917+
918+
agent.processor.process_message(&event, &pub_ref).await.unwrap();
919+
920+
let msgs = pub_ref.published_messages();
921+
assert!(msgs.len() >= 3, "with ack_emoji: must publish ack_reaction + typing + send_message");
922+
let reaction: discord_types::AddReactionCommand =
923+
serde_json::from_value(msgs[0].1.clone()).unwrap();
924+
assert_eq!(reaction.emoji, "⏳", "ack reaction must use the configured emoji");
925+
assert_eq!(reaction.message_id, 99, "ack reaction must target the original message");
926+
assert_eq!(reaction.channel_id, 200, "ack reaction must be in the right channel");
927+
}
928+
929+
/// Two agents with different prefixes must not share publisher state.
930+
#[tokio::test]
931+
async fn test_agent_publishers_are_independent() {
932+
let pub_a = MockPublisher::new("pfx-a");
933+
let pub_b = MockPublisher::new("pfx-b");
934+
let agent_a = DiscordAgent::new(
935+
MockNatsClient::new(), "pfx-a".to_string(), pub_a.clone(), "agent-a".to_string(),
936+
None, None, None, None, None, None, None, 20, 120, None,
937+
);
938+
let agent_b = DiscordAgent::new(
939+
MockNatsClient::new(), "pfx-b".to_string(), pub_b.clone(), "agent-b".to_string(),
940+
None, None, None, None, None, None, None, 20, 120, None,
941+
);
942+
943+
let event = make_message_event("sess-a", "ping", 100, 1);
944+
agent_a.processor.process_message(&event, &pub_a).await.unwrap();
945+
946+
assert!(!pub_a.is_empty(), "agent_a's publisher must have messages");
947+
assert!(pub_b.is_empty(), "agent_b's publisher must be unaffected");
948+
assert_ne!(agent_a.subscriber.prefix(), agent_b.subscriber.prefix());
949+
}
950+
}

rsworkspace/crates/discord-agent/src/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,11 @@ async fn main() -> Result<()> {
262262
args.stream_timeout_secs
263263
};
264264

265+
let publisher = discord_nats::MessagePublisher::new(nats_client.clone(), args.prefix.clone());
265266
let agent = DiscordAgent::new(
266267
nats_client,
267268
args.prefix,
269+
publisher,
268270
args.agent_name.clone(),
269271
llm_config,
270272
conversation_kv,

0 commit comments

Comments
 (0)