mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-07-09 17:28:58 +00:00
test: expand unit test coverage for agent, llm, llmloop, and tool packages
Add integration-style tests with fake LLM clients for agent dispatch and llmloop runner, plus new unit test files for gitcmd, session/history, tool/code_comment, tool/filereader_read, and viewer/store packages.
This commit is contained in:
kite
parent
98fe309f03
commit
904ecd3ae1
9 changed files with 1604 additions and 0 deletions
|
|
@ -1,15 +1,88 @@
|
|||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/config/template"
|
||||
"github.com/open-code-review/open-code-review/internal/config/toolsconfig"
|
||||
"github.com/open-code-review/open-code-review/internal/llm"
|
||||
"github.com/open-code-review/open-code-review/internal/model"
|
||||
"github.com/open-code-review/open-code-review/internal/tool"
|
||||
)
|
||||
|
||||
type fakeAgentClient struct {
|
||||
responses []*llm.ChatResponse
|
||||
calls int
|
||||
}
|
||||
|
||||
func (f *fakeAgentClient) CompletionsWithCtx(_ context.Context, _ llm.ChatRequest) (*llm.ChatResponse, error) {
|
||||
if f.calls >= len(f.responses) {
|
||||
content := ""
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{Message: llm.ResponseMessage{Content: &content}}},
|
||||
Model: "fake",
|
||||
}, nil
|
||||
}
|
||||
resp := f.responses[f.calls]
|
||||
f.calls++
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func agentTaskDoneResponse() *llm.ChatResponse {
|
||||
content := ""
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
ToolCalls: []llm.ToolCall{{
|
||||
ID: "call_done",
|
||||
Type: "function",
|
||||
Function: llm.FunctionCall{
|
||||
Name: "task_done",
|
||||
Arguments: `{}`,
|
||||
},
|
||||
}},
|
||||
},
|
||||
}},
|
||||
Model: "fake",
|
||||
Usage: &llm.UsageInfo{PromptTokens: 10, CompletionTokens: 5},
|
||||
}
|
||||
}
|
||||
|
||||
func codeCommentResponse(path string) *llm.ChatResponse {
|
||||
content := ""
|
||||
args := map[string]any{
|
||||
"path": path,
|
||||
"comments": []any{
|
||||
map[string]any{
|
||||
"content": "potential null pointer",
|
||||
"existing_code": "foo := bar.Baz()",
|
||||
},
|
||||
},
|
||||
}
|
||||
argsJSON, _ := json.Marshal(args)
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
ToolCalls: []llm.ToolCall{{
|
||||
ID: "call_comment",
|
||||
Type: "function",
|
||||
Function: llm.FunctionCall{
|
||||
Name: "code_comment",
|
||||
Arguments: string(argsJSON),
|
||||
},
|
||||
}},
|
||||
},
|
||||
}},
|
||||
Model: "fake",
|
||||
Usage: &llm.UsageInfo{PromptTokens: 50, CompletionTokens: 20},
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildFilterCommentsJSON(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
|
@ -302,3 +375,195 @@ func TestBuildToolDefs(t *testing.T) {
|
|||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestFilterLargeDiffs(t *testing.T) {
|
||||
a := New(Args{
|
||||
Template: template.Template{MaxTokens: 100},
|
||||
})
|
||||
|
||||
diffs := []model.Diff{
|
||||
{NewPath: "small.go", Diff: "short diff"},
|
||||
{NewPath: "large.go", Diff: strings.Repeat("word ", 500)},
|
||||
}
|
||||
|
||||
kept := a.filterLargeDiffs(diffs)
|
||||
if len(kept) != 1 {
|
||||
t.Fatalf("expected 1 kept diff, got %d", len(kept))
|
||||
}
|
||||
if kept[0].NewPath != "small.go" {
|
||||
t.Errorf("kept wrong file: %s", kept[0].NewPath)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterLargeDiffs_ZeroMaxTokens(t *testing.T) {
|
||||
a := New(Args{
|
||||
Template: template.Template{MaxTokens: 0},
|
||||
})
|
||||
|
||||
diffs := []model.Diff{{NewPath: "a.go", Diff: "some diff"}}
|
||||
kept := a.filterLargeDiffs(diffs)
|
||||
if len(kept) != 1 {
|
||||
t.Errorf("expected all kept when MaxTokens=0, got %d", len(kept))
|
||||
}
|
||||
}
|
||||
|
||||
func TestCountReviewable(t *testing.T) {
|
||||
a := New(Args{})
|
||||
diffs := []model.Diff{
|
||||
{NewPath: "main.go", Insertions: 10, Deletions: 2},
|
||||
{NewPath: "deleted.go", IsDeleted: true, Deletions: 20},
|
||||
{NewPath: "binary.bin", IsBinary: true},
|
||||
{NewPath: "helper.go", Insertions: 5},
|
||||
}
|
||||
|
||||
count := a.countReviewable(diffs)
|
||||
if count != 2 {
|
||||
t.Errorf("countReviewable = %d, want 2", count)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildChangeFilesExcept(t *testing.T) {
|
||||
a := New(Args{})
|
||||
a.diffs = []model.Diff{
|
||||
{NewPath: "main.go", OldPath: "main.go"},
|
||||
{NewPath: "helper.go", OldPath: "helper.go", IsNew: true},
|
||||
{NewPath: "removed.go", OldPath: "removed.go", IsDeleted: true},
|
||||
{NewPath: "renamed.go", OldPath: "old_name.go"},
|
||||
{NewPath: "bin.dat", OldPath: "bin.dat", IsBinary: true},
|
||||
}
|
||||
|
||||
got := a.buildChangeFilesExcept("main.go")
|
||||
if strings.Contains(got, "main.go") {
|
||||
t.Error("excluded file should not appear")
|
||||
}
|
||||
if !strings.Contains(got, "ADDED") {
|
||||
t.Error("expected ADDED status for new file")
|
||||
}
|
||||
if !strings.Contains(got, "DELETED") {
|
||||
t.Error("expected DELETED status")
|
||||
}
|
||||
if !strings.Contains(got, "RENAMED") {
|
||||
t.Error("expected RENAMED status")
|
||||
}
|
||||
if strings.Contains(got, "bin.dat") {
|
||||
t.Error("binary files should be skipped")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDispatchSubtasks_WithFakeLLM(t *testing.T) {
|
||||
client := &fakeAgentClient{responses: []*llm.ChatResponse{
|
||||
codeCommentResponse("main.go"),
|
||||
agentTaskDoneResponse(),
|
||||
}}
|
||||
|
||||
collector := tool.NewCommentCollector()
|
||||
reg := tool.NewRegistry()
|
||||
reg.Register(&tool.CodeCommentProvider{Collector: collector})
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "fake",
|
||||
CommentCollector: collector,
|
||||
Tools: reg,
|
||||
Template: template.Template{
|
||||
MaxTokens: 100000,
|
||||
MaxToolRequestTimes: 10,
|
||||
MainTask: template.LlmConversation{
|
||||
Messages: []template.ChatMessage{
|
||||
{Role: "user", Content: "Review {{diff}} for {{current_file_path}}"},
|
||||
},
|
||||
},
|
||||
},
|
||||
MainToolDefs: []llm.ToolDef{
|
||||
{Type: "function", Function: llm.FunctionDef{Name: "task_done", Description: "done"}},
|
||||
{Type: "function", Function: llm.FunctionDef{Name: "code_comment", Description: "comment"}},
|
||||
},
|
||||
})
|
||||
|
||||
a.diffs = []model.Diff{
|
||||
{NewPath: "main.go", OldPath: "main.go", Diff: "+new line", Insertions: 1},
|
||||
}
|
||||
a.currentDate = "2025-06-26 10:00"
|
||||
|
||||
comments, err := a.dispatchSubtasks(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("dispatchSubtasks: %v", err)
|
||||
}
|
||||
if len(comments) != 1 {
|
||||
t.Fatalf("expected 1 comment, got %d", len(comments))
|
||||
}
|
||||
if comments[0].Path != "main.go" {
|
||||
t.Errorf("Path = %q, want main.go", comments[0].Path)
|
||||
}
|
||||
if !strings.Contains(comments[0].Content, "null pointer") {
|
||||
t.Errorf("Content = %q", comments[0].Content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDispatchSubtasks_AllDeleted(t *testing.T) {
|
||||
client := &fakeAgentClient{}
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "fake",
|
||||
Template: template.Template{
|
||||
MaxTokens: 100000,
|
||||
MaxToolRequestTimes: 5,
|
||||
MainTask: template.LlmConversation{
|
||||
Messages: []template.ChatMessage{
|
||||
{Role: "user", Content: "Review {{diff}}"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
a.diffs = []model.Diff{
|
||||
{NewPath: "removed.go", IsDeleted: true},
|
||||
}
|
||||
a.currentDate = "2025-06-26 10:00"
|
||||
|
||||
comments, err := a.dispatchSubtasks(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("dispatchSubtasks: %v", err)
|
||||
}
|
||||
if len(comments) != 0 {
|
||||
t.Errorf("expected 0 comments for deleted file, got %d", len(comments))
|
||||
}
|
||||
if client.calls != 0 {
|
||||
t.Errorf("expected 0 LLM calls, got %d", client.calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAgent_TokenAccumulation(t *testing.T) {
|
||||
client := &fakeAgentClient{responses: []*llm.ChatResponse{
|
||||
agentTaskDoneResponse(),
|
||||
}}
|
||||
|
||||
a := New(Args{
|
||||
LLMClient: client,
|
||||
Model: "fake",
|
||||
Template: template.Template{
|
||||
MaxTokens: 100000,
|
||||
MaxToolRequestTimes: 10,
|
||||
MainTask: template.LlmConversation{
|
||||
Messages: []template.ChatMessage{
|
||||
{Role: "user", Content: "Review {{diff}}"},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
a.diffs = []model.Diff{
|
||||
{NewPath: "a.go", Diff: "+x", Insertions: 1},
|
||||
}
|
||||
a.currentDate = "2025-06-26 10:00"
|
||||
|
||||
_, err := a.dispatchSubtasks(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if a.TotalInputTokens() != 10 {
|
||||
t.Errorf("TotalInputTokens = %d, want 10", a.TotalInputTokens())
|
||||
}
|
||||
if a.TotalOutputTokens() != 5 {
|
||||
t.Errorf("TotalOutputTokens = %d, want 5", a.TotalOutputTokens())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
151
internal/gitcmd/runner_test.go
Normal file
151
internal/gitcmd/runner_test.go
Normal file
|
|
@ -0,0 +1,151 @@
|
|||
package gitcmd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func initRepo(t *testing.T) string {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
run := func(args ...string) {
|
||||
cmd := exec.Command("git", args...)
|
||||
cmd.Dir = dir
|
||||
cmd.Env = append(os.Environ(),
|
||||
"GIT_AUTHOR_NAME=test",
|
||||
"GIT_AUTHOR_EMAIL=test@test.com",
|
||||
"GIT_COMMITTER_NAME=test",
|
||||
"GIT_COMMITTER_EMAIL=test@test.com",
|
||||
)
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
t.Fatalf("git %v: %v\n%s", args, err, out)
|
||||
}
|
||||
}
|
||||
run("init")
|
||||
run("config", "user.email", "test@test.com")
|
||||
run("config", "user.name", "test")
|
||||
if err := os.WriteFile(filepath.Join(dir, "hello.txt"), []byte("hello\n"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
run("add", "hello.txt")
|
||||
run("commit", "-m", "init")
|
||||
return dir
|
||||
}
|
||||
|
||||
func TestRunner_New(t *testing.T) {
|
||||
r := New(0)
|
||||
if r == nil {
|
||||
t.Fatal("New(0) returned nil")
|
||||
}
|
||||
if cap(r.sem) != defaultMaxConcurrent {
|
||||
t.Errorf("default capacity = %d, want %d", cap(r.sem), defaultMaxConcurrent)
|
||||
}
|
||||
|
||||
r2 := New(4)
|
||||
if cap(r2.sem) != 4 {
|
||||
t.Errorf("capacity = %d, want 4", cap(r2.sem))
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_Run(t *testing.T) {
|
||||
dir := initRepo(t)
|
||||
r := New(2)
|
||||
|
||||
out, err := r.Run(context.Background(), dir, "log", "--oneline")
|
||||
if err != nil {
|
||||
t.Fatalf("Run error: %v", err)
|
||||
}
|
||||
if !strings.Contains(out, "init") {
|
||||
t.Errorf("expected 'init' in output: %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_Run_InvalidCommand(t *testing.T) {
|
||||
dir := initRepo(t)
|
||||
r := New(2)
|
||||
|
||||
_, err := r.Run(context.Background(), dir, "nonexistent-subcommand")
|
||||
if err == nil {
|
||||
t.Error("expected error for invalid git subcommand")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_Output(t *testing.T) {
|
||||
dir := initRepo(t)
|
||||
r := New(2)
|
||||
|
||||
out, err := r.Output(context.Background(), dir, "rev-parse", "HEAD")
|
||||
if err != nil {
|
||||
t.Fatalf("Output error: %v", err)
|
||||
}
|
||||
hash := strings.TrimSpace(string(out))
|
||||
if len(hash) != 40 {
|
||||
t.Errorf("expected 40-char hash, got %q", hash)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_RunSplit(t *testing.T) {
|
||||
dir := initRepo(t)
|
||||
r := New(2)
|
||||
|
||||
stdout, stderr, err := r.RunSplit(context.Background(), dir, "status", "--short")
|
||||
if err != nil {
|
||||
t.Fatalf("RunSplit error: %v", err)
|
||||
}
|
||||
_ = stderr
|
||||
if strings.Contains(stdout, "??") {
|
||||
t.Errorf("unexpected untracked files in clean repo: %q", stdout)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_Stream(t *testing.T) {
|
||||
dir := initRepo(t)
|
||||
r := New(2)
|
||||
|
||||
var content string
|
||||
err := r.Stream(context.Background(), dir, func(stdout io.Reader) error {
|
||||
data, err := io.ReadAll(stdout)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
content = string(data)
|
||||
return nil
|
||||
}, "show", "HEAD:hello.txt")
|
||||
if err != nil {
|
||||
t.Fatalf("Stream error: %v", err)
|
||||
}
|
||||
if content != "hello\n" {
|
||||
t.Errorf("Stream content = %q, want %q", content, "hello\n")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_ContextCancelled(t *testing.T) {
|
||||
r := New(1)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
_, err := r.Run(ctx, ".", "status")
|
||||
if err == nil {
|
||||
t.Error("expected error for cancelled context")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_AcquireTimeout(t *testing.T) {
|
||||
r := New(1)
|
||||
r.sem <- struct{}{}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||
defer cancel()
|
||||
|
||||
_, err := r.Run(ctx, ".", "status")
|
||||
if err == nil {
|
||||
t.Error("expected timeout error when semaphore full")
|
||||
}
|
||||
}
|
||||
|
|
@ -482,3 +482,90 @@ func TestAnthropicClient_NoExtraHeadersWhenEmpty(t *testing.T) {
|
|||
|
||||
// Verify the SDK constant is accessible (compile-time check).
|
||||
var _ anthropic.CacheControlEphemeralParam = anthropic.NewCacheControlEphemeralParam()
|
||||
|
||||
func TestCountTokens(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
text string
|
||||
want int
|
||||
}{
|
||||
{"empty", "", 0},
|
||||
{"single word", "hello", 1},
|
||||
{"sentence", "The quick brown fox jumps over the lazy dog.", 10},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := CountTokens(tt.text)
|
||||
if tt.want == 0 && got != 0 {
|
||||
t.Errorf("CountTokens(%q) = %d, want 0", tt.text, got)
|
||||
}
|
||||
if tt.want > 0 && got == 0 {
|
||||
t.Errorf("CountTokens(%q) = 0, expected > 0", tt.text)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCountTokensForModel(t *testing.T) {
|
||||
text := "Hello, world! This is a test."
|
||||
base := CountTokensForModel(text, "gpt-4")
|
||||
o1 := CountTokensForModel(text, "o1-mini")
|
||||
|
||||
if base == 0 {
|
||||
t.Error("cl100k_base should produce non-zero tokens")
|
||||
}
|
||||
if o1 == 0 {
|
||||
t.Error("o200k_base should produce non-zero tokens")
|
||||
}
|
||||
|
||||
if CountTokensForModel("", "gpt-4") != 0 {
|
||||
t.Error("empty text should return 0")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEncodingForModel(t *testing.T) {
|
||||
tests := []struct {
|
||||
model string
|
||||
want string
|
||||
}{
|
||||
{"gpt-4", "cl100k_base"},
|
||||
{"claude-3-opus", "cl100k_base"},
|
||||
{"", "cl100k_base"},
|
||||
{"o1-preview", "o200k_base"},
|
||||
{"o3-mini", "o200k_base"},
|
||||
{"o4-mini", "o200k_base"},
|
||||
{"GPT-O1", "o200k_base"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.model, func(t *testing.T) {
|
||||
got := encodingForModel(tt.model)
|
||||
if got != tt.want {
|
||||
t.Errorf("encodingForModel(%q) = %q, want %q", tt.model, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripThinkTags(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
{"no tags", "hello world", "hello world"},
|
||||
{"open only", "<think>partial", "partial"},
|
||||
{"close only", "partial</think>", "partial"},
|
||||
{"both tags", "<think>reasoning here</think>answer", "reasoning hereanswer"},
|
||||
{"multiple tags", "<think>a</think>b<think>c</think>d", "abcd"},
|
||||
{"empty", "", ""},
|
||||
{"tags only", "<think></think>", ""},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := stripThinkTags(tt.input)
|
||||
if got != tt.want {
|
||||
t.Errorf("stripThinkTags(%q) = %q, want %q", tt.input, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,10 +5,237 @@ import (
|
|||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/config/template"
|
||||
"github.com/open-code-review/open-code-review/internal/llm"
|
||||
"github.com/open-code-review/open-code-review/internal/session"
|
||||
"github.com/open-code-review/open-code-review/internal/tool"
|
||||
)
|
||||
|
||||
type fakeClient struct {
|
||||
responses []*llm.ChatResponse
|
||||
calls int
|
||||
}
|
||||
|
||||
func (f *fakeClient) CompletionsWithCtx(_ context.Context, _ llm.ChatRequest) (*llm.ChatResponse, error) {
|
||||
if f.calls >= len(f.responses) {
|
||||
content := ""
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{Message: llm.ResponseMessage{Content: &content}}},
|
||||
Model: "fake",
|
||||
}, nil
|
||||
}
|
||||
resp := f.responses[f.calls]
|
||||
f.calls++
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func taskDoneResponse() *llm.ChatResponse {
|
||||
content := ""
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
ToolCalls: []llm.ToolCall{{
|
||||
ID: "call_1",
|
||||
Type: "function",
|
||||
Function: llm.FunctionCall{
|
||||
Name: "task_done",
|
||||
Arguments: `{}`,
|
||||
},
|
||||
}},
|
||||
},
|
||||
}},
|
||||
Model: "fake",
|
||||
Usage: &llm.UsageInfo{PromptTokens: 10, CompletionTokens: 5},
|
||||
}
|
||||
}
|
||||
|
||||
func fileReadToolCallResponse(callID, args string) *llm.ChatResponse {
|
||||
content := ""
|
||||
return &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
ToolCalls: []llm.ToolCall{{
|
||||
ID: callID,
|
||||
Type: "function",
|
||||
Function: llm.FunctionCall{
|
||||
Name: "file_read",
|
||||
Arguments: args,
|
||||
},
|
||||
}},
|
||||
},
|
||||
}},
|
||||
Model: "fake",
|
||||
Usage: &llm.UsageInfo{PromptTokens: 20, CompletionTokens: 10},
|
||||
}
|
||||
}
|
||||
|
||||
type fakeFileReadProvider struct {
|
||||
result string
|
||||
}
|
||||
|
||||
func (f *fakeFileReadProvider) Tool() tool.Tool { return tool.FileRead }
|
||||
func (f *fakeFileReadProvider) Execute(_ context.Context, _ map[string]any) (string, error) {
|
||||
return f.result, nil
|
||||
}
|
||||
|
||||
func newTestDeps(client llm.LLMClient) Deps {
|
||||
reg := tool.NewRegistry()
|
||||
reg.Register(&fakeFileReadProvider{result: "package main\n"})
|
||||
return Deps{
|
||||
LLMClient: client,
|
||||
Model: "fake",
|
||||
Template: template.Template{MaxTokens: 100000, MaxToolRequestTimes: 10},
|
||||
Tools: reg,
|
||||
CommentCollector: tool.NewCommentCollector(),
|
||||
Session: session.New("/tmp/test-repo", "main", "fake", session.SessionOptions{}),
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPerFile_TaskDoneImmediately(t *testing.T) {
|
||||
client := &fakeClient{responses: []*llm.ChatResponse{taskDoneResponse()}}
|
||||
deps := newTestDeps(client)
|
||||
runner := NewRunner(deps)
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "review this file")}
|
||||
err := runner.RunPerFile(context.Background(), msgs, "main.go")
|
||||
if err != nil {
|
||||
t.Fatalf("RunPerFile: %v", err)
|
||||
}
|
||||
if client.calls != 1 {
|
||||
t.Errorf("expected 1 LLM call, got %d", client.calls)
|
||||
}
|
||||
if runner.TotalInputTokens() != 10 {
|
||||
t.Errorf("TotalInputTokens = %d, want 10", runner.TotalInputTokens())
|
||||
}
|
||||
if runner.TotalOutputTokens() != 5 {
|
||||
t.Errorf("TotalOutputTokens = %d, want 5", runner.TotalOutputTokens())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPerFile_ToolCallThenDone(t *testing.T) {
|
||||
client := &fakeClient{responses: []*llm.ChatResponse{
|
||||
fileReadToolCallResponse("call_1", `{"path":"main.go"}`),
|
||||
taskDoneResponse(),
|
||||
}}
|
||||
deps := newTestDeps(client)
|
||||
runner := NewRunner(deps)
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "review")}
|
||||
err := runner.RunPerFile(context.Background(), msgs, "main.go")
|
||||
if err != nil {
|
||||
t.Fatalf("RunPerFile: %v", err)
|
||||
}
|
||||
if client.calls != 2 {
|
||||
t.Errorf("expected 2 LLM calls, got %d", client.calls)
|
||||
}
|
||||
|
||||
toolCalls := runner.ToolCalls()
|
||||
if toolCalls["file_read"] != 1 {
|
||||
t.Errorf("file_read calls = %d, want 1", toolCalls["file_read"])
|
||||
}
|
||||
if runner.TotalInputTokens() != 30 {
|
||||
t.Errorf("TotalInputTokens = %d, want 30", runner.TotalInputTokens())
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPerFile_ContextCancelled(t *testing.T) {
|
||||
client := &fakeClient{responses: []*llm.ChatResponse{taskDoneResponse()}}
|
||||
deps := newTestDeps(client)
|
||||
runner := NewRunner(deps)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "review")}
|
||||
err := runner.RunPerFile(ctx, msgs, "main.go")
|
||||
if err == nil {
|
||||
t.Error("expected error for cancelled context")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunPerFile_UnknownTool(t *testing.T) {
|
||||
content := ""
|
||||
unknownToolResp := &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
ToolCalls: []llm.ToolCall{{
|
||||
ID: "call_x",
|
||||
Type: "function",
|
||||
Function: llm.FunctionCall{
|
||||
Name: "nonexistent_tool",
|
||||
Arguments: `{}`,
|
||||
},
|
||||
}},
|
||||
},
|
||||
}},
|
||||
Model: "fake",
|
||||
Usage: &llm.UsageInfo{PromptTokens: 5, CompletionTokens: 5},
|
||||
}
|
||||
client := &fakeClient{responses: []*llm.ChatResponse{unknownToolResp, taskDoneResponse()}}
|
||||
deps := newTestDeps(client)
|
||||
runner := NewRunner(deps)
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "review")}
|
||||
err := runner.RunPerFile(context.Background(), msgs, "main.go")
|
||||
if err != nil {
|
||||
t.Fatalf("RunPerFile: %v", err)
|
||||
}
|
||||
if client.calls != 2 {
|
||||
t.Errorf("expected 2 calls, got %d", client.calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_RecordWarning(t *testing.T) {
|
||||
deps := newTestDeps(&fakeClient{})
|
||||
runner := NewRunner(deps)
|
||||
|
||||
runner.RecordWarning("token_limit", "a.go", "approaching token limit")
|
||||
runner.RecordWarning("parse_error", "b.go", "invalid JSON")
|
||||
|
||||
warnings := runner.Warnings()
|
||||
if len(warnings) != 2 {
|
||||
t.Fatalf("expected 2 warnings, got %d", len(warnings))
|
||||
}
|
||||
if warnings[0].Type != "token_limit" {
|
||||
t.Errorf("Type = %q", warnings[0].Type)
|
||||
}
|
||||
if warnings[1].File != "b.go" {
|
||||
t.Errorf("File = %q", warnings[1].File)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunner_RecordUsage(t *testing.T) {
|
||||
deps := newTestDeps(&fakeClient{})
|
||||
runner := NewRunner(deps)
|
||||
|
||||
runner.RecordUsage(&llm.UsageInfo{
|
||||
PromptTokens: 100,
|
||||
CompletionTokens: 50,
|
||||
CacheReadTokens: 20,
|
||||
CacheWriteTokens: 10,
|
||||
})
|
||||
runner.RecordUsage(nil)
|
||||
|
||||
if runner.TotalInputTokens() != 100 {
|
||||
t.Errorf("input = %d", runner.TotalInputTokens())
|
||||
}
|
||||
if runner.TotalOutputTokens() != 50 {
|
||||
t.Errorf("output = %d", runner.TotalOutputTokens())
|
||||
}
|
||||
if runner.TotalCacheReadTokens() != 20 {
|
||||
t.Errorf("cache read = %d", runner.TotalCacheReadTokens())
|
||||
}
|
||||
if runner.TotalCacheWriteTokens() != 10 {
|
||||
t.Errorf("cache write = %d", runner.TotalCacheWriteTokens())
|
||||
}
|
||||
if runner.TotalTokensUsed() != 150 {
|
||||
t.Errorf("total = %d", runner.TotalTokensUsed())
|
||||
}
|
||||
}
|
||||
|
||||
func TestExecuteToolCall_CodeCommentOverridesHallucinatedPath(t *testing.T) {
|
||||
collector := tool.NewCommentCollector()
|
||||
reg := tool.NewRegistry()
|
||||
|
|
|
|||
99
internal/llmloop/pool_test.go
Normal file
99
internal/llmloop/pool_test.go
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
package llmloop
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/model"
|
||||
)
|
||||
|
||||
func TestNewCommentWorkerPool_Default(t *testing.T) {
|
||||
p := NewCommentWorkerPool(0)
|
||||
if cap(p.semaphore) != 8 {
|
||||
t.Errorf("default capacity = %d, want 8", cap(p.semaphore))
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewCommentWorkerPool_Custom(t *testing.T) {
|
||||
p := NewCommentWorkerPool(4)
|
||||
if cap(p.semaphore) != 4 {
|
||||
t.Errorf("capacity = %d, want 4", cap(p.semaphore))
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommentWorkerPool_SubmitAndAwait(t *testing.T) {
|
||||
p := NewCommentWorkerPool(2)
|
||||
|
||||
p.Submit(func() ([]model.LlmComment, error) {
|
||||
return []model.LlmComment{{Path: "a.go", Content: "issue 1"}}, nil
|
||||
})
|
||||
p.Submit(func() ([]model.LlmComment, error) {
|
||||
return []model.LlmComment{{Path: "b.go", Content: "issue 2"}, {Path: "b.go", Content: "issue 3"}}, nil
|
||||
})
|
||||
|
||||
results := p.Await()
|
||||
if len(results) != 3 {
|
||||
t.Fatalf("expected 3 results, got %d", len(results))
|
||||
}
|
||||
|
||||
paths := map[string]bool{}
|
||||
for _, r := range results {
|
||||
paths[r.Path] = true
|
||||
}
|
||||
if !paths["a.go"] || !paths["b.go"] {
|
||||
t.Errorf("unexpected paths: %v", results)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommentWorkerPool_ErrorDoesNotBlock(t *testing.T) {
|
||||
p := NewCommentWorkerPool(2)
|
||||
|
||||
p.Submit(func() ([]model.LlmComment, error) {
|
||||
return nil, errors.New("oops")
|
||||
})
|
||||
p.Submit(func() ([]model.LlmComment, error) {
|
||||
return []model.LlmComment{{Path: "ok.go", Content: "fine"}}, nil
|
||||
})
|
||||
|
||||
results := p.Await()
|
||||
if len(results) != 1 {
|
||||
t.Fatalf("expected 1 result after error, got %d", len(results))
|
||||
}
|
||||
if results[0].Path != "ok.go" {
|
||||
t.Errorf("Path = %q", results[0].Path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommentWorkerPool_Concurrency(t *testing.T) {
|
||||
p := NewCommentWorkerPool(3)
|
||||
var running atomic.Int32
|
||||
var maxRunning atomic.Int32
|
||||
|
||||
for i := 0; i < 10; i++ {
|
||||
p.Submit(func() ([]model.LlmComment, error) {
|
||||
cur := running.Add(1)
|
||||
for {
|
||||
old := maxRunning.Load()
|
||||
if cur <= old || maxRunning.CompareAndSwap(old, cur) {
|
||||
break
|
||||
}
|
||||
}
|
||||
running.Add(-1)
|
||||
return nil, nil
|
||||
})
|
||||
}
|
||||
|
||||
p.Await()
|
||||
if maxRunning.Load() > 3 {
|
||||
t.Errorf("max concurrent = %d, expected <= 3", maxRunning.Load())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommentWorkerPool_AwaitEmpty(t *testing.T) {
|
||||
p := NewCommentWorkerPool(2)
|
||||
results := p.Await()
|
||||
if results != nil {
|
||||
t.Errorf("expected nil for no submissions, got %v", results)
|
||||
}
|
||||
}
|
||||
214
internal/session/history_test.go
Normal file
214
internal/session/history_test.go
Normal file
|
|
@ -0,0 +1,214 @@
|
|||
package session
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/open-code-review/open-code-review/internal/llm"
|
||||
)
|
||||
|
||||
func TestNew(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "gpt-4", SessionOptions{
|
||||
ReviewMode: ReviewModeWorkspace,
|
||||
DiffFrom: "a",
|
||||
DiffTo: "b",
|
||||
DiffCommit: "c",
|
||||
})
|
||||
if sh == nil {
|
||||
t.Fatal("New returned nil")
|
||||
}
|
||||
if sh.SessionID == "" {
|
||||
t.Error("SessionID should not be empty")
|
||||
}
|
||||
if sh.RepoDir != "/tmp/repo" {
|
||||
t.Errorf("RepoDir = %q", sh.RepoDir)
|
||||
}
|
||||
if sh.GitBranch != "main" {
|
||||
t.Errorf("GitBranch = %q", sh.GitBranch)
|
||||
}
|
||||
if sh.Model != "gpt-4" {
|
||||
t.Errorf("Model = %q", sh.Model)
|
||||
}
|
||||
if sh.ReviewMode != ReviewModeWorkspace {
|
||||
t.Errorf("ReviewMode = %q", sh.ReviewMode)
|
||||
}
|
||||
if sh.DiffFrom != "a" || sh.DiffTo != "b" || sh.DiffCommit != "c" {
|
||||
t.Errorf("Diff fields mismatch")
|
||||
}
|
||||
if sh.StartTime.IsZero() {
|
||||
t.Error("StartTime should be set")
|
||||
}
|
||||
if sh.FileSessions == nil {
|
||||
t.Error("FileSessions map should be initialized")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetOrCreateFileSession(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
|
||||
fs1 := sh.GetOrCreateFileSession("main.go")
|
||||
if fs1 == nil {
|
||||
t.Fatal("nil FileSession")
|
||||
}
|
||||
if fs1.FilePath != "main.go" {
|
||||
t.Errorf("FilePath = %q", fs1.FilePath)
|
||||
}
|
||||
|
||||
fs2 := sh.GetOrCreateFileSession("main.go")
|
||||
if fs1 != fs2 {
|
||||
t.Error("expected same FileSession instance on second call")
|
||||
}
|
||||
|
||||
fs3 := sh.GetOrCreateFileSession("other.go")
|
||||
if fs3 == fs1 {
|
||||
t.Error("different paths should yield different sessions")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendTaskRecord(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "hello")}
|
||||
rec := fs.AppendTaskRecord(MainTask, msgs)
|
||||
if rec == nil {
|
||||
t.Fatal("nil TaskRecord")
|
||||
}
|
||||
if rec.Type != MainTask {
|
||||
t.Errorf("Type = %v", rec.Type)
|
||||
}
|
||||
if rec.RequestNo != 1 {
|
||||
t.Errorf("RequestNo = %d, want 1", rec.RequestNo)
|
||||
}
|
||||
|
||||
rec2 := fs.AppendTaskRecord(MainTask, msgs)
|
||||
if rec2.RequestNo != 2 {
|
||||
t.Errorf("second RequestNo = %d, want 2", rec2.RequestNo)
|
||||
}
|
||||
|
||||
rec3 := fs.AppendTaskRecord(PlanTask, msgs)
|
||||
if rec3.RequestNo != 1 {
|
||||
t.Errorf("PlanTask RequestNo = %d, want 1 (separate counter)", rec3.RequestNo)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendTaskRecord_DefensiveCopy(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
|
||||
msgs := []llm.Message{llm.NewTextMessage("user", "original")}
|
||||
rec := fs.AppendTaskRecord(MainTask, msgs)
|
||||
msgs[0] = llm.NewTextMessage("user", "mutated")
|
||||
|
||||
if rec.RequestMessages[0].ExtractText() == "mutated" {
|
||||
t.Error("AppendTaskRecord should store a copy of messages")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetResponse(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
rec := fs.AppendTaskRecord(MainTask, []llm.Message{llm.NewTextMessage("user", "hi")})
|
||||
|
||||
content := "response text"
|
||||
resp := &llm.ChatResponse{
|
||||
Choices: []llm.Choice{{
|
||||
Message: llm.ResponseMessage{
|
||||
Content: &content,
|
||||
},
|
||||
}},
|
||||
Model: "gpt-4",
|
||||
Usage: &llm.UsageInfo{
|
||||
PromptTokens: 100,
|
||||
CompletionTokens: 50,
|
||||
},
|
||||
}
|
||||
|
||||
rec.SetResponse(resp, 2*time.Second)
|
||||
|
||||
if rec.Response == nil {
|
||||
t.Fatal("Response should be set")
|
||||
}
|
||||
if rec.Response.Content != "response text" {
|
||||
t.Errorf("Content = %q", rec.Response.Content)
|
||||
}
|
||||
if rec.Response.Model != "gpt-4" {
|
||||
t.Errorf("Model = %q", rec.Response.Model)
|
||||
}
|
||||
if rec.Response.Usage.PromptTokens != 100 {
|
||||
t.Errorf("PromptTokens = %d", rec.Response.Usage.PromptTokens)
|
||||
}
|
||||
if rec.Response.Usage.CompletionTokens != 50 {
|
||||
t.Errorf("CompletionTokens = %d", rec.Response.Usage.CompletionTokens)
|
||||
}
|
||||
if rec.Duration != 2*time.Second {
|
||||
t.Errorf("Duration = %v", rec.Duration)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetResponse_EmptyResponse(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
rec := fs.AppendTaskRecord(MainTask, []llm.Message{llm.NewTextMessage("user", "hi")})
|
||||
|
||||
rec.SetResponse(nil, time.Second)
|
||||
if rec.Error == "" {
|
||||
t.Error("expected error for nil response")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetError(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
rec := fs.AppendTaskRecord(MainTask, []llm.Message{llm.NewTextMessage("user", "hi")})
|
||||
|
||||
rec.SetError(errors.New("timeout"), 5*time.Second)
|
||||
|
||||
if rec.Error != "timeout" {
|
||||
t.Errorf("Error = %q, want %q", rec.Error, "timeout")
|
||||
}
|
||||
if rec.Duration != 5*time.Second {
|
||||
t.Errorf("Duration = %v", rec.Duration)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLLMFailures(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
if sh.LLMFailures() != 0 {
|
||||
t.Errorf("initial failures = %d", sh.LLMFailures())
|
||||
}
|
||||
|
||||
fs := sh.GetOrCreateFileSession("a.go")
|
||||
rec := fs.AppendTaskRecord(MainTask, nil)
|
||||
rec.SetError(errors.New("fail1"), time.Second)
|
||||
|
||||
rec2 := fs.AppendTaskRecord(MainTask, nil)
|
||||
rec2.SetError(errors.New("fail2"), time.Second)
|
||||
|
||||
if sh.LLMFailures() != 2 {
|
||||
t.Errorf("failures = %d, want 2", sh.LLMFailures())
|
||||
}
|
||||
}
|
||||
|
||||
func TestAddToolResult(t *testing.T) {
|
||||
sh := New("/tmp/repo", "main", "model", SessionOptions{})
|
||||
fs := sh.GetOrCreateFileSession("file.go")
|
||||
rec := fs.AppendTaskRecord(MainTask, nil)
|
||||
|
||||
rec.AddToolResult("file_read", `{"path":"main.go"}`, "package main")
|
||||
|
||||
if len(rec.ToolResults) != 1 {
|
||||
t.Fatalf("len = %d", len(rec.ToolResults))
|
||||
}
|
||||
tr := rec.ToolResults[0]
|
||||
if tr.ToolName != "file_read" {
|
||||
t.Errorf("ToolName = %q", tr.ToolName)
|
||||
}
|
||||
if tr.Arguments != `{"path":"main.go"}` {
|
||||
t.Errorf("Arguments = %q", tr.Arguments)
|
||||
}
|
||||
if tr.Result != "package main" {
|
||||
t.Errorf("Result = %q", tr.Result)
|
||||
}
|
||||
}
|
||||
190
internal/tool/code_comment_test.go
Normal file
190
internal/tool/code_comment_test.go
Normal file
|
|
@ -0,0 +1,190 @@
|
|||
package tool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestParseComments(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
args map[string]any
|
||||
wantCount int
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "valid comments array",
|
||||
args: map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": []any{
|
||||
map[string]any{"content": "issue 1", "existing_code": "old"},
|
||||
map[string]any{"content": "issue 2", "suggestion_code": "new"},
|
||||
},
|
||||
},
|
||||
wantCount: 2,
|
||||
},
|
||||
{
|
||||
name: "comments as JSON string",
|
||||
args: map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": `[{"content":"from string"}]`,
|
||||
},
|
||||
wantCount: 1,
|
||||
},
|
||||
{
|
||||
name: "missing path skips comment",
|
||||
args: map[string]any{
|
||||
"comments": []any{
|
||||
map[string]any{"content": "no path"},
|
||||
},
|
||||
},
|
||||
wantCount: 0,
|
||||
},
|
||||
{
|
||||
name: "missing content skips comment",
|
||||
args: map[string]any{
|
||||
"path": "file.go",
|
||||
"comments": []any{
|
||||
map[string]any{"existing_code": "has no content"},
|
||||
},
|
||||
},
|
||||
wantCount: 0,
|
||||
},
|
||||
{
|
||||
name: "empty comments array returns error",
|
||||
args: map[string]any{"path": "x.go", "comments": []any{}},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "no comments key returns error",
|
||||
args: map[string]any{"path": "x.go"},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "invalid JSON string returns error",
|
||||
args: map[string]any{"path": "x.go", "comments": "not json"},
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "thinking field preserved",
|
||||
args: map[string]any{
|
||||
"path": "a.go",
|
||||
"comments": []any{
|
||||
map[string]any{"content": "c", "thinking": "my reasoning"},
|
||||
},
|
||||
},
|
||||
wantCount: 1,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
comments, errMsg := ParseComments(tt.args)
|
||||
if tt.wantErr {
|
||||
if errMsg == "" {
|
||||
t.Error("expected error message, got empty")
|
||||
}
|
||||
return
|
||||
}
|
||||
if errMsg != "" {
|
||||
t.Fatalf("unexpected error: %s", errMsg)
|
||||
}
|
||||
if len(comments) != tt.wantCount {
|
||||
t.Errorf("len(comments) = %d, want %d", len(comments), tt.wantCount)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseComments_Fields(t *testing.T) {
|
||||
args := map[string]any{
|
||||
"path": "src/app.ts",
|
||||
"comments": []any{
|
||||
map[string]any{
|
||||
"content": "fix null check",
|
||||
"existing_code": "if (x == null)",
|
||||
"suggestion_code": "if (x === null)",
|
||||
"thinking": "strict equality is safer",
|
||||
},
|
||||
},
|
||||
}
|
||||
comments, errMsg := ParseComments(args)
|
||||
if errMsg != "" {
|
||||
t.Fatal(errMsg)
|
||||
}
|
||||
if len(comments) != 1 {
|
||||
t.Fatal("expected 1 comment")
|
||||
}
|
||||
c := comments[0]
|
||||
if c.Path != "src/app.ts" {
|
||||
t.Errorf("Path = %q", c.Path)
|
||||
}
|
||||
if c.Content != "fix null check" {
|
||||
t.Errorf("Content = %q", c.Content)
|
||||
}
|
||||
if c.ExistingCode != "if (x == null)" {
|
||||
t.Errorf("ExistingCode = %q", c.ExistingCode)
|
||||
}
|
||||
if c.SuggestionCode != "if (x === null)" {
|
||||
t.Errorf("SuggestionCode = %q", c.SuggestionCode)
|
||||
}
|
||||
if c.Thinking != "strict equality is safer" {
|
||||
t.Errorf("Thinking = %q", c.Thinking)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCodeCommentProvider_Execute(t *testing.T) {
|
||||
t.Run("adds comments to collector", func(t *testing.T) {
|
||||
collector := NewCommentCollector()
|
||||
p := &CodeCommentProvider{Collector: collector}
|
||||
result, err := p.Execute(context.Background(), map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": []any{
|
||||
map[string]any{"content": "issue 1"},
|
||||
map[string]any{"content": "issue 2"},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result != CommentSucceed {
|
||||
t.Errorf("result = %q, want %q", result, CommentSucceed)
|
||||
}
|
||||
if len(collector.Comments()) != 2 {
|
||||
t.Errorf("collector has %d comments, want 2", len(collector.Comments()))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("nil collector returns error message", func(t *testing.T) {
|
||||
p := &CodeCommentProvider{Collector: nil}
|
||||
result, err := p.Execute(context.Background(), map[string]any{
|
||||
"path": "main.go",
|
||||
"comments": []any{map[string]any{"content": "x"}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result == CommentSucceed {
|
||||
t.Error("expected error message for nil collector")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("invalid args returns error message", func(t *testing.T) {
|
||||
collector := NewCommentCollector()
|
||||
p := &CodeCommentProvider{Collector: collector}
|
||||
result, err := p.Execute(context.Background(), map[string]any{})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result == CommentSucceed {
|
||||
t.Error("expected error message for empty args")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("tool type is CodeComment", func(t *testing.T) {
|
||||
p := &CodeCommentProvider{}
|
||||
if p.Tool() != CodeComment {
|
||||
t.Errorf("Tool() = %v, want CodeComment", p.Tool())
|
||||
}
|
||||
})
|
||||
}
|
||||
130
internal/tool/filereader_read_test.go
Normal file
130
internal/tool/filereader_read_test.go
Normal file
|
|
@ -0,0 +1,130 @@
|
|||
package tool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestFileReader_Read_Workspace(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
content := "line1\nline2\nline3\n"
|
||||
if err := os.WriteFile(filepath.Join(dir, "test.go"), []byte(content), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
got, err := fr.Read(context.Background(), "test.go")
|
||||
if err != nil {
|
||||
t.Fatalf("Read() error: %v", err)
|
||||
}
|
||||
if got != content {
|
||||
t.Errorf("Read() = %q, want %q", got, content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileReader_Read_WorkspaceNotFound(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
_, err := fr.Read(context.Background(), "missing.go")
|
||||
if err == nil {
|
||||
t.Error("expected error for missing file")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileReader_Read_PathTraversal(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
|
||||
_, err := fr.Read(context.Background(), "../../../etc/passwd")
|
||||
if err == nil {
|
||||
t.Error("expected error for path traversal")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileReader_Read_SymlinkOutsideRepo(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
outside := t.TempDir()
|
||||
secretFile := filepath.Join(outside, "secret.txt")
|
||||
if err := os.WriteFile(secretFile, []byte("sensitive"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
link := filepath.Join(dir, "link.txt")
|
||||
if err := os.Symlink(secretFile, link); err != nil {
|
||||
t.Skipf("symlinks not supported: %v", err)
|
||||
}
|
||||
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
_, err := fr.Read(context.Background(), "link.txt")
|
||||
if err == nil {
|
||||
t.Error("expected error for symlink pointing outside repo")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileReader_ReadLines_Workspace(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
content := "aaa\nbbb\nccc\nddd\n"
|
||||
if err := os.WriteFile(filepath.Join(dir, "lines.txt"), []byte(content), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
|
||||
t.Run("all lines", func(t *testing.T) {
|
||||
lines, total, err := fr.ReadLines(context.Background(), "lines.txt", 1, 100)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if total != 5 {
|
||||
t.Errorf("total = %d, want 5", total)
|
||||
}
|
||||
if len(lines) != 5 {
|
||||
t.Errorf("lines count = %d, want 5", len(lines))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("start from line 2 with limit", func(t *testing.T) {
|
||||
lines, total, err := fr.ReadLines(context.Background(), "lines.txt", 2, 2)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if total != 5 {
|
||||
t.Errorf("total = %d, want 5", total)
|
||||
}
|
||||
if len(lines) != 2 {
|
||||
t.Fatalf("lines count = %d, want 2", len(lines))
|
||||
}
|
||||
if lines[0] != "bbb" || lines[1] != "ccc" {
|
||||
t.Errorf("lines = %v, want [bbb ccc]", lines)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("path traversal rejected", func(t *testing.T) {
|
||||
_, _, err := fr.ReadLines(context.Background(), "../../etc/passwd", 1, 10)
|
||||
if err == nil {
|
||||
t.Error("expected error for path traversal")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestFileReader_Read_SubdirectoryFile(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
sub := filepath.Join(dir, "src", "pkg")
|
||||
if err := os.MkdirAll(sub, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(sub, "main.go"), []byte("package main"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
fr := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
|
||||
got, err := fr.Read(context.Background(), "src/pkg/main.go")
|
||||
if err != nil {
|
||||
t.Fatalf("Read() error: %v", err)
|
||||
}
|
||||
if got != "package main" {
|
||||
t.Errorf("Read() = %q, want %q", got, "package main")
|
||||
}
|
||||
}
|
||||
241
internal/viewer/store_test.go
Normal file
241
internal/viewer/store_test.go
Normal file
|
|
@ -0,0 +1,241 @@
|
|||
package viewer
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func writeJSONL(t *testing.T, path string, lines ...string) {
|
||||
t.Helper()
|
||||
var content string
|
||||
for _, l := range lines {
|
||||
content += l + "\n"
|
||||
}
|
||||
if err := os.WriteFile(path, []byte(content), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverRepos_Empty(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
repos, err := DiscoverRepos(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(repos) != 0 {
|
||||
t.Errorf("expected 0 repos, got %d", len(repos))
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverRepos_NonExistentDir(t *testing.T) {
|
||||
repos, err := DiscoverRepos("/nonexistent/path/abc123")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if repos != nil {
|
||||
t.Errorf("expected nil for non-existent dir, got %v", repos)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverRepos_SkipsFiles(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "stray.txt"), []byte("x"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
repos, err := DiscoverRepos(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(repos) != 0 {
|
||||
t.Errorf("expected 0 repos, got %d", len(repos))
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverRepos_FindsRepos(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
|
||||
repoA := filepath.Join(root, "repo-a")
|
||||
repoB := filepath.Join(root, "repo-b")
|
||||
if err := os.MkdirAll(repoA, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(repoB, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
writeJSONL(t, filepath.Join(repoA, "session1.jsonl"),
|
||||
`{"type":"session_start","timestamp":"2025-01-01T10:00:00Z"}`)
|
||||
writeJSONL(t, filepath.Join(repoA, "session2.jsonl"),
|
||||
`{"type":"session_start","timestamp":"2025-01-02T10:00:00Z"}`)
|
||||
writeJSONL(t, filepath.Join(repoB, "session3.jsonl"),
|
||||
`{"type":"session_start","timestamp":"2025-01-03T10:00:00Z"}`)
|
||||
|
||||
repos, err := DiscoverRepos(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(repos) != 2 {
|
||||
t.Fatalf("expected 2 repos, got %d", len(repos))
|
||||
}
|
||||
if repos[0].EncodedPath != "repo-b" {
|
||||
t.Errorf("expected most recent repo first, got %q", repos[0].EncodedPath)
|
||||
}
|
||||
if repos[1].SessionCount != 2 {
|
||||
t.Errorf("repo-a session count = %d, want 2", repos[1].SessionCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiscoverRepos_SkipsDirsWithNoJSONL(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
emptyRepo := filepath.Join(root, "empty-repo")
|
||||
if err := os.MkdirAll(emptyRepo, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(emptyRepo, "readme.txt"), []byte("hi"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
repos, err := DiscoverRepos(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(repos) != 0 {
|
||||
t.Errorf("expected 0 repos for dir with no .jsonl, got %d", len(repos))
|
||||
}
|
||||
}
|
||||
|
||||
func TestListSessions(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
repoDir := filepath.Join(root, "myrepo")
|
||||
if err := os.MkdirAll(repoDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
writeJSONL(t, filepath.Join(repoDir, "aaa.jsonl"),
|
||||
`{"type":"session_start","timestamp":"2025-03-01T09:00:00Z","cwd":"/home/user/proj","gitBranch":"main","model":"gpt-4","reviewMode":"workspace"}`,
|
||||
`{"type":"session_end","duration_seconds":120.5,"files_reviewed":["a.go","b.go"],"llm_failures":1}`)
|
||||
|
||||
writeJSONL(t, filepath.Join(repoDir, "bbb.jsonl"),
|
||||
`{"type":"session_start","timestamp":"2025-03-02T10:00:00Z","cwd":"/home/user/proj","gitBranch":"feat","model":"claude","reviewMode":"commit","diffCommit":"abc123"}`,
|
||||
`{"type":"session_end","duration_seconds":60.0,"files_reviewed":["c.go"],"llm_failures":0}`)
|
||||
|
||||
// Non-jsonl file should be skipped
|
||||
if err := os.WriteFile(filepath.Join(repoDir, "notes.txt"), []byte("ignored"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
sessions, err := ListSessions(root, "myrepo")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(sessions) != 2 {
|
||||
t.Fatalf("expected 2 sessions, got %d", len(sessions))
|
||||
}
|
||||
|
||||
// Should be sorted newest first
|
||||
if sessions[0].SessionID != "bbb" {
|
||||
t.Errorf("expected newest session first, got %q", sessions[0].SessionID)
|
||||
}
|
||||
if sessions[0].Model != "claude" {
|
||||
t.Errorf("Model = %q", sessions[0].Model)
|
||||
}
|
||||
if sessions[0].ReviewMode != "commit" {
|
||||
t.Errorf("ReviewMode = %q", sessions[0].ReviewMode)
|
||||
}
|
||||
if sessions[0].DiffCommit != "abc123" {
|
||||
t.Errorf("DiffCommit = %q", sessions[0].DiffCommit)
|
||||
}
|
||||
if sessions[0].DurationSec != 60.0 {
|
||||
t.Errorf("DurationSec = %f", sessions[0].DurationSec)
|
||||
}
|
||||
if sessions[0].FileCount != 1 {
|
||||
t.Errorf("FileCount = %d", sessions[0].FileCount)
|
||||
}
|
||||
|
||||
if sessions[1].SessionID != "aaa" {
|
||||
t.Errorf("second session = %q", sessions[1].SessionID)
|
||||
}
|
||||
if sessions[1].LLMFailures != 1 {
|
||||
t.Errorf("LLMFailures = %d", sessions[1].LLMFailures)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPeekSession(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "test.jsonl")
|
||||
writeJSONL(t, path,
|
||||
`{"type":"session_start","timestamp":"2025-06-15T14:30:00Z","cwd":"/repo","gitBranch":"dev","model":"gpt-4o","reviewMode":"range","diffFrom":"a1b2","diffTo":"c3d4"}`,
|
||||
`{"type":"llm_request","filePath":"main.go","taskType":"main_task","request_no":1}`,
|
||||
`{"type":"llm_response","filePath":"main.go","taskType":"main_task","content":"looks good"}`,
|
||||
`{"type":"session_end","duration_seconds":45.2,"files_reviewed":["main.go","util.go"],"llm_failures":2}`)
|
||||
|
||||
s, err := peekSession(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
expected := time.Date(2025, 6, 15, 14, 30, 0, 0, time.UTC)
|
||||
if !s.Timestamp.Equal(expected) {
|
||||
t.Errorf("Timestamp = %v, want %v", s.Timestamp, expected)
|
||||
}
|
||||
if s.CWD != "/repo" {
|
||||
t.Errorf("CWD = %q", s.CWD)
|
||||
}
|
||||
if s.GitBranch != "dev" {
|
||||
t.Errorf("GitBranch = %q", s.GitBranch)
|
||||
}
|
||||
if s.Model != "gpt-4o" {
|
||||
t.Errorf("Model = %q", s.Model)
|
||||
}
|
||||
if s.ReviewMode != "range" {
|
||||
t.Errorf("ReviewMode = %q", s.ReviewMode)
|
||||
}
|
||||
if s.DiffFrom != "a1b2" {
|
||||
t.Errorf("DiffFrom = %q", s.DiffFrom)
|
||||
}
|
||||
if s.DiffTo != "c3d4" {
|
||||
t.Errorf("DiffTo = %q", s.DiffTo)
|
||||
}
|
||||
if s.DurationSec != 45.2 {
|
||||
t.Errorf("DurationSec = %f", s.DurationSec)
|
||||
}
|
||||
if len(s.FilesReviewed) != 2 {
|
||||
t.Errorf("FilesReviewed = %v", s.FilesReviewed)
|
||||
}
|
||||
if s.LLMFailures != 2 {
|
||||
t.Errorf("LLMFailures = %d", s.LLMFailures)
|
||||
}
|
||||
if s.FileCount != 2 {
|
||||
t.Errorf("FileCount = %d", s.FileCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPeekSession_MissingFile(t *testing.T) {
|
||||
_, err := peekSession("/nonexistent/path/session.jsonl")
|
||||
if err == nil {
|
||||
t.Error("expected error for missing file")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPeekSession_NoSessionEnd(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "partial.jsonl")
|
||||
writeJSONL(t, path,
|
||||
`{"type":"session_start","timestamp":"2025-01-01T00:00:00Z","cwd":"/x","model":"m"}`)
|
||||
|
||||
s, err := peekSession(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if s.CWD != "/x" {
|
||||
t.Errorf("CWD = %q", s.CWD)
|
||||
}
|
||||
if s.DurationSec != 0 {
|
||||
t.Errorf("DurationSec should be 0 without session_end, got %f", s.DurationSec)
|
||||
}
|
||||
if s.FileCount != 0 {
|
||||
t.Errorf("FileCount should be 0 without session_end, got %d", s.FileCount)
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue