feat: US-014 - Add WebTool tests

Added comprehensive test coverage for WebTool (WebSearchTool, WebFetchTool) with 9 test cases:
- TestWebTool_WebFetch_Success: Verifies successful URL fetching
- TestWebTool_WebFetch_JSON: Verifies JSON content handling
- TestWebTool_WebFetch_InvalidURL: Verifies error handling for invalid URLs
- TestWebTool_WebFetch_UnsupportedScheme: Verifies only http/https allowed
- TestWebTool_WebFetch_MissingURL: Verifies missing URL parameter handling
- TestWebTool_WebFetch_Truncation: Verifies content truncation at maxChars
- TestWebTool_WebSearch_NoApiKey: Verifies API key requirement
- TestWebTool_WebSearch_MissingQuery: Verifies missing query parameter
- TestWebTool_WebFetch_HTMLExtraction: Verifies HTML tag removal and text extraction
- TestWebTool_WebFetch_MissingDomain: Verifies domain validation

WebTool implementation already conforms to ToolResult specification:
- WebFetch returns ForUser=fetched content, ForLLM=summary with byte count
- WebSearch returns ForUser=search results, ForLLM=result count
- Errors return ErrorResult with IsError=true

Tests use httptest.NewServer for mock HTTP servers, avoiding external API dependencies.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
yinwm
2026-02-12 19:54:44 +08:00
parent 88014ecaff
commit 0ac93d4429
3 changed files with 293 additions and 4 deletions

View File

@@ -212,7 +212,7 @@
"go test ./pkg/tools -run TestWebTool passes" "go test ./pkg/tools -run TestWebTool passes"
], ],
"priority": 14, "priority": 14,
"passes": false, "passes": true,
"notes": "" "notes": ""
}, },
{ {

View File

@@ -6,12 +6,12 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
## Progress ## Progress
### Completed (10/21) ### Completed (14/21)
- US-001: Add ToolResult struct and helper functions - US-001: Add ToolResult struct and helper functions
- US-002: Modify Tool interface to return *ToolResult - US-002: Modify Tool interface to return *ToolResult
- US-004: Delete isToolConfirmationMessage function (already removed in commit 488e7a9) - US-004: Delete isToolConfirmationMessage function (already removed in commit 488e7a9)
- US-005: Update AgentLoop tool result processing logic - US-005: Update AgentLoop tool result logic
- US-006: Add AsyncCallback type and AsyncTool interface - US-006: Add AsyncCallback type and AsyncTool interface
- US-007: Heartbeat async task execution support - US-007: Heartbeat async task execution support
- US-008: Inject callback into async tools in AgentLoop - US-008: Inject callback into async tools in AgentLoop
@@ -20,6 +20,9 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
- US-011: Refactor MessageTool to use ToolResult - US-011: Refactor MessageTool to use ToolResult
- US-012: Refactor ShellTool to use ToolResult - US-012: Refactor ShellTool to use ToolResult
- US-013: Refactor FilesystemTool to use ToolResult - US-013: Refactor FilesystemTool to use ToolResult
- US-014: Refactor WebTool to use ToolResult
- US-012: Refactor ShellTool to use ToolResult
- US-013: Refactor FilesystemTool to use ToolResult
### In Progress ### In Progress
@@ -40,7 +43,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
| US-011 | Refactor MessageTool to use ToolResult | Completed | Added test file message_test.go | | US-011 | Refactor MessageTool to use ToolResult | Completed | Added test file message_test.go |
| US-012 | Refactor ShellTool to use ToolResult | Completed | | | US-012 | Refactor ShellTool to use ToolResult | Completed | |
| US-013 | Refactor FilesystemTool to use ToolResult | Completed | Added test file filesystem_test.go | | US-013 | Refactor FilesystemTool to use ToolResult | Completed | Added test file filesystem_test.go |
| US-014 | Refactor WebTool to use ToolResult | Completed | | | US-014 | Refactor WebTool to use ToolResult | Completed | Added test file web_test.go |
| US-015 | Refactor EditTool to use ToolResult | Completed | | | US-015 | Refactor EditTool to use ToolResult | Completed | |
| US-016 | Refactor CronTool to use ToolResult | Pending | | | US-016 | Refactor CronTool to use ToolResult | Pending | |
| US-017 | Refactor SpawnTool to use AsyncTool and callbacks | Pending | | | US-017 | Refactor SpawnTool to use AsyncTool and callbacks | Pending | |
@@ -269,4 +272,27 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
- **Gotchas encountered:** 最初误解了 NewToolResult 的行为,以为它会设置 ForUser但实际上它只设置 ForLLM。 - **Gotchas encountered:** 最初误解了 NewToolResult 的行为,以为它会设置 ForUser但实际上它只设置 ForLLM。
- **Useful context:** WriteFileTool 会自动创建不存在的目录(使用 os.MkdirAll这是文件写入工具的重要功能。 - **Useful context:** WriteFileTool 会自动创建不存在的目录(使用 os.MkdirAll这是文件写入工具的重要功能。
---
## [2026-02-12] - US-014
- What was implemented:
- WebTool 已经完全使用 ToolResult 返回值(无需修改实现)
- 添加了完整的测试文件 `pkg/tools/web_test.go`,包含 9 个测试用例
- 测试覆盖了所有验收标准:
- WebFetchTool 成功返回 ForUser=获取的内容ForLLM=摘要
- WebSearchTool 缺少 API Key 返回 ErrorResult
- URL 验证测试(无效 URL、不支持 scheme、缺少域名
- JSON 内容处理测试
- HTML 提取和清理测试(移除 script/style 标签)
- 内容截断测试
- 缺少参数错误处理
- Files changed:
- `pkg/tools/web_test.go` (新增)
- **Learnings for future iterations:**
- **Patterns discovered:** WebTool 使用 httptest.NewServer 创建模拟服务器进行测试,避免依赖外部 API。WebFetchTool 返回 JSON 格式的结构化内容给用户,包含 url、status、extractor、truncated、length、text 字段。
- **Gotchas encountered:** 无
- **Useful context:** WebSearchTool 需要配置 BRAVE_API_KEY 环境变量才能正常工作。WebFetchTool 支持多种内容类型JSON格式化、HTML文本提取、纯文本。
--- ---

263
pkg/tools/web_test.go Normal file
View File

@@ -0,0 +1,263 @@
package tools
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
// TestWebTool_WebFetch_Success verifies successful URL fetching
func TestWebTool_WebFetch_Success(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html")
w.WriteHeader(http.StatusOK)
w.Write([]byte("<html><body><h1>Test Page</h1><p>Content here</p></body></html>"))
}))
defer server.Close()
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": server.URL,
}
result := tool.Execute(ctx, args)
// Success should not be an error
if result.IsError {
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
}
// ForUser should contain the fetched content
if !strings.Contains(result.ForUser, "Test Page") {
t.Errorf("Expected ForUser to contain 'Test Page', got: %s", result.ForUser)
}
// ForLLM should contain summary
if !strings.Contains(result.ForLLM, "bytes") && !strings.Contains(result.ForLLM, "extractor") {
t.Errorf("Expected ForLLM to contain summary, got: %s", result.ForLLM)
}
}
// TestWebTool_WebFetch_JSON verifies JSON content handling
func TestWebTool_WebFetch_JSON(t *testing.T) {
testData := map[string]string{"key": "value", "number": "123"}
expectedJSON, _ := json.MarshalIndent(testData, "", " ")
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
w.Write(expectedJSON)
}))
defer server.Close()
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": server.URL,
}
result := tool.Execute(ctx, args)
// Success should not be an error
if result.IsError {
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
}
// ForUser should contain formatted JSON
if !strings.Contains(result.ForUser, "key") && !strings.Contains(result.ForUser, "value") {
t.Errorf("Expected ForUser to contain JSON data, got: %s", result.ForUser)
}
}
// TestWebTool_WebFetch_InvalidURL verifies error handling for invalid URL
func TestWebTool_WebFetch_InvalidURL(t *testing.T) {
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": "not-a-valid-url",
}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error for invalid URL")
}
// Should contain error message (either "invalid URL" or scheme error)
if !strings.Contains(result.ForLLM, "URL") && !strings.Contains(result.ForUser, "URL") {
t.Errorf("Expected error message for invalid URL, got ForLLM: %s", result.ForLLM)
}
}
// TestWebTool_WebFetch_UnsupportedScheme verifies error handling for non-http URLs
func TestWebTool_WebFetch_UnsupportedScheme(t *testing.T) {
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": "ftp://example.com/file.txt",
}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error for unsupported URL scheme")
}
// Should mention only http/https allowed
if !strings.Contains(result.ForLLM, "http/https") && !strings.Contains(result.ForUser, "http/https") {
t.Errorf("Expected scheme error message, got ForLLM: %s", result.ForLLM)
}
}
// TestWebTool_WebFetch_MissingURL verifies error handling for missing URL
func TestWebTool_WebFetch_MissingURL(t *testing.T) {
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error when URL is missing")
}
// Should mention URL is required
if !strings.Contains(result.ForLLM, "url is required") && !strings.Contains(result.ForUser, "url is required") {
t.Errorf("Expected 'url is required' message, got ForLLM: %s", result.ForLLM)
}
}
// TestWebTool_WebFetch_Truncation verifies content truncation
func TestWebTool_WebFetch_Truncation(t *testing.T) {
longContent := strings.Repeat("x", 20000)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/plain")
w.WriteHeader(http.StatusOK)
w.Write([]byte(longContent))
}))
defer server.Close()
tool := NewWebFetchTool(1000) // Limit to 1000 chars
ctx := context.Background()
args := map[string]interface{}{
"url": server.URL,
}
result := tool.Execute(ctx, args)
// Success should not be an error
if result.IsError {
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
}
// ForUser should contain truncated content (not the full 20000 chars)
resultMap := make(map[string]interface{})
json.Unmarshal([]byte(result.ForUser), &resultMap)
if text, ok := resultMap["text"].(string); ok {
if len(text) > 1100 { // Allow some margin
t.Errorf("Expected content to be truncated to ~1000 chars, got: %d", len(text))
}
}
// Should be marked as truncated
if truncated, ok := resultMap["truncated"].(bool); !ok || !truncated {
t.Errorf("Expected 'truncated' to be true in result")
}
}
// TestWebTool_WebSearch_NoApiKey verifies error handling when API key is missing
func TestWebTool_WebSearch_NoApiKey(t *testing.T) {
tool := NewWebSearchTool("", 5)
ctx := context.Background()
args := map[string]interface{}{
"query": "test",
}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error when API key is missing")
}
// Should mention missing API key
if !strings.Contains(result.ForLLM, "BRAVE_API_KEY") && !strings.Contains(result.ForUser, "BRAVE_API_KEY") {
t.Errorf("Expected API key error message, got ForLLM: %s", result.ForLLM)
}
}
// TestWebTool_WebSearch_MissingQuery verifies error handling for missing query
func TestWebTool_WebSearch_MissingQuery(t *testing.T) {
tool := NewWebSearchTool("test-key", 5)
ctx := context.Background()
args := map[string]interface{}{}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error when query is missing")
}
}
// TestWebTool_WebFetch_HTMLExtraction verifies HTML text extraction
func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html")
w.WriteHeader(http.StatusOK)
w.Write([]byte(`<html><body><script>alert('test');</script><style>body{color:red;}</style><h1>Title</h1><p>Content</p></body></html>`))
}))
defer server.Close()
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": server.URL,
}
result := tool.Execute(ctx, args)
// Success should not be an error
if result.IsError {
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
}
// ForUser should contain extracted text (without script/style tags)
if !strings.Contains(result.ForUser, "Title") && !strings.Contains(result.ForUser, "Content") {
t.Errorf("Expected ForUser to contain extracted text, got: %s", result.ForUser)
}
// Should NOT contain script or style tags
if strings.Contains(result.ForUser, "<script>") || strings.Contains(result.ForUser, "<style>") {
t.Errorf("Expected script/style tags to be removed, got: %s", result.ForUser)
}
}
// TestWebTool_WebFetch_MissingDomain verifies error handling for URL without domain
func TestWebTool_WebFetch_MissingDomain(t *testing.T) {
tool := NewWebFetchTool(50000)
ctx := context.Background()
args := map[string]interface{}{
"url": "https://",
}
result := tool.Execute(ctx, args)
// Should return error result
if !result.IsError {
t.Errorf("Expected error for URL without domain")
}
// Should mention missing domain
if !strings.Contains(result.ForLLM, "domain") && !strings.Contains(result.ForUser, "domain") {
t.Errorf("Expected domain error message, got ForLLM: %s", result.ForLLM)
}
}