test(quality): replace bare .unwrap() with .expect() in agent and shell tests
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>
This commit is contained in:
parent
bec1dc7b8c
commit
22bd03c65a
2 changed files with 34 additions and 16 deletions
|
|
@ -678,7 +678,8 @@ mod tests {
|
|||
..crate::config::MemoryConfig::default()
|
||||
};
|
||||
let mem: Arc<dyn Memory> = 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<dyn Observer> = 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<dyn Memory> = 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<dyn Observer> = 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");
|
||||
|
|
|
|||
|
|
@ -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"));
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue