test(context): add /context hooks tests for HookExecutor, ContextManager (#1239)

Author: hayemaxi

crates/q_cli/src/cli/chat/context.rs

@@ -521,7 +521,8 @@ impl ContextManager {
         self.save_config(global).await
     }
 
-    /// Run all the currently enabled hooks from both the global and profile contexts
+    /// Run all the currently enabled hooks from both the global and profile contexts.
+    /// Skipped hooks (disabled) will not appear in the output.
     /// # Arguments
     /// * `updates` - output stream to write hook run status to
     /// # Returns
@@ -753,6 +754,7 @@ fn validate_profile_name(name: &str) -> Result<()> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::cli::chat::hooks::HookTrigger;
 
     // Helper function to create a test ContextManager with Context
     pub async fn create_test_context_manager() -> Result<ContextManager> {
@@ -856,4 +858,123 @@ mod tests {
 
         Ok(())
     }
+
+    #[tokio::test]
+    async fn test_add_hook() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        // Test adding hook to profile config
+        manager.add_hook("test_hook".to_string(), hook.clone(), false).await?;
+        assert!(manager.profile_config.hooks.contains_key("test_hook"));
+
+        // Test adding hook to global config
+        manager.add_hook("global_hook".to_string(), hook.clone(), true).await?;
+        assert!(manager.global_config.hooks.contains_key("global_hook"));
+
+        // Test adding duplicate hook name
+        assert!(manager.add_hook("test_hook".to_string(), hook, false).await.is_err());
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_remove_hook() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        manager.add_hook("test_hook".to_string(), hook, false).await?;
+
+        // Test removing existing hook
+        manager.remove_hook("test_hook", false).await?;
+        assert!(!manager.profile_config.hooks.contains_key("test_hook"));
+
+        // Test removing non-existent hook
+        assert!(manager.remove_hook("test_hook", false).await.is_err());
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_set_hook_disabled() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        manager.add_hook("test_hook".to_string(), hook, false).await?;
+
+        // Test disabling hook
+        manager.set_hook_disabled("test_hook", false, true).await?;
+        assert!(manager.profile_config.hooks.get("test_hook").unwrap().disabled);
+
+        // Test enabling hook
+        manager.set_hook_disabled("test_hook", false, false).await?;
+        assert!(!manager.profile_config.hooks.get("test_hook").unwrap().disabled);
+
+        // Test with non-existent hook
+        assert!(manager.set_hook_disabled("nonexistent", false, true).await.is_err());
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_set_all_hooks_disabled() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook1 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+        let hook2 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        manager.add_hook("hook1".to_string(), hook1, false).await?;
+        manager.add_hook("hook2".to_string(), hook2, false).await?;
+
+        // Test disabling all hooks
+        manager.set_all_hooks_disabled(false, true).await?;
+        assert!(manager.profile_config.hooks.values().all(|h| h.disabled));
+
+        // Test enabling all hooks
+        manager.set_all_hooks_disabled(false, false).await?;
+        assert!(manager.profile_config.hooks.values().all(|h| !h.disabled));
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_run_hooks() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook1 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+        let hook2 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        manager.add_hook("hook1".to_string(), hook1, false).await?;
+        manager.add_hook("hook2".to_string(), hook2, false).await?;
+
+        // Create a buffer to capture the output
+        let mut output = Vec::new();
+
+        // Run the hooks
+        let results = manager.run_hooks(&mut output).await;
+        assert_eq!(results.len(), 2); // Should include both hooks
+
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn test_hooks_across_profiles() -> Result<()> {
+        let mut manager = create_test_context_manager().await?;
+        let hook1 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+        let hook2 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo test".to_string());
+
+        manager.add_hook("profile_hook".to_string(), hook1, false).await?;
+        manager.add_hook("global_hook".to_string(), hook2, true).await?;
+
+        let results = manager.run_hooks(&mut Vec::new()).await;
+        assert_eq!(results.len(), 2); // Should include both hooks
+
+        // Create and switch to a new profile
+        manager.create_profile("test_profile").await?;
+        manager.switch_profile("test_profile").await?;
+
+        let results = manager.run_hooks(&mut Vec::new()).await;
+        assert_eq!(results.len(), 1); // Should include global hook
+        assert_eq!(results[0].0.name, "global_hook");
+
+        Ok(())
+    }
 }

crates/q_cli/src/cli/chat/conversation_state.rs

@@ -878,6 +878,7 @@ mod tests {
                 .as_sendable_conversation_state(Some(conversation_start_context.to_string()))
                 .await;
             let hist = s.history.as_ref().unwrap();
+            #[allow(clippy::match_wildcard_for_single_variants)]
             match &hist[0] {
                 ChatMessage::UserInputMessage(user) => {
                     assert!(
@@ -886,7 +887,6 @@ mod tests {
                         user.content
                     );
                 },
-                #[allow(clippy::match_wildcard_for_single_variants)]
                 _ => panic!("Expected user message."),
             }
             assert!(

crates/q_cli/src/cli/chat/hooks.rs

@@ -281,3 +281,204 @@ impl HookExecutor {
         cache.insert(hook.name.clone(), hook_output);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::str::from_utf8;
+    use std::time::Duration;
+
+    use tokio::time::sleep;
+
+    use super::*;
+
+    #[test]
+    fn test_hook_creation() {
+        let command = "echo 'hello'";
+        let hook = Hook::new_inline_hook(HookTrigger::PerPrompt, command.to_string());
+
+        assert_eq!(hook.r#type, HookType::Inline);
+        assert!(!hook.disabled);
+        assert_eq!(hook.timeout_ms, DEFAULT_TIMEOUT_MS);
+        assert_eq!(hook.max_output_size, DEFAULT_MAX_OUTPUT_SIZE);
+        assert_eq!(hook.cache_ttl_seconds, DEFAULT_CACHE_TTL_SECONDS);
+        assert_eq!(hook.command, Some(command.to_string()));
+        assert_eq!(hook.trigger, HookTrigger::PerPrompt);
+        assert!(!hook.is_global);
+    }
+
+    #[tokio::test]
+    async fn test_hook_executor_cached_conversation_start() {
+        let mut executor = HookExecutor::new();
+        let mut hook1 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo 'test1'".to_string());
+        hook1.is_global = true;
+
+        let mut hook2 = Hook::new_inline_hook(HookTrigger::ConversationStart, "echo 'test2'".to_string());
+        hook2.is_global = false;
+
+        // First execution should run the command
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(from_utf8(&output).unwrap().contains("Running 2 hooks"));
+
+        // Second execution should use cache
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(output.is_empty()); // Should not have run the hook, so no output.
+    }
+
+    #[tokio::test]
+    async fn test_hook_executor_cached_per_prompt() {
+        let mut executor = HookExecutor::new();
+        let mut hook1 = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test1'".to_string());
+        hook1.is_global = true;
+        hook1.cache_ttl_seconds = 60;
+
+        let mut hook2 = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test2'".to_string());
+        hook2.is_global = false;
+        hook2.cache_ttl_seconds = 60;
+
+        // First execution should run the command
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(from_utf8(&output).unwrap().contains("Running 2 hooks"));
+
+        // Second execution should use cache
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(output.is_empty()); // Should not have run the hook, so no output.
+    }
+
+    #[tokio::test]
+    async fn test_hook_executor_not_cached_per_prompt() {
+        let mut executor = HookExecutor::new();
+        let mut hook1 = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test1'".to_string());
+        hook1.is_global = true;
+
+        let mut hook2 = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test2'".to_string());
+        hook2.is_global = false;
+
+        // First execution should run the command
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(from_utf8(&output).unwrap().contains("Running 2 hooks"));
+
+        // Second execution should use cache
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook1, &hook2], &mut output).await;
+
+        assert_eq!(results.len(), 2);
+        assert!(results[0].1.contains("test1"));
+        assert!(results[1].1.contains("test2"));
+        assert!(from_utf8(&output).unwrap().contains("Running 2 hooks"));
+    }
+
+    #[tokio::test]
+    async fn test_hook_timeout() {
+        let mut executor = HookExecutor::new();
+        let mut hook = Hook::new_inline_hook(HookTrigger::PerPrompt, "sleep 2".to_string());
+        hook.timeout_ms = 100; // Set very short timeout
+
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook], &mut output).await;
+
+        assert_eq!(results.len(), 0); // Should fail due to timeout
+    }
+
+    #[tokio::test]
+    async fn test_disabled_hook() {
+        let mut executor = HookExecutor::new();
+        let mut hook = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test'".to_string());
+        hook.disabled = true;
+
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook], &mut output).await;
+
+        assert_eq!(results.len(), 0); // Disabled hook should not run
+    }
+
+    #[tokio::test]
+    async fn test_cache_expiration() {
+        let mut executor = HookExecutor::new();
+        let mut hook = Hook::new_inline_hook(HookTrigger::PerPrompt, "echo 'test'".to_string());
+        hook.cache_ttl_seconds = 1;
+
+        let mut output = Vec::new();
+
+        // First execution
+        let results1 = executor.run_hooks(vec![&hook], &mut output).await;
+        assert_eq!(results1.len(), 1);
+
+        // Wait for cache to expire
+        sleep(Duration::from_millis(1001)).await;
+
+        // Second execution should run command again
+        let results2 = executor.run_hooks(vec![&hook], &mut output).await;
+        assert_eq!(results2.len(), 1);
+    }
+
+    #[test]
+    fn test_hook_cache_storage() {
+        let mut executor: HookExecutor = HookExecutor::new();
+        let hook = Hook::new_inline_hook(HookTrigger::PerPrompt, "".to_string());
+
+        let cached_hook = CachedHook {
+            output: "test output".to_string(),
+            expiry: None,
+        };
+
+        executor.insert_cache(&hook, cached_hook.clone());
+
+        assert_eq!(executor.get_cache(&hook), Some("test output".to_string()));
+    }
+
+    #[test]
+    fn test_hook_cache_storage_expired() {
+        let mut executor: HookExecutor = HookExecutor::new();
+        let hook = Hook::new_inline_hook(HookTrigger::PerPrompt, "".to_string());
+
+        let cached_hook = CachedHook {
+            output: "test output".to_string(),
+            expiry: Some(Instant::now()),
+        };
+
+        executor.insert_cache(&hook, cached_hook.clone());
+
+        // Item should not return since it is expired
+        assert_eq!(executor.get_cache(&hook), None);
+    }
+
+    #[tokio::test]
+    async fn test_max_output_size() {
+        let mut executor = HookExecutor::new();
+        let mut hook = Hook::new_inline_hook(
+            HookTrigger::PerPrompt,
+            "for i in {1..1000}; do echo $i; done".to_string(),
+        );
+        hook.max_output_size = 100;
+
+        let mut output = Vec::new();
+        let results = executor.run_hooks(vec![&hook], &mut output).await;
+
+        assert!(results[0].1.len() <= hook.max_output_size + " ... truncated".len());
+    }
+}