From 22bd03c65af54a0ba23a18f0e17bc5f19d4e05bc Mon Sep 17 00:00:00 2001 From: Alex Gorevski Date: Thu, 19 Feb 2026 13:23:33 -0800 Subject: [PATCH] test(quality): replace bare .unwrap() with .expect() in agent and shell tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace bare .unwrap() calls with descriptive .expect() messages in src/agent/agent.rs and src/tools/shell.rs test modules. Adds meaningful failure context for memory creation, agent builder, and tool execution assertions. Addresses audit finding on test assertion quality (ยง5.2). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/agent/agent.rs | 10 ++++++---- src/tools/shell.rs | 40 ++++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/agent/agent.rs b/src/agent/agent.rs index 8dad045..7b41d16 100644 --- a/src/agent/agent.rs +++ b/src/agent/agent.rs @@ -678,7 +678,8 @@ mod tests { ..crate::config::MemoryConfig::default() }; let mem: Arc = Arc::from( - crate::memory::create_memory(&memory_cfg, std::path::Path::new("/tmp"), None).unwrap(), + crate::memory::create_memory(&memory_cfg, std::path::Path::new("/tmp"), None) + .expect("memory creation should succeed with valid config"), ); let observer: Arc = Arc::from(crate::observability::NoopObserver {}); @@ -690,7 +691,7 @@ mod tests { .tool_dispatcher(Box::new(XmlToolDispatcher)) .workspace_dir(std::path::PathBuf::from("/tmp")) .build() - .unwrap(); + .expect("agent builder should succeed with valid config"); let response = agent.turn("hi").await.unwrap(); assert_eq!(response, "hello"); @@ -720,7 +721,8 @@ mod tests { ..crate::config::MemoryConfig::default() }; let mem: Arc = Arc::from( - crate::memory::create_memory(&memory_cfg, std::path::Path::new("/tmp"), None).unwrap(), + crate::memory::create_memory(&memory_cfg, std::path::Path::new("/tmp"), None) + .expect("memory creation should succeed with valid config"), ); let observer: Arc = Arc::from(crate::observability::NoopObserver {}); @@ -732,7 +734,7 @@ mod tests { .tool_dispatcher(Box::new(NativeToolDispatcher)) .workspace_dir(std::path::PathBuf::from("/tmp")) .build() - .unwrap(); + .expect("agent builder should succeed with valid config"); let response = agent.turn("hi").await.unwrap(); assert_eq!(response, "done"); diff --git a/src/tools/shell.rs b/src/tools/shell.rs index f06c543..4392bdb 100644 --- a/src/tools/shell.rs +++ b/src/tools/shell.rs @@ -198,7 +198,7 @@ mod tests { assert!(schema["properties"]["command"].is_object()); assert!(schema["required"] .as_array() - .unwrap() + .expect("schema required field should be an array") .contains(&json!("command"))); assert!(schema["properties"]["approved"].is_object()); } @@ -209,7 +209,7 @@ mod tests { let result = tool .execute(json!({"command": "echo hello"})) .await - .unwrap(); + .expect("echo command execution should succeed"); assert!(result.success); assert!(result.output.trim().contains("hello")); assert!(result.error.is_none()); @@ -218,7 +218,10 @@ mod tests { #[tokio::test] async fn shell_blocks_disallowed_command() { let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime()); - let result = tool.execute(json!({"command": "rm -rf /"})).await.unwrap(); + let result = tool + .execute(json!({"command": "rm -rf /"})) + .await + .expect("disallowed command execution should return a result"); assert!(!result.success); let error = result.error.as_deref().unwrap_or(""); assert!(error.contains("not allowed") || error.contains("high-risk")); @@ -227,9 +230,16 @@ mod tests { #[tokio::test] async fn shell_blocks_readonly() { let tool = ShellTool::new(test_security(AutonomyLevel::ReadOnly), test_runtime()); - let result = tool.execute(json!({"command": "ls"})).await.unwrap(); + let result = tool + .execute(json!({"command": "ls"})) + .await + .expect("readonly command execution should return a result"); assert!(!result.success); - assert!(result.error.as_ref().unwrap().contains("not allowed")); + assert!(result + .error + .as_ref() + .expect("error field should be present for blocked command") + .contains("not allowed")); } #[tokio::test] @@ -253,7 +263,7 @@ mod tests { let result = tool .execute(json!({"command": "ls /nonexistent_dir_xyz"})) .await - .unwrap(); + .expect("command with nonexistent path should return a result"); assert!(!result.success); } @@ -296,7 +306,10 @@ mod tests { let _g2 = EnvGuard::set("ZEROCLAW_API_KEY", "sk-test-secret-67890"); let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime()); - let result = tool.execute(json!({"command": "env"})).await.unwrap(); + let result = tool + .execute(json!({"command": "env"})) + .await + .expect("env command execution should succeed"); assert!(result.success); assert!( !result.output.contains("sk-test-secret-12345"), @@ -315,7 +328,7 @@ mod tests { let result = tool .execute(json!({"command": "echo $HOME"})) .await - .unwrap(); + .expect("echo HOME command should succeed"); assert!(result.success); assert!( !result.output.trim().is_empty(), @@ -325,7 +338,7 @@ mod tests { let result = tool .execute(json!({"command": "echo $PATH"})) .await - .unwrap(); + .expect("echo PATH command should succeed"); assert!(result.success); assert!( !result.output.trim().is_empty(), @@ -346,7 +359,7 @@ mod tests { let denied = tool .execute(json!({"command": "touch zeroclaw_shell_approval_test"})) .await - .unwrap(); + .expect("unapproved command should return a result"); assert!(!denied.success); assert!(denied .error @@ -360,7 +373,7 @@ mod tests { "approved": true })) .await - .unwrap(); + .expect("approved command execution should succeed"); assert!(allowed.success); let _ = @@ -420,7 +433,10 @@ mod tests { ..SecurityPolicy::default() }); let tool = ShellTool::new(security, test_runtime()); - let result = tool.execute(json!({"command": "echo test"})).await.unwrap(); + let result = tool + .execute(json!({"command": "echo test"})) + .await + .expect("rate-limited command should return a result"); assert!(!result.success); assert!(result.error.as_deref().unwrap_or("").contains("Rate limit")); }