From c61b03f215fe89684625734610f264ae0b9f88b2 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sun, 5 Apr 2026 18:43:33 -0400 Subject: [PATCH 1/2] =?UTF-8?q?Add=20tests=20for=20runners=20and=20cmd=20p?= =?UTF-8?q?ackages=20=E2=80=94=20143=20=E2=86=92=20179=20total=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runners/claude_test.go: - Claude JSON response parsing (full, error, empty, invalid, partial) - TruncStr edge cases (empty, exact, one-over, zero-limit, unicode) - RunOpts/RunResult zero-value defaults - Runner Name() for all 3 runners cmd/workflow_test.go: - Workflow name validation regex (12 cases incl path traversal, shell injection) - formatAge (4 duration ranges + 3 boundary cases) --- src/cmd/workflow_test.go | 81 +++++++++++++++++++++ src/runners/claude_test.go | 144 +++++++++++++++++++++++++++++++++++++ 2 files changed, 225 insertions(+) create mode 100644 src/cmd/workflow_test.go create mode 100644 src/runners/claude_test.go diff --git a/src/cmd/workflow_test.go b/src/cmd/workflow_test.go new file mode 100644 index 0000000..276ce42 --- /dev/null +++ b/src/cmd/workflow_test.go @@ -0,0 +1,81 @@ +package cmd + +import ( + "regexp" + "testing" + "time" +) + +func TestWorkflowNameValidation(t *testing.T) { + validPattern := regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) + + tests := []struct { + name string + input string + valid bool + }{ + {"simple", "feature", true}, + {"with-dash", "self-improve", true}, + {"with-underscore", "my_workflow", true}, + {"with-numbers", "v2-test", true}, + {"path-traversal", "../etc/passwd", false}, + {"absolute-path", "/etc/passwd", false}, + {"spaces", "my workflow", false}, + {"dots", "my.workflow", false}, + {"empty", "", false}, + {"shell-injection", "foo;rm -rf /", false}, + {"backtick", "foo`id`", false}, + {"dollar", "foo$HOME", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := validPattern.MatchString(tt.input) + if got != tt.valid { + t.Errorf("validate(%q) = %v, want %v", tt.input, got, tt.valid) + } + }) + } +} + +func TestFormatAge(t *testing.T) { + tests := []struct { + name string + age time.Duration + want string + }{ + {"just now", 30 * time.Second, "just now"}, + {"minutes", 5 * time.Minute, "5m ago"}, + {"hours", 3 * time.Hour, "3h ago"}, + {"days", 48 * time.Hour, "2d ago"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := formatAge(time.Now().Add(-tt.age)) + if got != tt.want { + t.Errorf("formatAge(-%v) = %q, want %q", tt.age, got, tt.want) + } + }) + } +} + +func TestFormatAge_Boundaries(t *testing.T) { + // Exactly 1 minute should show "1m ago", not "just now" + got := formatAge(time.Now().Add(-61 * time.Second)) + if got != "1m ago" { + t.Errorf("formatAge(-61s) = %q, want %q", got, "1m ago") + } + + // Exactly 1 hour should show "1h ago" + got = formatAge(time.Now().Add(-61 * time.Minute)) + if got != "1h ago" { + t.Errorf("formatAge(-61m) = %q, want %q", got, "1h ago") + } + + // Exactly 24 hours should show "1d ago" + got = formatAge(time.Now().Add(-25 * time.Hour)) + if got != "1d ago" { + t.Errorf("formatAge(-25h) = %q, want %q", got, "1d ago") + } +} diff --git a/src/runners/claude_test.go b/src/runners/claude_test.go new file mode 100644 index 0000000..f9fbc1c --- /dev/null +++ b/src/runners/claude_test.go @@ -0,0 +1,144 @@ +package runners + +import ( + "encoding/json" + "testing" +) + +func TestClaudeRunnerName(t *testing.T) { + r := &ClaudeRunner{} + if r.Name() != "claude" { + t.Errorf("got %q, want %q", r.Name(), "claude") + } +} + +func TestCodexRunnerName(t *testing.T) { + r := &CodexRunner{} + if r.Name() != "codex" { + t.Errorf("got %q, want %q", r.Name(), "codex") + } +} + +func TestGeminiRunnerName(t *testing.T) { + r := &GeminiRunner{} + if r.Name() != "gemini" { + t.Errorf("got %q, want %q", r.Name(), "gemini") + } +} + +func TestClaudeResponseParsing(t *testing.T) { + tests := []struct { + name string + json string + wantOut string + wantCost float64 + wantIn int + wantSess string + wantErr bool + }{ + { + name: "full response", + json: `{"result":"hello world","session_id":"sess-123","is_error":false,"total_cost_usd":0.05,"usage":{"input_tokens":100,"output_tokens":50}}`, + wantOut: "hello world", + wantCost: 0.05, + wantIn: 100, + wantSess: "sess-123", + }, + { + name: "error response", + json: `{"result":"something went wrong","is_error":true,"total_cost_usd":0.01,"usage":{"input_tokens":10,"output_tokens":5}}`, + wantOut: "something went wrong", + wantCost: 0.01, + wantIn: 10, + }, + { + name: "empty response", + json: `{"result":"","session_id":"","total_cost_usd":0,"usage":{"input_tokens":0,"output_tokens":0}}`, + wantOut: "", + wantCost: 0, + wantIn: 0, + }, + { + name: "invalid json", + json: `not json at all`, + wantErr: true, + }, + { + name: "missing fields", + json: `{"result":"partial"}`, + wantOut: "partial", + wantCost: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var resp claudeResponse + err := json.Unmarshal([]byte(tt.json), &resp) + if tt.wantErr { + if err == nil { + t.Fatal("expected parse error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.Result != tt.wantOut { + t.Errorf("result = %q, want %q", resp.Result, tt.wantOut) + } + if resp.TotalCostUSD != tt.wantCost { + t.Errorf("cost = %f, want %f", resp.TotalCostUSD, tt.wantCost) + } + if resp.Usage.InputTokens != tt.wantIn { + t.Errorf("input tokens = %d, want %d", resp.Usage.InputTokens, tt.wantIn) + } + if resp.SessionID != tt.wantSess { + t.Errorf("session = %q, want %q", resp.SessionID, tt.wantSess) + } + }) + } +} + +func TestTruncStr_EdgeCases(t *testing.T) { + tests := []struct { + name string + s string + n int + want string + }{ + {"empty", "", 10, ""}, + {"exact length", "abcde", 5, "abcde"}, + {"one over", "abcdef", 5, "abcde..."}, + {"zero limit", "abc", 0, "..."}, + {"unicode", "hello 世界！", 7, "hello 世..."}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := TruncStr(tt.s, tt.n) + if got != tt.want { + t.Errorf("TruncStr(%q, %d) = %q, want %q", tt.s, tt.n, got, tt.want) + } + }) + } +} + +func TestRunOptsDefaults(t *testing.T) { + opts := RunOpts{} + if opts.WorkDir != "" { + t.Error("default WorkDir should be empty") + } + if opts.MaxTurns != 0 { + t.Error("default MaxTurns should be 0") + } + if opts.AllowedTools != "" { + t.Error("default AllowedTools should be empty") + } +} + +func TestRunResultDefaults(t *testing.T) { + r := RunResult{} + if r.Output != "" || r.CostUSD != 0 || r.ExitCode != 0 { + t.Error("default RunResult should have zero values") + } +} From 8c9b8a4e6b9af7a0e06c46d3134ac34ed03578bc Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Sun, 5 Apr 2026 18:50:21 -0400 Subject: [PATCH 2/2] Fix review findings: extract regex, fix boundaries, remove noise tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract validWorkflowName regex to package-level var in workflow.go so tests validate production code, not a copy (critical review finding) - Fix formatAge boundary tests: add true boundaries (59s, 59m, 23h) and label 61s/61m/25h correctly as post-boundary - Add 5 more security vectors: null byte, pipe, newline, lone dot, double dot - Add TestResolveRunnerFrom_EmptyName for empty --agent flag path - Remove TestRunOptsDefaults and TestRunResultDefaults (tested Go zero-value guarantees, not application logic) 143 → 183 total tests. --- src/cmd/workflow.go | 4 +++- src/cmd/workflow_test.go | 48 +++++++++++++++++++++++++++++++------- src/runners/claude_test.go | 20 ---------------- 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/src/cmd/workflow.go b/src/cmd/workflow.go index 7fd1d9c..d391381 100644 --- a/src/cmd/workflow.go +++ b/src/cmd/workflow.go @@ -12,6 +12,8 @@ import ( "github.com/spf13/cobra" ) +var validWorkflowName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) + var workflowCmd = &cobra.Command{ Use: "workflow [name] [description...]", Short: "Run a YAML workflow by name", @@ -33,7 +35,7 @@ var workflowCmd = &cobra.Command{ } // Validate workflow name to prevent path traversal - if !regexp.MustCompile(`^[a-zA-Z0-9_-]+$`).MatchString(name) { + if !validWorkflowName.MatchString(name) { return fmt.Errorf("invalid workflow name %q — use only letters, numbers, hyphens, underscores", name) } diff --git a/src/cmd/workflow_test.go b/src/cmd/workflow_test.go index 276ce42..af94851 100644 --- a/src/cmd/workflow_test.go +++ b/src/cmd/workflow_test.go @@ -1,14 +1,13 @@ package cmd import ( - "regexp" "testing" "time" + + "github.com/5uck1ess/devkit/runners" ) func TestWorkflowNameValidation(t *testing.T) { - validPattern := regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) - tests := []struct { name string input string @@ -26,13 +25,18 @@ func TestWorkflowNameValidation(t *testing.T) { {"shell-injection", "foo;rm -rf /", false}, {"backtick", "foo`id`", false}, {"dollar", "foo$HOME", false}, + {"null-byte", "foo\x00bar", false}, + {"pipe", "foo|cat", false}, + {"newline", "foo\nbar", false}, + {"lone-dot", ".", false}, + {"double-dot", "..", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := validPattern.MatchString(tt.input) + got := validWorkflowName.MatchString(tt.input) if got != tt.valid { - t.Errorf("validate(%q) = %v, want %v", tt.input, got, tt.valid) + t.Errorf("validWorkflowName.MatchString(%q) = %v, want %v", tt.input, got, tt.valid) } }) } @@ -61,21 +65,47 @@ func TestFormatAge(t *testing.T) { } func TestFormatAge_Boundaries(t *testing.T) { - // Exactly 1 minute should show "1m ago", not "just now" - got := formatAge(time.Now().Add(-61 * time.Second)) + // At exactly 59s — still "just now" (d < time.Minute) + got := formatAge(time.Now().Add(-59 * time.Second)) + if got != "just now" { + t.Errorf("formatAge(-59s) = %q, want %q", got, "just now") + } + + // At 61s — crosses minute boundary, should be "1m ago" + got = formatAge(time.Now().Add(-61 * time.Second)) if got != "1m ago" { t.Errorf("formatAge(-61s) = %q, want %q", got, "1m ago") } - // Exactly 1 hour should show "1h ago" + // At 59m — still minutes, should be "59m ago" + got = formatAge(time.Now().Add(-59 * time.Minute)) + if got != "59m ago" { + t.Errorf("formatAge(-59m) = %q, want %q", got, "59m ago") + } + + // At 61m — crosses hour boundary, should be "1h ago" got = formatAge(time.Now().Add(-61 * time.Minute)) if got != "1h ago" { t.Errorf("formatAge(-61m) = %q, want %q", got, "1h ago") } - // Exactly 24 hours should show "1d ago" + // At 23h — still hours + got = formatAge(time.Now().Add(-23 * time.Hour)) + if got != "23h ago" { + t.Errorf("formatAge(-23h) = %q, want %q", got, "23h ago") + } + + // At 25h — crosses day boundary, should be "1d ago" got = formatAge(time.Now().Add(-25 * time.Hour)) if got != "1d ago" { t.Errorf("formatAge(-25h) = %q, want %q", got, "1d ago") } } + +func TestResolveRunnerFrom_EmptyName(t *testing.T) { + available := []runners.Runner{&stubRunner{"claude"}} + _, err := resolveRunnerFrom("", available) + if err == nil { + t.Fatal("expected error for empty agent name") + } +} diff --git a/src/runners/claude_test.go b/src/runners/claude_test.go index f9fbc1c..d6339f8 100644 --- a/src/runners/claude_test.go +++ b/src/runners/claude_test.go @@ -122,23 +122,3 @@ func TestTruncStr_EdgeCases(t *testing.T) { }) } } - -func TestRunOptsDefaults(t *testing.T) { - opts := RunOpts{} - if opts.WorkDir != "" { - t.Error("default WorkDir should be empty") - } - if opts.MaxTurns != 0 { - t.Error("default MaxTurns should be 0") - } - if opts.AllowedTools != "" { - t.Error("default AllowedTools should be empty") - } -} - -func TestRunResultDefaults(t *testing.T) { - r := RunResult{} - if r.Output != "" || r.CostUSD != 0 || r.ExitCode != 0 { - t.Error("default RunResult should have zero values") - } -}