Skip to content

Commit 0ac93d4

Browse files
yinwmclaude
andcommitted
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>
1 parent 88014ec commit 0ac93d4

File tree

3 files changed

+293
-4
lines changed

3 files changed

+293
-4
lines changed

.ralph/prd.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@
212212
"go test ./pkg/tools -run TestWebTool passes"
213213
],
214214
"priority": 14,
215-
"passes": false,
215+
"passes": true,
216216
"notes": ""
217217
},
218218
{

.ralph/progress.txt

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
66

77
## Progress
88

9-
### Completed (10/21)
9+
### Completed (14/21)
1010

1111
- US-001: Add ToolResult struct and helper functions
1212
- US-002: Modify Tool interface to return *ToolResult
1313
- US-004: Delete isToolConfirmationMessage function (already removed in commit 488e7a9)
14-
- US-005: Update AgentLoop tool result processing logic
14+
- US-005: Update AgentLoop tool result logic
1515
- US-006: Add AsyncCallback type and AsyncTool interface
1616
- US-007: Heartbeat async task execution support
1717
- US-008: Inject callback into async tools in AgentLoop
@@ -20,6 +20,9 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
2020
- US-011: Refactor MessageTool to use ToolResult
2121
- US-012: Refactor ShellTool to use ToolResult
2222
- US-013: Refactor FilesystemTool to use ToolResult
23+
- US-014: Refactor WebTool to use ToolResult
24+
- US-012: Refactor ShellTool to use ToolResult
25+
- US-013: Refactor FilesystemTool to use ToolResult
2326

2427
### In Progress
2528

@@ -40,7 +43,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
4043
| US-011 | Refactor MessageTool to use ToolResult | Completed | Added test file message_test.go |
4144
| US-012 | Refactor ShellTool to use ToolResult | Completed | |
4245
| US-013 | Refactor FilesystemTool to use ToolResult | Completed | Added test file filesystem_test.go |
43-
| US-014 | Refactor WebTool to use ToolResult | Completed | |
46+
| US-014 | Refactor WebTool to use ToolResult | Completed | Added test file web_test.go |
4447
| US-015 | Refactor EditTool to use ToolResult | Completed | |
4548
| US-016 | Refactor CronTool to use ToolResult | Pending | |
4649
| US-017 | Refactor SpawnTool to use AsyncTool and callbacks | Pending | |
@@ -269,4 +272,27 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改
269272
- **Gotchas encountered:** 最初误解了 NewToolResult 的行为,以为它会设置 ForUser,但实际上它只设置 ForLLM。
270273
- **Useful context:** WriteFileTool 会自动创建不存在的目录(使用 os.MkdirAll),这是文件写入工具的重要功能。
271274

275+
---
276+
277+
## [2026-02-12] - US-014
278+
- What was implemented:
279+
- WebTool 已经完全使用 ToolResult 返回值(无需修改实现)
280+
- 添加了完整的测试文件 `pkg/tools/web_test.go`,包含 9 个测试用例
281+
- 测试覆盖了所有验收标准:
282+
- WebFetchTool 成功返回 ForUser=获取的内容,ForLLM=摘要
283+
- WebSearchTool 缺少 API Key 返回 ErrorResult
284+
- URL 验证测试(无效 URL、不支持 scheme、缺少域名)
285+
- JSON 内容处理测试
286+
- HTML 提取和清理测试(移除 script/style 标签)
287+
- 内容截断测试
288+
- 缺少参数错误处理
289+
290+
- Files changed:
291+
- `pkg/tools/web_test.go` (新增)
292+
293+
- **Learnings for future iterations:**
294+
- **Patterns discovered:** WebTool 使用 httptest.NewServer 创建模拟服务器进行测试,避免依赖外部 API。WebFetchTool 返回 JSON 格式的结构化内容给用户,包含 url、status、extractor、truncated、length、text 字段。
295+
- **Gotchas encountered:** 无
296+
- **Useful context:** WebSearchTool 需要配置 BRAVE_API_KEY 环境变量才能正常工作。WebFetchTool 支持多种内容类型:JSON(格式化)、HTML(文本提取)、纯文本。
297+
272298
---

pkg/tools/web_test.go

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
package tools
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"net/http"
7+
"net/http/httptest"
8+
"strings"
9+
"testing"
10+
)
11+
12+
// TestWebTool_WebFetch_Success verifies successful URL fetching
13+
func TestWebTool_WebFetch_Success(t *testing.T) {
14+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
15+
w.Header().Set("Content-Type", "text/html")
16+
w.WriteHeader(http.StatusOK)
17+
w.Write([]byte("<html><body><h1>Test Page</h1><p>Content here</p></body></html>"))
18+
}))
19+
defer server.Close()
20+
21+
tool := NewWebFetchTool(50000)
22+
ctx := context.Background()
23+
args := map[string]interface{}{
24+
"url": server.URL,
25+
}
26+
27+
result := tool.Execute(ctx, args)
28+
29+
// Success should not be an error
30+
if result.IsError {
31+
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
32+
}
33+
34+
// ForUser should contain the fetched content
35+
if !strings.Contains(result.ForUser, "Test Page") {
36+
t.Errorf("Expected ForUser to contain 'Test Page', got: %s", result.ForUser)
37+
}
38+
39+
// ForLLM should contain summary
40+
if !strings.Contains(result.ForLLM, "bytes") && !strings.Contains(result.ForLLM, "extractor") {
41+
t.Errorf("Expected ForLLM to contain summary, got: %s", result.ForLLM)
42+
}
43+
}
44+
45+
// TestWebTool_WebFetch_JSON verifies JSON content handling
46+
func TestWebTool_WebFetch_JSON(t *testing.T) {
47+
testData := map[string]string{"key": "value", "number": "123"}
48+
expectedJSON, _ := json.MarshalIndent(testData, "", " ")
49+
50+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
51+
w.Header().Set("Content-Type", "application/json")
52+
w.WriteHeader(http.StatusOK)
53+
w.Write(expectedJSON)
54+
}))
55+
defer server.Close()
56+
57+
tool := NewWebFetchTool(50000)
58+
ctx := context.Background()
59+
args := map[string]interface{}{
60+
"url": server.URL,
61+
}
62+
63+
result := tool.Execute(ctx, args)
64+
65+
// Success should not be an error
66+
if result.IsError {
67+
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
68+
}
69+
70+
// ForUser should contain formatted JSON
71+
if !strings.Contains(result.ForUser, "key") && !strings.Contains(result.ForUser, "value") {
72+
t.Errorf("Expected ForUser to contain JSON data, got: %s", result.ForUser)
73+
}
74+
}
75+
76+
// TestWebTool_WebFetch_InvalidURL verifies error handling for invalid URL
77+
func TestWebTool_WebFetch_InvalidURL(t *testing.T) {
78+
tool := NewWebFetchTool(50000)
79+
ctx := context.Background()
80+
args := map[string]interface{}{
81+
"url": "not-a-valid-url",
82+
}
83+
84+
result := tool.Execute(ctx, args)
85+
86+
// Should return error result
87+
if !result.IsError {
88+
t.Errorf("Expected error for invalid URL")
89+
}
90+
91+
// Should contain error message (either "invalid URL" or scheme error)
92+
if !strings.Contains(result.ForLLM, "URL") && !strings.Contains(result.ForUser, "URL") {
93+
t.Errorf("Expected error message for invalid URL, got ForLLM: %s", result.ForLLM)
94+
}
95+
}
96+
97+
// TestWebTool_WebFetch_UnsupportedScheme verifies error handling for non-http URLs
98+
func TestWebTool_WebFetch_UnsupportedScheme(t *testing.T) {
99+
tool := NewWebFetchTool(50000)
100+
ctx := context.Background()
101+
args := map[string]interface{}{
102+
"url": "ftp://example.com/file.txt",
103+
}
104+
105+
result := tool.Execute(ctx, args)
106+
107+
// Should return error result
108+
if !result.IsError {
109+
t.Errorf("Expected error for unsupported URL scheme")
110+
}
111+
112+
// Should mention only http/https allowed
113+
if !strings.Contains(result.ForLLM, "http/https") && !strings.Contains(result.ForUser, "http/https") {
114+
t.Errorf("Expected scheme error message, got ForLLM: %s", result.ForLLM)
115+
}
116+
}
117+
118+
// TestWebTool_WebFetch_MissingURL verifies error handling for missing URL
119+
func TestWebTool_WebFetch_MissingURL(t *testing.T) {
120+
tool := NewWebFetchTool(50000)
121+
ctx := context.Background()
122+
args := map[string]interface{}{}
123+
124+
result := tool.Execute(ctx, args)
125+
126+
// Should return error result
127+
if !result.IsError {
128+
t.Errorf("Expected error when URL is missing")
129+
}
130+
131+
// Should mention URL is required
132+
if !strings.Contains(result.ForLLM, "url is required") && !strings.Contains(result.ForUser, "url is required") {
133+
t.Errorf("Expected 'url is required' message, got ForLLM: %s", result.ForLLM)
134+
}
135+
}
136+
137+
// TestWebTool_WebFetch_Truncation verifies content truncation
138+
func TestWebTool_WebFetch_Truncation(t *testing.T) {
139+
longContent := strings.Repeat("x", 20000)
140+
141+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
142+
w.Header().Set("Content-Type", "text/plain")
143+
w.WriteHeader(http.StatusOK)
144+
w.Write([]byte(longContent))
145+
}))
146+
defer server.Close()
147+
148+
tool := NewWebFetchTool(1000) // Limit to 1000 chars
149+
ctx := context.Background()
150+
args := map[string]interface{}{
151+
"url": server.URL,
152+
}
153+
154+
result := tool.Execute(ctx, args)
155+
156+
// Success should not be an error
157+
if result.IsError {
158+
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
159+
}
160+
161+
// ForUser should contain truncated content (not the full 20000 chars)
162+
resultMap := make(map[string]interface{})
163+
json.Unmarshal([]byte(result.ForUser), &resultMap)
164+
if text, ok := resultMap["text"].(string); ok {
165+
if len(text) > 1100 { // Allow some margin
166+
t.Errorf("Expected content to be truncated to ~1000 chars, got: %d", len(text))
167+
}
168+
}
169+
170+
// Should be marked as truncated
171+
if truncated, ok := resultMap["truncated"].(bool); !ok || !truncated {
172+
t.Errorf("Expected 'truncated' to be true in result")
173+
}
174+
}
175+
176+
// TestWebTool_WebSearch_NoApiKey verifies error handling when API key is missing
177+
func TestWebTool_WebSearch_NoApiKey(t *testing.T) {
178+
tool := NewWebSearchTool("", 5)
179+
ctx := context.Background()
180+
args := map[string]interface{}{
181+
"query": "test",
182+
}
183+
184+
result := tool.Execute(ctx, args)
185+
186+
// Should return error result
187+
if !result.IsError {
188+
t.Errorf("Expected error when API key is missing")
189+
}
190+
191+
// Should mention missing API key
192+
if !strings.Contains(result.ForLLM, "BRAVE_API_KEY") && !strings.Contains(result.ForUser, "BRAVE_API_KEY") {
193+
t.Errorf("Expected API key error message, got ForLLM: %s", result.ForLLM)
194+
}
195+
}
196+
197+
// TestWebTool_WebSearch_MissingQuery verifies error handling for missing query
198+
func TestWebTool_WebSearch_MissingQuery(t *testing.T) {
199+
tool := NewWebSearchTool("test-key", 5)
200+
ctx := context.Background()
201+
args := map[string]interface{}{}
202+
203+
result := tool.Execute(ctx, args)
204+
205+
// Should return error result
206+
if !result.IsError {
207+
t.Errorf("Expected error when query is missing")
208+
}
209+
}
210+
211+
// TestWebTool_WebFetch_HTMLExtraction verifies HTML text extraction
212+
func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) {
213+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
214+
w.Header().Set("Content-Type", "text/html")
215+
w.WriteHeader(http.StatusOK)
216+
w.Write([]byte(`<html><body><script>alert('test');</script><style>body{color:red;}</style><h1>Title</h1><p>Content</p></body></html>`))
217+
}))
218+
defer server.Close()
219+
220+
tool := NewWebFetchTool(50000)
221+
ctx := context.Background()
222+
args := map[string]interface{}{
223+
"url": server.URL,
224+
}
225+
226+
result := tool.Execute(ctx, args)
227+
228+
// Success should not be an error
229+
if result.IsError {
230+
t.Errorf("Expected success, got IsError=true: %s", result.ForLLM)
231+
}
232+
233+
// ForUser should contain extracted text (without script/style tags)
234+
if !strings.Contains(result.ForUser, "Title") && !strings.Contains(result.ForUser, "Content") {
235+
t.Errorf("Expected ForUser to contain extracted text, got: %s", result.ForUser)
236+
}
237+
238+
// Should NOT contain script or style tags
239+
if strings.Contains(result.ForUser, "<script>") || strings.Contains(result.ForUser, "<style>") {
240+
t.Errorf("Expected script/style tags to be removed, got: %s", result.ForUser)
241+
}
242+
}
243+
244+
// TestWebTool_WebFetch_MissingDomain verifies error handling for URL without domain
245+
func TestWebTool_WebFetch_MissingDomain(t *testing.T) {
246+
tool := NewWebFetchTool(50000)
247+
ctx := context.Background()
248+
args := map[string]interface{}{
249+
"url": "https://",
250+
}
251+
252+
result := tool.Execute(ctx, args)
253+
254+
// Should return error result
255+
if !result.IsError {
256+
t.Errorf("Expected error for URL without domain")
257+
}
258+
259+
// Should mention missing domain
260+
if !strings.Contains(result.ForLLM, "domain") && !strings.Contains(result.ForUser, "domain") {
261+
t.Errorf("Expected domain error message, got ForLLM: %s", result.ForLLM)
262+
}
263+
}

0 commit comments

Comments
 (0)