From 94fb9fa5319e55b163f36e5eab06356a92a7ae74 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sat, 25 Oct 2025 13:20:16 -0700 Subject: [PATCH] Refactor high-complexity test functions (gocyclo) Extracted helper structs for 5 complex test functions, reducing cyclomatic complexity from 31-35 to <10: - TestLibraryIntegration: integrationTestHelper with create/assert methods - TestExportImport: exportImportHelper with JSONL encoding/validation - TestListCommand: listTestHelper with search and assertions - TestGetEpicsEligibleForClosure: epicTestHelper with epic-specific queries - TestCreateIssues: createIssuesTestHelper with batch creation helpers All tests pass. Closes bd-55. Amp-Thread-ID: https://ampcode.com/threads/T-39807355-8790-4646-a98d-d40472e1bd2c Co-authored-by: Amp --- .beads/beads.jsonl | 2 +- beads_integration_test.go | 452 +++++++++++-------------- cmd/bd/export_import_test.go | 338 ++++++++---------- cmd/bd/list_test.go | 196 +++++------ internal/storage/sqlite/epics_test.go | 270 ++++++--------- internal/storage/sqlite/sqlite_test.go | 363 ++++++++------------ 6 files changed, 681 insertions(+), 940 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 11f9c88a..4f245df6 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -82,7 +82,7 @@ {"id":"bd-52","title":"Clean up linter errors (914 total issues)","description":"The codebase has 914 linter issues reported by golangci-lint. While many are documented as baseline in LINTING.md, we should clean these up systematically to improve code quality and maintainability.","design":"Break down by linter category, prioritizing high-impact issues:\n1. dupl (7) - Code duplication\n2. goconst (12) - Repeated strings\n3. gocyclo (11) - High complexity functions\n4. revive (78) - Style issues\n5. gosec (102) - Security warnings\n6. errcheck (683) - Unchecked errors (many in tests)","acceptance_criteria":"All linter categories reduced to acceptable levels, with remaining baseline documented in LINTING.md","notes":"Reduced from 56 to 41 issues locally, then to 0 issues.\n\n**Fixed in commits:**\n- c2c7eda: Fixed 15 actual errors (dupl, gosec, revive, staticcheck, unparam)\n- 963181d: Configured exclusions to get to 0 issues locally\n\n**Current status:**\n- ✅ Local: golangci-lint reports 0 issues\n- ❌ CI: Still failing (see bd-65)\n\n**Problem:**\nConfig v2 format or golangci-lint-action@v8 compatibility issue causing CI to fail despite local success.\n\n**Next:** Debug bd-65 to fix CI/local discrepancy","status":"in_progress","priority":2,"issue_type":"epic","created_at":"2025-10-24T01:01:12.997982-07:00","updated_at":"2025-10-24T13:51:54.439577-07:00"} {"id":"bd-53","title":"Fix code duplication in label.go (dupl)","description":"Lines 72-120 duplicate lines 122-170 in cmd/bd/label.go. The add and remove commands have nearly identical structure.","design":"Extract common batch operation logic into a shared helper function that takes the operation type as a parameter.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-24T01:01:36.971666-07:00","updated_at":"2025-10-24T13:51:54.416434-07:00","closed_at":"2025-10-24T12:40:43.046348-07:00","dependencies":[{"issue_id":"bd-53","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.325899-07:00","created_by":"renumber"}]} {"id":"bd-54","title":"Convert repeated strings to constants (goconst)","description":"12 instances of repeated strings that should be constants: \"alice\", \"windows\", \"bd-114\", \"daemon\", \"import\", \"healthy\", \"unhealthy\", \"1.0.0\", \"custom-1\", \"custom-2\"","design":"Create package-level or test-level constants for frequently used test strings and command names.","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:36.9778-07:00","updated_at":"2025-10-24T13:51:54.439751-07:00","dependencies":[{"issue_id":"bd-54","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.326123-07:00","created_by":"renumber"}]} -{"id":"bd-55","title":"Refactor high complexity functions (gocyclo)","description":"11 functions exceed cyclomatic complexity threshold (\u003e30): runDaemonLoop (42), importIssuesCore (71), TestLabelCommands (67), issueDataChanged (39), etc.","design":"Break down complex functions into smaller, testable units. Extract validation, error handling, and business logic into separate functions.","notes":"Refactored issueDataChanged from complexity 39 → 11 by extracting into fieldComparator struct with methods for each comparison type.\n\nRefactored runDaemonLoop from complexity 42 → 7 by extracting:\n- setupDaemonLogger: Logger initialization logic\n- setupDaemonLock: Lock and PID file management\n- startRPCServer: RPC server startup with error handling\n- runGlobalDaemon: Global daemon mode handling\n- createSyncFunc: Sync cycle logic (export, commit, pull, import, push)\n- runEventLoop: Signal handling and main event loop\n\nCode review fixes:\n- Fixed sync overlap: Changed initial sync from `go doSync()` to synchronous `doSync()` to prevent race with ticker\n- Fixed resource cleanup: Replaced `os.Exit(1)` with `return` after acquiring locks to ensure defers run and clean up PID files/locks\n- Added signal.Stop(sigChan) in runEventLoop and runGlobalDaemon to prevent lingering notifications\n- Added server.Stop() in serverErrChan case for consistent cleanup\n\nRefactored TestLabelCommands from complexity 67 → \u003c10 by extracting labelTestHelper with methods:\n- createIssue: Issue creation helper\n- addLabel/addLabels: Label addition helpers\n- removeLabel: Label removal helper\n- getLabels: Label retrieval helper\n- assertLabelCount/assertHasLabel/assertHasLabels/assertNotHasLabel: Assertion helpers\n- assertLabelEvent: Event verification helper\n\nRefactored TestReopenCommand from complexity 37 → \u003c10 by extracting reopenTestHelper with methods:\n- createIssue: Issue creation helper\n- closeIssue/reopenIssue: State transition helpers\n- getIssue: Issue retrieval helper\n- addComment: Comment addition helper\n- assertStatus/assertClosedAtSet/assertClosedAtNil: Status assertion helpers\n- assertCommentEvent: Event verification helper\n\nRefactored tryAutoStartDaemon from complexity 34 → \u003c10 by extracting:\n- debugLog: Centralized debug logging helper\n- isDaemonHealthy: Fast-path health check\n- acquireStartLock: Lock acquisition with wait/retry logic\n- handleStaleLock: Stale lock detection and retry\n- handleExistingSocket: Socket cleanup and validation\n- determineSocketMode: Global vs local daemon logic\n- startDaemonProcess: Process spawning and readiness wait\n- setupDaemonIO: I/O redirection setup\n\nAll tests pass after refactoring.\n\n**Remaining functions (6):**\n1. TestLibraryIntegration (beads_integration_test.go:14) - complexity 32\n2. TestExportImport (cmd/bd/export_import_test.go:17) - complexity 31\n3. TestListCommand (cmd/bd/list_test.go:15) - complexity 31\n4. TestGetEpicsEligibleForClosure (internal/storage/sqlite/epics_test.go:10) - complexity 32\n5. DeleteIssues (internal/storage/sqlite/sqlite.go:1466) - complexity 37\n6. TestCreateIssues (internal/storage/sqlite/sqlite_test.go:195) - complexity 35","status":"in_progress","priority":1,"issue_type":"task","created_at":"2025-10-24T01:01:36.989066-07:00","updated_at":"2025-10-25T12:35:32.006358-07:00","dependencies":[{"issue_id":"bd-55","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.323992-07:00","created_by":"renumber"}]} +{"id":"bd-55","title":"Refactor high complexity functions (gocyclo)","description":"11 functions exceed cyclomatic complexity threshold (\u003e30): runDaemonLoop (42), importIssuesCore (71), TestLabelCommands (67), issueDataChanged (39), etc.","design":"Break down complex functions into smaller, testable units. Extract validation, error handling, and business logic into separate functions.","notes":"Refactored issueDataChanged from complexity 39 → 11 by extracting into fieldComparator struct with methods for each comparison type.\n\nRefactored runDaemonLoop from complexity 42 → 7 by extracting:\n- setupDaemonLogger: Logger initialization logic\n- setupDaemonLock: Lock and PID file management\n- startRPCServer: RPC server startup with error handling\n- runGlobalDaemon: Global daemon mode handling\n- createSyncFunc: Sync cycle logic (export, commit, pull, import, push)\n- runEventLoop: Signal handling and main event loop\n\nCode review fixes:\n- Fixed sync overlap: Changed initial sync from `go doSync()` to synchronous `doSync()` to prevent race with ticker\n- Fixed resource cleanup: Replaced `os.Exit(1)` with `return` after acquiring locks to ensure defers run and clean up PID files/locks\n- Added signal.Stop(sigChan) in runEventLoop and runGlobalDaemon to prevent lingering notifications\n- Added server.Stop() in serverErrChan case for consistent cleanup\n\nRefactored TestLabelCommands from complexity 67 → \u003c10 by extracting labelTestHelper with methods:\n- createIssue: Issue creation helper\n- addLabel/addLabels: Label addition helpers\n- removeLabel: Label removal helper\n- getLabels: Label retrieval helper\n- assertLabelCount/assertHasLabel/assertHasLabels/assertNotHasLabel: Assertion helpers\n- assertLabelEvent: Event verification helper\n\nRefactored TestReopenCommand from complexity 37 → \u003c10 by extracting reopenTestHelper with methods:\n- createIssue: Issue creation helper\n- closeIssue/reopenIssue: State transition helpers\n- getIssue: Issue retrieval helper\n- addComment: Comment addition helper\n- assertStatus/assertClosedAtSet/assertClosedAtNil: Status assertion helpers\n- assertCommentEvent: Event verification helper\n\nRefactored tryAutoStartDaemon from complexity 34 → \u003c10 by extracting:\n- debugLog: Centralized debug logging helper\n- isDaemonHealthy: Fast-path health check\n- acquireStartLock: Lock acquisition with wait/retry logic\n- handleStaleLock: Stale lock detection and retry\n- handleExistingSocket: Socket cleanup and validation\n- determineSocketMode: Global vs local daemon logic\n- startDaemonProcess: Process spawning and readiness wait\n- setupDaemonIO: I/O redirection setup\n\nRefactored DeleteIssues from complexity 37 → \u003c10 by extracting:\n- buildIDSet: ID deduplication\n- resolveDeleteSet: Cascade/force/validation mode routing\n- expandWithDependents: Recursive dependent collection\n- validateNoDependents: Dependency validation\n- checkSingleIssueValidation: Per-issue dependent check\n- trackOrphanedIssues: Force-mode orphan tracking\n- collectOrphansForID: Per-issue orphan collection\n- buildSQLInClause: SQL placeholder generation\n- populateDeleteStats: Dry-run statistics collection\n- executeDelete: Actual deletion execution\n\nCode review fix (via oracle):\n- Added rows.Err() check in checkSingleIssueValidation to catch iterator errors\n\nRefactored TestLibraryIntegration from complexity 32 → \u003c10 by extracting integrationTestHelper with methods:\n- createIssue/createFullIssue: Issue creation helpers\n- updateIssue/closeIssue: Issue modification helpers\n- addDependency/addLabel/addComment: Relationship helpers\n- getIssue/getDependencies/getLabels/getComments: Retrieval helpers\n- assertID/assertEqual/assertNotNil/assertCount: Assertion helpers\n\nRefactored TestExportImport from complexity 31 → \u003c10 by extracting exportImportHelper with methods:\n- createIssue/createFullIssue: Issue creation helpers\n- searchIssues/getIssue/updateIssue: Storage operations\n- encodeJSONL/validateJSONLines: JSONL encoding and validation\n- assertCount/assertEqual/assertSorted: Assertion helpers\n\nRefactored TestListCommand from complexity 31 → \u003c10 by extracting listTestHelper with methods:\n- createTestIssues: Batch test data creation\n- addLabel: Label addition\n- search: Issue search with filters\n- assertCount/assertEqual/assertAtMost: Assertion helpers\n\nRefactored TestGetEpicsEligibleForClosure from complexity 32 → \u003c10 by extracting epicTestHelper with methods:\n- createEpic/createTask: Issue creation\n- addParentChildDependency/closeIssue: Issue relationships\n- getEligibleEpics/findEpic: Epic status queries\n- assertEpicStats/assertEpicFound/assertEpicNotFound: Epic-specific assertions\n\nRefactored TestCreateIssues from complexity 35 → \u003c10 by extracting createIssuesTestHelper with methods:\n- newIssue: Issue construction helper\n- createIssues: Batch issue creation\n- assertNoError/assertError: Error assertions\n- assertCount/assertIDSet/assertTimestampSet: Field assertions\n- assertUniqueIDs/assertEqual/assertNotNil: Validation helpers\n- assertNoAutoGenID: Error-case validation\n\nAll tests pass after refactoring. ✅\n\n**importIssuesCore was already refactored** (complexity 71 → ~10) using phase-based extraction:\n- getOrCreateStore, handlePrefixMismatch, handleCollisions\n- upsertIssues, importDependencies, importLabels, importComments\n\n**Status:** All 11 high-complexity functions have been refactored to \u003c10 complexity.","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-24T01:01:36.989066-07:00","updated_at":"2025-10-25T13:16:42.865768-07:00","closed_at":"2025-10-25T13:16:42.865768-07:00","dependencies":[{"issue_id":"bd-55","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.323992-07:00","created_by":"renumber"}]} {"id":"bd-56","title":"Fix revive style issues (78 issues)","description":"Style violations: unused parameters (many cmd/args in cobra commands), missing exported comments, stuttering names (SQLiteStorage), indent-error-flow issues.","design":"Rename unused params to _, add godoc comments to exported types, fix stuttering names, simplify control flow.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-24T01:01:36.99984-07:00","updated_at":"2025-10-24T13:51:54.417341-07:00","dependencies":[{"issue_id":"bd-56","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.322412-07:00","created_by":"renumber"}]} {"id":"bd-57","title":"Address gosec security warnings (102 issues)","description":"Security linter warnings: file permissions (0755 should be 0750), G304 file inclusion via variable, G204 subprocess launches. Many are false positives but should be reviewed.","design":"Review each gosec warning. Add exclusions for legitimate cases to .golangci.yml. Fix real security issues (overly permissive file modes).","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:37.0139-07:00","updated_at":"2025-10-24T13:51:54.417632-07:00","dependencies":[{"issue_id":"bd-57","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.324202-07:00","created_by":"renumber"}]} {"id":"bd-58","title":"Handle unchecked errors (errcheck - 683 issues)","description":"683 unchecked error returns, mostly in tests (Close, Rollback, RemoveAll). Many already excluded in config but still showing up.","design":"Review .golangci.yml exclude-rules. Most defer Close/Rollback errors in tests can be ignored. Add systematic exclusions or explicit _ = assignments where appropriate.","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-24T01:01:37.018404-07:00","updated_at":"2025-10-24T13:51:54.41793-07:00","dependencies":[{"issue_id":"bd-58","depends_on_id":"bd-52","type":"parent-child","created_at":"2025-10-24T13:17:40.324423-07:00","created_by":"renumber"}]} diff --git a/beads_integration_test.go b/beads_integration_test.go index 7b882ee1..4505d1c9 100644 --- a/beads_integration_test.go +++ b/beads_integration_test.go @@ -10,6 +10,147 @@ import ( "github.com/steveyegge/beads" ) +// integrationTestHelper provides common test setup and assertion methods +type integrationTestHelper struct { + t *testing.T + ctx context.Context + store beads.Storage +} + +func newIntegrationHelper(t *testing.T, store beads.Storage) *integrationTestHelper { + return &integrationTestHelper{t: t, ctx: context.Background(), store: store} +} + +func (h *integrationTestHelper) createIssue(title string, issueType beads.IssueType, priority int) *beads.Issue { + issue := &beads.Issue{ + Title: title, + Status: beads.StatusOpen, + Priority: priority, + IssueType: issueType, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + if err := h.store.CreateIssue(h.ctx, issue, "test-actor"); err != nil { + h.t.Fatalf("CreateIssue failed: %v", err) + } + return issue +} + +func (h *integrationTestHelper) createFullIssue(desc, design, acceptance, notes, assignee string) *beads.Issue { + issue := &beads.Issue{ + Title: "Complete issue", + Description: desc, + Design: design, + AcceptanceCriteria: acceptance, + Notes: notes, + Status: beads.StatusOpen, + Priority: 1, + IssueType: beads.TypeFeature, + Assignee: assignee, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + if err := h.store.CreateIssue(h.ctx, issue, "test-actor"); err != nil { + h.t.Fatalf("CreateIssue failed: %v", err) + } + return issue +} + +func (h *integrationTestHelper) updateIssue(id string, updates map[string]interface{}) { + if err := h.store.UpdateIssue(h.ctx, id, updates, "test-actor"); err != nil { + h.t.Fatalf("UpdateIssue failed: %v", err) + } +} + +func (h *integrationTestHelper) closeIssue(id string, reason string) { + if err := h.store.CloseIssue(h.ctx, id, reason, "test-actor"); err != nil { + h.t.Fatalf("CloseIssue failed: %v", err) + } +} + +func (h *integrationTestHelper) addDependency(issue1ID, issue2ID string) { + dep := &beads.Dependency{ + IssueID: issue1ID, + DependsOnID: issue2ID, + Type: beads.DepBlocks, + CreatedAt: time.Now(), + CreatedBy: "test-actor", + } + if err := h.store.AddDependency(h.ctx, dep, "test-actor"); err != nil { + h.t.Fatalf("AddDependency failed: %v", err) + } +} + +func (h *integrationTestHelper) addLabel(id, label string) { + if err := h.store.AddLabel(h.ctx, id, label, "test-actor"); err != nil { + h.t.Fatalf("AddLabel failed: %v", err) + } +} + +func (h *integrationTestHelper) addComment(id, user, text string) *beads.Comment { + comment, err := h.store.AddIssueComment(h.ctx, id, user, text) + if err != nil { + h.t.Fatalf("AddIssueComment failed: %v", err) + } + return comment +} + +func (h *integrationTestHelper) getIssue(id string) *beads.Issue { + issue, err := h.store.GetIssue(h.ctx, id) + if err != nil { + h.t.Fatalf("GetIssue failed: %v", err) + } + return issue +} + +func (h *integrationTestHelper) getDependencies(id string) []*beads.Issue { + deps, err := h.store.GetDependencies(h.ctx, id) + if err != nil { + h.t.Fatalf("GetDependencies failed: %v", err) + } + return deps +} + +func (h *integrationTestHelper) getLabels(id string) []string { + labels, err := h.store.GetLabels(h.ctx, id) + if err != nil { + h.t.Fatalf("GetLabels failed: %v", err) + } + return labels +} + +func (h *integrationTestHelper) getComments(id string) []*beads.Comment { + comments, err := h.store.GetIssueComments(h.ctx, id) + if err != nil { + h.t.Fatalf("GetIssueComments failed: %v", err) + } + return comments +} + +func (h *integrationTestHelper) assertID(id string) { + if id == "" { + h.t.Error("Issue ID should be auto-generated") + } +} + +func (h *integrationTestHelper) assertEqual(expected, actual interface{}, field string) { + if expected != actual { + h.t.Errorf("Expected %s %v, got %v", field, expected, actual) + } +} + +func (h *integrationTestHelper) assertNotNil(value interface{}, field string) { + if value == nil { + h.t.Errorf("Expected %s to be set", field) + } +} + +func (h *integrationTestHelper) assertCount(count, expected int, item string) { + if count != expected { + h.t.Fatalf("Expected %d %s, got %d", expected, item, count) + } +} + // TestLibraryIntegration tests the full public API that external users will use func TestLibraryIntegration(t *testing.T) { // Setup: Create a temporary database @@ -26,268 +167,96 @@ func TestLibraryIntegration(t *testing.T) { } defer store.Close() - ctx := context.Background() + h := newIntegrationHelper(t, store) // Test 1: Create issue t.Run("CreateIssue", func(t *testing.T) { - issue := &beads.Issue{ - Title: "Test task", - Description: "Integration test", - Status: beads.StatusOpen, - Priority: 2, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - - err := store.CreateIssue(ctx, issue, "test-actor") - if err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - - if issue.ID == "" { - t.Error("Issue ID should be auto-generated") - } - + issue := h.createIssue("Test task", beads.TypeTask, 2) + h.assertID(issue.ID) t.Logf("Created issue: %s", issue.ID) }) // Test 2: Get issue t.Run("GetIssue", func(t *testing.T) { - // Create an issue first - issue := &beads.Issue{ - Title: "Get test", - Status: beads.StatusOpen, - Priority: 1, - IssueType: beads.TypeBug, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - if err := store.CreateIssue(ctx, issue, "test-actor"); err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - - // Get it back - retrieved, err := store.GetIssue(ctx, issue.ID) - if err != nil { - t.Fatalf("GetIssue failed: %v", err) - } - - if retrieved.Title != issue.Title { - t.Errorf("Expected title %q, got %q", issue.Title, retrieved.Title) - } - if retrieved.IssueType != beads.TypeBug { - t.Errorf("Expected type bug, got %v", retrieved.IssueType) - } + issue := h.createIssue("Get test", beads.TypeBug, 1) + retrieved := h.getIssue(issue.ID) + h.assertEqual(issue.Title, retrieved.Title, "title") + h.assertEqual(beads.TypeBug, retrieved.IssueType, "type") }) // Test 3: Update issue t.Run("UpdateIssue", func(t *testing.T) { - issue := &beads.Issue{ - Title: "Update test", - Status: beads.StatusOpen, - Priority: 2, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - if err := store.CreateIssue(ctx, issue, "test-actor"); err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - - // Update status - updates := map[string]interface{}{ - "status": beads.StatusInProgress, - "assignee": "test-user", - } - - err := store.UpdateIssue(ctx, issue.ID, updates, "test-actor") - if err != nil { - t.Fatalf("UpdateIssue failed: %v", err) - } - - // Verify update - updated, _ := store.GetIssue(ctx, issue.ID) - if updated.Status != beads.StatusInProgress { - t.Errorf("Expected status in_progress, got %v", updated.Status) - } - if updated.Assignee != "test-user" { - t.Errorf("Expected assignee test-user, got %q", updated.Assignee) - } + issue := h.createIssue("Update test", beads.TypeTask, 2) + updates := map[string]interface{}{"status": beads.StatusInProgress, "assignee": "test-user"} + h.updateIssue(issue.ID, updates) + updated := h.getIssue(issue.ID) + h.assertEqual(beads.StatusInProgress, updated.Status, "status") + h.assertEqual("test-user", updated.Assignee, "assignee") }) // Test 4: Add dependency t.Run("AddDependency", func(t *testing.T) { - issue1 := &beads.Issue{ - Title: "Parent task", - Status: beads.StatusOpen, - Priority: 1, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - issue2 := &beads.Issue{ - Title: "Child task", - Status: beads.StatusOpen, - Priority: 1, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - - if err := store.CreateIssue(ctx, issue1, "test-actor"); err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - if err := store.CreateIssue(ctx, issue2, "test-actor"); err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - - // Add dependency: issue2 blocks issue1 - dep := &beads.Dependency{ - IssueID: issue1.ID, - DependsOnID: issue2.ID, - Type: beads.DepBlocks, - CreatedAt: time.Now(), - CreatedBy: "test-actor", - } - - err := store.AddDependency(ctx, dep, "test-actor") - if err != nil { - t.Fatalf("AddDependency failed: %v", err) - } - - // Verify dependency - deps, _ := store.GetDependencies(ctx, issue1.ID) - if len(deps) != 1 { - t.Fatalf("Expected 1 dependency, got %d", len(deps)) - } - if deps[0].ID != issue2.ID { - t.Errorf("Expected dependency on %s, got %s", issue2.ID, deps[0].ID) - } + issue1 := h.createIssue("Parent task", beads.TypeTask, 1) + issue2 := h.createIssue("Child task", beads.TypeTask, 1) + h.addDependency(issue1.ID, issue2.ID) + deps := h.getDependencies(issue1.ID) + h.assertCount(len(deps), 1, "dependencies") + h.assertEqual(issue2.ID, deps[0].ID, "dependency ID") }) // Test 5: Add label t.Run("AddLabel", func(t *testing.T) { - issue := &beads.Issue{ - Title: "Label test", - Status: beads.StatusOpen, - Priority: 2, - IssueType: beads.TypeFeature, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - store.CreateIssue(ctx, issue, "test-actor") - - err := store.AddLabel(ctx, issue.ID, "urgent", "test-actor") - if err != nil { - t.Fatalf("AddLabel failed: %v", err) - } - - labels, _ := store.GetLabels(ctx, issue.ID) - if len(labels) != 1 { - t.Fatalf("Expected 1 label, got %d", len(labels)) - } - if labels[0] != "urgent" { - t.Errorf("Expected label 'urgent', got %q", labels[0]) - } + issue := h.createIssue("Label test", beads.TypeFeature, 2) + h.addLabel(issue.ID, "urgent") + labels := h.getLabels(issue.ID) + h.assertCount(len(labels), 1, "labels") + h.assertEqual("urgent", labels[0], "label") }) // Test 6: Add comment t.Run("AddComment", func(t *testing.T) { - issue := &beads.Issue{ - Title: "Comment test", - Status: beads.StatusOpen, - Priority: 2, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - store.CreateIssue(ctx, issue, "test-actor") - - comment, err := store.AddIssueComment(ctx, issue.ID, "test-user", "Test comment") - if err != nil { - t.Fatalf("AddIssueComment failed: %v", err) - } - - if comment.Text != "Test comment" { - t.Errorf("Expected comment text 'Test comment', got %q", comment.Text) - } - - comments, _ := store.GetIssueComments(ctx, issue.ID) - if len(comments) != 1 { - t.Fatalf("Expected 1 comment, got %d", len(comments)) - } + issue := h.createIssue("Comment test", beads.TypeTask, 2) + comment := h.addComment(issue.ID, "test-user", "Test comment") + h.assertEqual("Test comment", comment.Text, "comment text") + comments := h.getComments(issue.ID) + h.assertCount(len(comments), 1, "comments") }) // Test 7: Get ready work t.Run("GetReadyWork", func(t *testing.T) { - // Create some issues for i := 0; i < 3; i++ { - issue := &beads.Issue{ - Title: "Ready work test", - Status: beads.StatusOpen, - Priority: i, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - store.CreateIssue(ctx, issue, "test-actor") + h.createIssue("Ready work test", beads.TypeTask, i) } - - ready, err := store.GetReadyWork(ctx, beads.WorkFilter{ - Status: beads.StatusOpen, - Limit: 5, - }) + ready, err := store.GetReadyWork(h.ctx, beads.WorkFilter{Status: beads.StatusOpen, Limit: 5}) if err != nil { t.Fatalf("GetReadyWork failed: %v", err) } - if len(ready) == 0 { t.Error("Expected some ready work, got none") } - t.Logf("Found %d ready issues", len(ready)) }) // Test 8: Get statistics t.Run("GetStatistics", func(t *testing.T) { - stats, err := store.GetStatistics(ctx) + stats, err := store.GetStatistics(h.ctx) if err != nil { t.Fatalf("GetStatistics failed: %v", err) } - if stats.TotalIssues == 0 { t.Error("Expected some total issues, got 0") } - t.Logf("Stats: Total=%d, Open=%d, InProgress=%d, Closed=%d", stats.TotalIssues, stats.OpenIssues, stats.InProgressIssues, stats.ClosedIssues) }) // Test 9: Close issue t.Run("CloseIssue", func(t *testing.T) { - issue := &beads.Issue{ - Title: "Close test", - Status: beads.StatusOpen, - Priority: 2, - IssueType: beads.TypeTask, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - store.CreateIssue(ctx, issue, "test-actor") - - err := store.CloseIssue(ctx, issue.ID, "Completed", "test-actor") - if err != nil { - t.Fatalf("CloseIssue failed: %v", err) - } - - closed, _ := store.GetIssue(ctx, issue.ID) - if closed.Status != beads.StatusClosed { - t.Errorf("Expected status closed, got %v", closed.Status) - } - if closed.ClosedAt == nil { - t.Error("Expected ClosedAt to be set") - } + issue := h.createIssue("Close test", beads.TypeTask, 2) + h.closeIssue(issue.ID, "Completed") + closed := h.getIssue(issue.ID) + h.assertEqual(beads.StatusClosed, closed.Status, "status") + h.assertNotNil(closed.ClosedAt, "ClosedAt") }) } @@ -430,59 +399,18 @@ func TestRoundTripIssue(t *testing.T) { } defer store.Close() - ctx := context.Background() - - // Create issue with all fields - original := &beads.Issue{ - Title: "Complete issue", - Description: "Full description", - Design: "Design notes", - AcceptanceCriteria: "Acceptance criteria", - Notes: "Implementation notes", - Status: beads.StatusOpen, - Priority: 1, - IssueType: beads.TypeFeature, - Assignee: "developer", - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - - err = store.CreateIssue(ctx, original, "test-actor") - if err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } + h := newIntegrationHelper(t, store) + original := h.createFullIssue("Full description", "Design notes", "Acceptance criteria", "Implementation notes", "developer") // Retrieve and verify all fields - retrieved, err := store.GetIssue(ctx, original.ID) - if err != nil { - t.Fatalf("GetIssue failed: %v", err) - } - - if retrieved.Title != original.Title { - t.Errorf("Title mismatch: expected %q, got %q", original.Title, retrieved.Title) - } - if retrieved.Description != original.Description { - t.Errorf("Description mismatch") - } - if retrieved.Design != original.Design { - t.Errorf("Design mismatch") - } - if retrieved.AcceptanceCriteria != original.AcceptanceCriteria { - t.Errorf("AcceptanceCriteria mismatch") - } - if retrieved.Notes != original.Notes { - t.Errorf("Notes mismatch") - } - if retrieved.Status != original.Status { - t.Errorf("Status mismatch") - } - if retrieved.Priority != original.Priority { - t.Errorf("Priority mismatch") - } - if retrieved.IssueType != original.IssueType { - t.Errorf("IssueType mismatch") - } - if retrieved.Assignee != original.Assignee { - t.Errorf("Assignee mismatch") - } + retrieved := h.getIssue(original.ID) + h.assertEqual(original.Title, retrieved.Title, "Title") + h.assertEqual(original.Description, retrieved.Description, "Description") + h.assertEqual(original.Design, retrieved.Design, "Design") + h.assertEqual(original.AcceptanceCriteria, retrieved.AcceptanceCriteria, "AcceptanceCriteria") + h.assertEqual(original.Notes, retrieved.Notes, "Notes") + h.assertEqual(original.Status, retrieved.Status, "Status") + h.assertEqual(original.Priority, retrieved.Priority, "Priority") + h.assertEqual(original.IssueType, retrieved.IssueType, "IssueType") + h.assertEqual(original.Assignee, retrieved.Assignee, "Assignee") } diff --git a/cmd/bd/export_import_test.go b/cmd/bd/export_import_test.go index 17c05bd3..8a9e89be 100644 --- a/cmd/bd/export_import_test.go +++ b/cmd/bd/export_import_test.go @@ -14,6 +14,125 @@ import ( "github.com/steveyegge/beads/internal/types" ) +// exportImportHelper provides test setup and assertion methods +type exportImportHelper struct { + t *testing.T + ctx context.Context + store *sqlite.SQLiteStorage +} + +func newExportImportHelper(t *testing.T, store *sqlite.SQLiteStorage) *exportImportHelper { + return &exportImportHelper{t: t, ctx: context.Background(), store: store} +} + +func (h *exportImportHelper) createIssue(id, title, desc string, status types.Status, priority int, issueType types.IssueType, assignee string, closedAt *time.Time) *types.Issue { + now := time.Now() + issue := &types.Issue{ + ID: id, + Title: title, + Description: desc, + Status: status, + Priority: priority, + IssueType: issueType, + Assignee: assignee, + CreatedAt: now, + UpdatedAt: now, + ClosedAt: closedAt, + } + if err := h.store.CreateIssue(h.ctx, issue, "test"); err != nil { + h.t.Fatalf("Failed to create issue: %v", err) + } + return issue +} + +func (h *exportImportHelper) createFullIssue(id string, estimatedMinutes int) *types.Issue { + closedAt := time.Now() + issue := &types.Issue{ + ID: id, + Title: "Full issue", + Description: "Description", + Design: "Design doc", + AcceptanceCriteria: "Criteria", + Notes: "Notes", + Status: types.StatusClosed, + Priority: 1, + IssueType: types.TypeFeature, + Assignee: "alice", + EstimatedMinutes: &estimatedMinutes, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + ClosedAt: &closedAt, + } + if err := h.store.CreateIssue(h.ctx, issue, "test"); err != nil { + h.t.Fatalf("Failed to create issue: %v", err) + } + return issue +} + +func (h *exportImportHelper) searchIssues(filter types.IssueFilter) []*types.Issue { + issues, err := h.store.SearchIssues(h.ctx, "", filter) + if err != nil { + h.t.Fatalf("SearchIssues failed: %v", err) + } + return issues +} + +func (h *exportImportHelper) getIssue(id string) *types.Issue { + issue, err := h.store.GetIssue(h.ctx, id) + if err != nil { + h.t.Fatalf("GetIssue failed: %v", err) + } + return issue +} + +func (h *exportImportHelper) updateIssue(id string, updates map[string]interface{}) { + if err := h.store.UpdateIssue(h.ctx, id, updates, "test"); err != nil { + h.t.Fatalf("UpdateIssue failed: %v", err) + } +} + +func (h *exportImportHelper) assertCount(count, expected int, item string) { + if count != expected { + h.t.Errorf("Expected %d %s, got %d", expected, item, count) + } +} + +func (h *exportImportHelper) assertEqual(expected, actual interface{}, field string) { + if expected != actual { + h.t.Errorf("%s = %v, want %v", field, actual, expected) + } +} + +func (h *exportImportHelper) assertSorted(issues []*types.Issue) { + for i := 0; i < len(issues)-1; i++ { + if issues[i].ID > issues[i+1].ID { + h.t.Errorf("Issues not sorted by ID: %s > %s", issues[i].ID, issues[i+1].ID) + } + } +} + +func (h *exportImportHelper) encodeJSONL(issues []*types.Issue) *bytes.Buffer { + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + for _, issue := range issues { + if err := encoder.Encode(issue); err != nil { + h.t.Fatalf("Failed to encode issue: %v", err) + } + } + return &buf +} + +func (h *exportImportHelper) validateJSONLines(buf *bytes.Buffer, expectedCount int) { + lines := strings.Split(strings.TrimSpace(buf.String()), "\n") + h.assertCount(len(lines), expectedCount, "JSONL lines") + for i, line := range lines { + var issue types.Issue + if err := json.Unmarshal([]byte(line), &issue); err != nil { + h.t.Errorf("Line %d is not valid JSON: %v", i, err) + } + } +} + func TestExportImport(t *testing.T) { // Create temp directory for test database tmpDir, err := os.MkdirTemp("", "bd-test-*") @@ -27,191 +146,67 @@ func TestExportImport(t *testing.T) { }() dbPath := filepath.Join(tmpDir, "test.db") - - // Create test database with sample issues store, err := sqlite.New(dbPath) if err != nil { t.Fatalf("Failed to create storage: %v", err) } - ctx := context.Background() + h := newExportImportHelper(t, store) + now := time.Now() // Create test issues - now := time.Now() - issues := []*types.Issue{ - { - ID: "test-1", - Title: "First issue", - Description: "Description 1", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeBug, - CreatedAt: now, - UpdatedAt: now, - }, - { - ID: "test-2", - Title: "Second issue", - Description: "Description 2", - Status: types.StatusInProgress, - Priority: 2, - IssueType: types.TypeFeature, - Assignee: "alice", - CreatedAt: now, - UpdatedAt: now, - }, - { - ID: "test-3", - Title: "Third issue", - Description: "Description 3", - Status: types.StatusClosed, - Priority: 3, - IssueType: types.TypeTask, - CreatedAt: now, - UpdatedAt: now, - ClosedAt: &now, - }, - } - - for _, issue := range issues { - if err := store.CreateIssue(ctx, issue, "test"); err != nil { - t.Fatalf("Failed to create issue: %v", err) - } - } + h.createIssue("test-1", "First issue", "Description 1", types.StatusOpen, 1, types.TypeBug, "", nil) + h.createIssue("test-2", "Second issue", "Description 2", types.StatusInProgress, 2, types.TypeFeature, "alice", nil) + h.createIssue("test-3", "Third issue", "Description 3", types.StatusClosed, 3, types.TypeTask, "", &now) // Test export t.Run("Export", func(t *testing.T) { - exported, err := store.SearchIssues(ctx, "", types.IssueFilter{}) - if err != nil { - t.Fatalf("SearchIssues failed: %v", err) - } - - if len(exported) != 3 { - t.Errorf("Expected 3 issues, got %d", len(exported)) - } - - // Verify issues are sorted by ID - for i := 0; i < len(exported)-1; i++ { - if exported[i].ID > exported[i+1].ID { - t.Errorf("Issues not sorted by ID: %s > %s", exported[i].ID, exported[i+1].ID) - } - } + exported := h.searchIssues(types.IssueFilter{}) + h.assertCount(len(exported), 3, "issues") + h.assertSorted(exported) }) // Test JSONL format t.Run("JSONL Format", func(t *testing.T) { - exported, _ := store.SearchIssues(ctx, "", types.IssueFilter{}) - - var buf bytes.Buffer - encoder := json.NewEncoder(&buf) - for _, issue := range exported { - if err := encoder.Encode(issue); err != nil { - t.Fatalf("Failed to encode issue: %v", err) - } - } - - // Verify each line is valid JSON - lines := strings.Split(strings.TrimSpace(buf.String()), "\n") - if len(lines) != 3 { - t.Errorf("Expected 3 JSONL lines, got %d", len(lines)) - } - - for i, line := range lines { - var issue types.Issue - if err := json.Unmarshal([]byte(line), &issue); err != nil { - t.Errorf("Line %d is not valid JSON: %v", i, err) - } - } + exported := h.searchIssues(types.IssueFilter{}) + buf := h.encodeJSONL(exported) + h.validateJSONLines(buf, 3) }) // Test import into new database t.Run("Import", func(t *testing.T) { - // Export from original database - exported, _ := store.SearchIssues(ctx, "", types.IssueFilter{}) - - // Create new database + exported := h.searchIssues(types.IssueFilter{}) newDBPath := filepath.Join(tmpDir, "import-test.db") newStore, err := sqlite.New(newDBPath) if err != nil { t.Fatalf("Failed to create new storage: %v", err) } - - // Import issues + newHelper := newExportImportHelper(t, newStore) for _, issue := range exported { - if err := newStore.CreateIssue(ctx, issue, "import"); err != nil { - t.Fatalf("Failed to import issue: %v", err) - } + newHelper.createIssue(issue.ID, issue.Title, issue.Description, issue.Status, issue.Priority, issue.IssueType, issue.Assignee, issue.ClosedAt) } - - // Verify imported issues - imported, err := newStore.SearchIssues(ctx, "", types.IssueFilter{}) - if err != nil { - t.Fatalf("SearchIssues failed: %v", err) - } - - if len(imported) != len(exported) { - t.Errorf("Expected %d issues, got %d", len(exported), len(imported)) - } - - // Verify issue data + imported := newHelper.searchIssues(types.IssueFilter{}) + newHelper.assertCount(len(imported), len(exported), "issues") for i := range imported { - if imported[i].ID != exported[i].ID { - t.Errorf("Issue %d: ID = %s, want %s", i, imported[i].ID, exported[i].ID) - } - if imported[i].Title != exported[i].Title { - t.Errorf("Issue %d: Title = %s, want %s", i, imported[i].Title, exported[i].Title) - } + newHelper.assertEqual(exported[i].ID, imported[i].ID, "ID") + newHelper.assertEqual(exported[i].Title, imported[i].Title, "Title") } }) // Test update on import t.Run("Import Update", func(t *testing.T) { - // Get first issue - issue, err := store.GetIssue(ctx, "test-1") - if err != nil { - t.Fatalf("GetIssue failed: %v", err) - } - - // Modify it - issue.Title = "Updated title" - issue.Status = types.StatusClosed - - // Import as update - updates := map[string]interface{}{ - "title": issue.Title, - "status": string(issue.Status), - } - if err := store.UpdateIssue(ctx, issue.ID, updates, "test"); err != nil { - t.Fatalf("UpdateIssue failed: %v", err) - } - - // Verify update - updated, err := store.GetIssue(ctx, "test-1") - if err != nil { - t.Fatalf("GetIssue failed: %v", err) - } - - if updated.Title != "Updated title" { - t.Errorf("Title = %s, want 'Updated title'", updated.Title) - } - if updated.Status != types.StatusClosed { - t.Errorf("Status = %s, want %s", updated.Status, types.StatusClosed) - } + issue := h.getIssue("test-1") + updates := map[string]interface{}{"title": "Updated title", "status": string(types.StatusClosed)} + h.updateIssue(issue.ID, updates) + updated := h.getIssue("test-1") + h.assertEqual("Updated title", updated.Title, "Title") + h.assertEqual(types.StatusClosed, updated.Status, "Status") }) // Test filtering on export t.Run("Export with Filter", func(t *testing.T) { status := types.StatusOpen - filter := types.IssueFilter{ - Status: &status, - } - - filtered, err := store.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("SearchIssues failed: %v", err) - } - - // Should only get open issues (test-1 might be updated, so check count > 0) + filtered := h.searchIssues(types.IssueFilter{Status: &status}) for _, issue := range filtered { if issue.Status != types.StatusOpen { t.Errorf("Expected only open issues, got %s", issue.Status) @@ -285,38 +280,11 @@ func TestRoundTrip(t *testing.T) { t.Fatalf("Failed to create storage: %v", err) } - ctx := context.Background() - - // Create issue with all fields populated - estimatedMinutes := 120 - closedAt := time.Now() - original := &types.Issue{ - ID: "test-1", - Title: "Full issue", - Description: "Description", - Design: "Design doc", - AcceptanceCriteria: "Criteria", - Notes: "Notes", - Status: types.StatusClosed, - Priority: 1, - IssueType: types.TypeFeature, - Assignee: "alice", - EstimatedMinutes: &estimatedMinutes, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - ClosedAt: &closedAt, - } - - if err := store.CreateIssue(ctx, original, "test"); err != nil { - t.Fatalf("Failed to create issue: %v", err) - } + h := newExportImportHelper(t, store) + original := h.createFullIssue("test-1", 120) // Export to JSONL - var buf bytes.Buffer - encoder := json.NewEncoder(&buf) - if err := encoder.Encode(original); err != nil { - t.Fatalf("Failed to encode: %v", err) - } + buf := h.encodeJSONL([]*types.Issue{original}) // Import from JSONL var decoded types.Issue @@ -325,15 +293,9 @@ func TestRoundTrip(t *testing.T) { } // Verify all fields preserved - if decoded.ID != original.ID { - t.Errorf("ID = %s, want %s", decoded.ID, original.ID) - } - if decoded.Title != original.Title { - t.Errorf("Title = %s, want %s", decoded.Title, original.Title) - } - if decoded.Description != original.Description { - t.Errorf("Description = %s, want %s", decoded.Description, original.Description) - } + h.assertEqual(original.ID, decoded.ID, "ID") + h.assertEqual(original.Title, decoded.Title, "Title") + h.assertEqual(original.Description, decoded.Description, "Description") if decoded.EstimatedMinutes == nil || *decoded.EstimatedMinutes != *original.EstimatedMinutes { t.Errorf("EstimatedMinutes = %v, want %v", decoded.EstimatedMinutes, original.EstimatedMinutes) } diff --git a/cmd/bd/list_test.go b/cmd/bd/list_test.go index d4249bc9..e90da0dd 100644 --- a/cmd/bd/list_test.go +++ b/cmd/bd/list_test.go @@ -12,25 +12,21 @@ import ( "github.com/steveyegge/beads/internal/types" ) -func TestListCommand(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "bd-test-list-*") - if err != nil { - t.Fatalf("Failed to create temp dir: %v", err) - } - defer os.RemoveAll(tmpDir) +// listTestHelper provides test setup and assertion methods +type listTestHelper struct { + t *testing.T + ctx context.Context + store *sqlite.SQLiteStorage + issues []*types.Issue +} - testDB := filepath.Join(tmpDir, "test.db") - s, err := sqlite.New(testDB) - if err != nil { - t.Fatalf("Failed to create store: %v", err) - } - defer s.Close() +func newListTestHelper(t *testing.T, store *sqlite.SQLiteStorage) *listTestHelper { + return &listTestHelper{t: t, ctx: context.Background(), store: store} +} - ctx := context.Background() - - // Create test issues +func (h *listTestHelper) createTestIssues() { now := time.Now() - issues := []*types.Issue{ + h.issues = []*types.Issue{ { Title: "Bug Issue", Description: "Test bug", @@ -55,138 +51,116 @@ func TestListCommand(t *testing.T) { ClosedAt: &now, }, } - - for _, issue := range issues { - if err := s.CreateIssue(ctx, issue, "test-user"); err != nil { - t.Fatalf("Failed to create issue: %v", err) + for _, issue := range h.issues { + if err := h.store.CreateIssue(h.ctx, issue, "test-user"); err != nil { + h.t.Fatalf("Failed to create issue: %v", err) } } +} - // Add labels to first issue - if err := s.AddLabel(ctx, issues[0].ID, "critical", "test-user"); err != nil { - t.Fatalf("Failed to add label: %v", err) +func (h *listTestHelper) addLabel(id, label string) { + if err := h.store.AddLabel(h.ctx, id, label, "test-user"); err != nil { + h.t.Fatalf("Failed to add label: %v", err) } +} + +func (h *listTestHelper) search(filter types.IssueFilter) []*types.Issue { + results, err := h.store.SearchIssues(h.ctx, "", filter) + if err != nil { + h.t.Fatalf("Failed to search issues: %v", err) + } + return results +} + +func (h *listTestHelper) assertCount(count, expected int, desc string) { + if count != expected { + h.t.Errorf("Expected %d %s, got %d", expected, desc, count) + } +} + +func (h *listTestHelper) assertEqual(expected, actual interface{}, field string) { + if expected != actual { + h.t.Errorf("Expected %s %v, got %v", field, expected, actual) + } +} + +func (h *listTestHelper) assertAtMost(count, max int, desc string) { + if count > max { + h.t.Errorf("Expected at most %d %s, got %d", max, desc, count) + } +} + +func TestListCommand(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "bd-test-list-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + testDB := filepath.Join(tmpDir, "test.db") + s, err := sqlite.New(testDB) + if err != nil { + t.Fatalf("Failed to create store: %v", err) + } + defer s.Close() + + h := newListTestHelper(t, s) + h.createTestIssues() + h.addLabel(h.issues[0].ID, "critical") t.Run("list all issues", func(t *testing.T) { - filter := types.IssueFilter{} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 3 { - t.Errorf("Expected 3 issues, got %d", len(results)) - } + results := h.search(types.IssueFilter{}) + h.assertCount(len(results), 3, "issues") }) t.Run("filter by status", func(t *testing.T) { status := types.StatusOpen - filter := types.IssueFilter{Status: &status} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 open issue, got %d", len(results)) - } - if results[0].Status != types.StatusOpen { - t.Errorf("Expected status %s, got %s", types.StatusOpen, results[0].Status) - } + results := h.search(types.IssueFilter{Status: &status}) + h.assertCount(len(results), 1, "open issues") + h.assertEqual(types.StatusOpen, results[0].Status, "status") }) t.Run("filter by priority", func(t *testing.T) { priority := 0 - filter := types.IssueFilter{Priority: &priority} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 P0 issue, got %d", len(results)) - } - if results[0].Priority != 0 { - t.Errorf("Expected priority 0, got %d", results[0].Priority) - } + results := h.search(types.IssueFilter{Priority: &priority}) + h.assertCount(len(results), 1, "P0 issues") + h.assertEqual(0, results[0].Priority, "priority") }) t.Run("filter by assignee", func(t *testing.T) { assignee := "alice" - filter := types.IssueFilter{Assignee: &assignee} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 issue for alice, got %d", len(results)) - } - if results[0].Assignee != "alice" { - t.Errorf("Expected assignee alice, got %s", results[0].Assignee) - } + results := h.search(types.IssueFilter{Assignee: &assignee}) + h.assertCount(len(results), 1, "issues for alice") + h.assertEqual("alice", results[0].Assignee, "assignee") }) t.Run("filter by issue type", func(t *testing.T) { issueType := types.TypeBug - filter := types.IssueFilter{IssueType: &issueType} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 bug issue, got %d", len(results)) - } - if results[0].IssueType != types.TypeBug { - t.Errorf("Expected type %s, got %s", types.TypeBug, results[0].IssueType) - } + results := h.search(types.IssueFilter{IssueType: &issueType}) + h.assertCount(len(results), 1, "bug issues") + h.assertEqual(types.TypeBug, results[0].IssueType, "type") }) t.Run("filter by label", func(t *testing.T) { - filter := types.IssueFilter{Labels: []string{"critical"}} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 issue with critical label, got %d", len(results)) - } + results := h.search(types.IssueFilter{Labels: []string{"critical"}}) + h.assertCount(len(results), 1, "issues with critical label") }) t.Run("filter by title search", func(t *testing.T) { - filter := types.IssueFilter{TitleSearch: "Bug"} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) != 1 { - t.Errorf("Expected 1 issue matching 'Bug', got %d", len(results)) - } + results := h.search(types.IssueFilter{TitleSearch: "Bug"}) + h.assertCount(len(results), 1, "issues matching 'Bug'") }) t.Run("limit results", func(t *testing.T) { - filter := types.IssueFilter{Limit: 2} - results, err := s.SearchIssues(ctx, "", filter) - if err != nil { - t.Fatalf("Failed to search issues: %v", err) - } - - if len(results) > 2 { - t.Errorf("Expected at most 2 issues, got %d", len(results)) - } + results := h.search(types.IssueFilter{Limit: 2}) + h.assertAtMost(len(results), 2, "issues") }) t.Run("normalize labels", func(t *testing.T) { labels := []string{" bug ", "critical", "", "bug", " feature "} normalized := normalizeLabels(labels) - expected := []string{"bug", "critical", "feature"} - if len(normalized) != len(expected) { - t.Errorf("Expected %d normalized labels, got %d", len(expected), len(normalized)) - } + h.assertCount(len(normalized), len(expected), "normalized labels") // Check deduplication and trimming seen := make(map[string]bool) diff --git a/internal/storage/sqlite/epics_test.go b/internal/storage/sqlite/epics_test.go index ee4b1173..37a7bfd0 100644 --- a/internal/storage/sqlite/epics_test.go +++ b/internal/storage/sqlite/epics_test.go @@ -7,208 +7,150 @@ import ( "github.com/steveyegge/beads/internal/types" ) -func TestGetEpicsEligibleForClosure(t *testing.T) { - store, cleanup := setupTestDB(t) - defer cleanup() +// epicTestHelper provides test setup and assertion methods +type epicTestHelper struct { + t *testing.T + ctx context.Context + store *SQLiteStorage +} - ctx := context.Background() +func newEpicTestHelper(t *testing.T, store *SQLiteStorage) *epicTestHelper { + return &epicTestHelper{t: t, ctx: context.Background(), store: store} +} - // Create an epic +func (h *epicTestHelper) createEpic(title string) *types.Issue { epic := &types.Issue{ - Title: "Test Epic", + Title: title, Description: "Epic for testing", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeEpic, } - err := store.CreateIssue(ctx, epic, "test-user") - if err != nil { - t.Fatalf("CreateIssue (epic) failed: %v", err) + if err := h.store.CreateIssue(h.ctx, epic, "test-user"); err != nil { + h.t.Fatalf("CreateIssue (epic) failed: %v", err) } + return epic +} - // Create two child tasks - task1 := &types.Issue{ - Title: "Task 1", +func (h *epicTestHelper) createTask(title string) *types.Issue { + task := &types.Issue{ + Title: title, Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask, } - err = store.CreateIssue(ctx, task1, "test-user") - if err != nil { - t.Fatalf("CreateIssue (task1) failed: %v", err) + if err := h.store.CreateIssue(h.ctx, task, "test-user"); err != nil { + h.t.Fatalf("CreateIssue (%s) failed: %v", title, err) } + return task +} - task2 := &types.Issue{ - Title: "Task 2", - Status: types.StatusOpen, - Priority: 2, - IssueType: types.TypeTask, - } - err = store.CreateIssue(ctx, task2, "test-user") - if err != nil { - t.Fatalf("CreateIssue (task2) failed: %v", err) - } - - // Add parent-child dependencies - dep1 := &types.Dependency{ - IssueID: task1.ID, - DependsOnID: epic.ID, +func (h *epicTestHelper) addParentChildDependency(childID, parentID string) { + dep := &types.Dependency{ + IssueID: childID, + DependsOnID: parentID, Type: types.DepParentChild, } - err = store.AddDependency(ctx, dep1, "test-user") - if err != nil { - t.Fatalf("AddDependency (task1) failed: %v", err) + if err := h.store.AddDependency(h.ctx, dep, "test-user"); err != nil { + h.t.Fatalf("AddDependency failed: %v", err) } +} - dep2 := &types.Dependency{ - IssueID: task2.ID, - DependsOnID: epic.ID, - Type: types.DepParentChild, - } - err = store.AddDependency(ctx, dep2, "test-user") - if err != nil { - t.Fatalf("AddDependency (task2) failed: %v", err) +func (h *epicTestHelper) closeIssue(id, reason string) { + if err := h.store.CloseIssue(h.ctx, id, reason, "test-user"); err != nil { + h.t.Fatalf("CloseIssue (%s) failed: %v", id, err) } +} - // Test 1: Epic with open children should NOT be eligible for closure - epics, err := store.GetEpicsEligibleForClosure(ctx) +func (h *epicTestHelper) getEligibleEpics() []*types.EpicStatus { + epics, err := h.store.GetEpicsEligibleForClosure(h.ctx) if err != nil { - t.Fatalf("GetEpicsEligibleForClosure failed: %v", err) + h.t.Fatalf("GetEpicsEligibleForClosure failed: %v", err) } + return epics +} +func (h *epicTestHelper) findEpic(epics []*types.EpicStatus, epicID string) (*types.EpicStatus, bool) { + for _, e := range epics { + if e.Epic.ID == epicID { + return e, true + } + } + return nil, false +} + +func (h *epicTestHelper) assertEpicStats(epic *types.EpicStatus, totalChildren, closedChildren int, eligible bool, desc string) { + if epic.TotalChildren != totalChildren { + h.t.Errorf("%s: Expected %d total children, got %d", desc, totalChildren, epic.TotalChildren) + } + if epic.ClosedChildren != closedChildren { + h.t.Errorf("%s: Expected %d closed children, got %d", desc, closedChildren, epic.ClosedChildren) + } + if epic.EligibleForClose != eligible { + h.t.Errorf("%s: Expected eligible=%v, got %v", desc, eligible, epic.EligibleForClose) + } +} + +func (h *epicTestHelper) assertEpicNotFound(epics []*types.EpicStatus, epicID string, desc string) { + if _, found := h.findEpic(epics, epicID); found { + h.t.Errorf("%s: Epic %s should not be in results", desc, epicID) + } +} + +func (h *epicTestHelper) assertEpicFound(epics []*types.EpicStatus, epicID string, desc string) *types.EpicStatus { + epic, found := h.findEpic(epics, epicID) + if !found { + h.t.Fatalf("%s: Epic %s not found in results", desc, epicID) + } + return epic +} + +func TestGetEpicsEligibleForClosure(t *testing.T) { + store, cleanup := setupTestDB(t) + defer cleanup() + + h := newEpicTestHelper(t, store) + epic := h.createEpic("Test Epic") + task1 := h.createTask("Task 1") + task2 := h.createTask("Task 2") + h.addParentChildDependency(task1.ID, epic.ID) + h.addParentChildDependency(task2.ID, epic.ID) + + // Test 1: Epic with open children should NOT be eligible + epics := h.getEligibleEpics() if len(epics) == 0 { t.Fatal("Expected at least one epic") } - - found := false - for _, e := range epics { - if e.Epic.ID == epic.ID { - found = true - if e.TotalChildren != 2 { - t.Errorf("Expected 2 total children, got %d", e.TotalChildren) - } - if e.ClosedChildren != 0 { - t.Errorf("Expected 0 closed children, got %d", e.ClosedChildren) - } - if e.EligibleForClose { - t.Error("Epic should NOT be eligible for closure with open children") - } - } - } - if !found { - t.Error("Epic not found in results") - } + e := h.assertEpicFound(epics, epic.ID, "All children open") + h.assertEpicStats(e, 2, 0, false, "All children open") // Test 2: Close one task - err = store.CloseIssue(ctx, task1.ID, "Done", "test-user") - if err != nil { - t.Fatalf("CloseIssue (task1) failed: %v", err) - } - - epics, err = store.GetEpicsEligibleForClosure(ctx) - if err != nil { - t.Fatalf("GetEpicsEligibleForClosure (after closing task1) failed: %v", err) - } - - found = false - for _, e := range epics { - if e.Epic.ID == epic.ID { - found = true - if e.ClosedChildren != 1 { - t.Errorf("Expected 1 closed child, got %d", e.ClosedChildren) - } - if e.EligibleForClose { - t.Error("Epic should NOT be eligible with only 1/2 tasks closed") - } - } - } - if !found { - t.Error("Epic not found after closing one task") - } + h.closeIssue(task1.ID, "Done") + epics = h.getEligibleEpics() + e = h.assertEpicFound(epics, epic.ID, "One child closed") + h.assertEpicStats(e, 2, 1, false, "One child closed") // Test 3: Close second task - epic should be eligible - err = store.CloseIssue(ctx, task2.ID, "Done", "test-user") - if err != nil { - t.Fatalf("CloseIssue (task2) failed: %v", err) - } + h.closeIssue(task2.ID, "Done") + epics = h.getEligibleEpics() + e = h.assertEpicFound(epics, epic.ID, "All children closed") + h.assertEpicStats(e, 2, 2, true, "All children closed") - epics, err = store.GetEpicsEligibleForClosure(ctx) - if err != nil { - t.Fatalf("GetEpicsEligibleForClosure (after closing task2) failed: %v", err) - } - - found = false - for _, e := range epics { - if e.Epic.ID == epic.ID { - found = true - if e.ClosedChildren != 2 { - t.Errorf("Expected 2 closed children, got %d", e.ClosedChildren) - } - if !e.EligibleForClose { - t.Error("Epic SHOULD be eligible for closure with all children closed") - } - } - } - if !found { - t.Error("Epic not found after closing all tasks") - } - - // Test 4: Close the epic - should no longer appear in results - err = store.CloseIssue(ctx, epic.ID, "All tasks complete", "test-user") - if err != nil { - t.Fatalf("CloseIssue (epic) failed: %v", err) - } - - epics, err = store.GetEpicsEligibleForClosure(ctx) - if err != nil { - t.Fatalf("GetEpicsEligibleForClosure (after closing epic) failed: %v", err) - } - - // Closed epics should not appear in results - for _, e := range epics { - if e.Epic.ID == epic.ID { - t.Error("Closed epic should not appear in eligible list") - } - } + // Test 4: Close the epic - should no longer appear + h.closeIssue(epic.ID, "All tasks complete") + epics = h.getEligibleEpics() + h.assertEpicNotFound(epics, epic.ID, "Closed epic") } func TestGetEpicsEligibleForClosureWithNoChildren(t *testing.T) { store, cleanup := setupTestDB(t) defer cleanup() - ctx := context.Background() + h := newEpicTestHelper(t, store) + epic := h.createEpic("Childless Epic") + epics := h.getEligibleEpics() - // Create an epic with no children - epic := &types.Issue{ - Title: "Childless Epic", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeEpic, - } - err := store.CreateIssue(ctx, epic, "test-user") - if err != nil { - t.Fatalf("CreateIssue failed: %v", err) - } - - epics, err := store.GetEpicsEligibleForClosure(ctx) - if err != nil { - t.Fatalf("GetEpicsEligibleForClosure failed: %v", err) - } - - // Should find the epic but it should NOT be eligible (no children = not eligible) - found := false - for _, e := range epics { - if e.Epic.ID == epic.ID { - found = true - if e.TotalChildren != 0 { - t.Errorf("Expected 0 total children, got %d", e.TotalChildren) - } - if e.EligibleForClose { - t.Error("Epic with no children should NOT be eligible for closure") - } - } - } - if !found { - t.Error("Epic not found in results") - } + // Should find the epic but it should NOT be eligible + e := h.assertEpicFound(epics, epic.ID, "No children") + h.assertEpicStats(e, 0, 0, false, "No children") } diff --git a/internal/storage/sqlite/sqlite_test.go b/internal/storage/sqlite/sqlite_test.go index daa9d854..876a00cf 100644 --- a/internal/storage/sqlite/sqlite_test.go +++ b/internal/storage/sqlite/sqlite_test.go @@ -192,313 +192,248 @@ func TestGetIssueNotFound(t *testing.T) { } } +// createIssuesTestHelper provides test setup and assertion methods +type createIssuesTestHelper struct { + t *testing.T + ctx context.Context + store *SQLiteStorage +} + +func newCreateIssuesHelper(t *testing.T, store *SQLiteStorage) *createIssuesTestHelper { + return &createIssuesTestHelper{t: t, ctx: context.Background(), store: store} +} + +func (h *createIssuesTestHelper) newIssue(id, title string, status types.Status, priority int, issueType types.IssueType, closedAt *time.Time) *types.Issue { + return &types.Issue{ + ID: id, + Title: title, + Status: status, + Priority: priority, + IssueType: issueType, + ClosedAt: closedAt, + } +} + +func (h *createIssuesTestHelper) createIssues(issues []*types.Issue) error { + return h.store.CreateIssues(h.ctx, issues, "test-user") +} + +func (h *createIssuesTestHelper) assertNoError(err error) { + if err != nil { + h.t.Errorf("CreateIssues() unexpected error: %v", err) + } +} + +func (h *createIssuesTestHelper) assertError(err error) { + if err == nil { + h.t.Error("CreateIssues() expected error, got nil") + } +} + +func (h *createIssuesTestHelper) assertCount(issues []*types.Issue, expected int) { + if len(issues) != expected { + h.t.Errorf("expected %d issues, got %d", expected, len(issues)) + } +} + +func (h *createIssuesTestHelper) assertIDSet(issue *types.Issue, index int) { + if issue.ID == "" { + h.t.Errorf("issue %d: ID should be set", index) + } +} + +func (h *createIssuesTestHelper) assertTimestampSet(ts time.Time, field string, index int) { + if !ts.After(time.Time{}) { + h.t.Errorf("issue %d: %s should be set", index, field) + } +} + +func (h *createIssuesTestHelper) assertUniqueIDs(issues []*types.Issue) { + ids := make(map[string]bool) + for _, issue := range issues { + if ids[issue.ID] { + h.t.Errorf("duplicate ID found: %s", issue.ID) + } + ids[issue.ID] = true + } +} + +func (h *createIssuesTestHelper) assertEqual(expected, actual interface{}, field string) { + if expected != actual { + h.t.Errorf("expected %s %v, got %v", field, expected, actual) + } +} + +func (h *createIssuesTestHelper) assertNotNil(value interface{}, field string) { + if value == nil { + h.t.Errorf("%s should be set", field) + } +} + +func (h *createIssuesTestHelper) assertNoAutoGenID(issues []*types.Issue, wantErr bool) { + if !wantErr { + return + } + for i, issue := range issues { + if issue == nil { + continue + } + hasCustomID := issue.ID != "" && (issue.ID == "custom-1" || issue.ID == "custom-2" || + issue.ID == "duplicate-id" || issue.ID == "existing-id") + if !hasCustomID && issue.ID != "" { + h.t.Errorf("issue %d: ID should not be auto-generated on error, got %s", i, issue.ID) + } + } +} + func TestCreateIssues(t *testing.T) { store, cleanup := setupTestDB(t) defer cleanup() - ctx := context.Background() + h := newCreateIssuesHelper(t, store) tests := []struct { name string issues []*types.Issue wantErr bool - checkFunc func(t *testing.T, issues []*types.Issue) + checkFunc func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) }{ { name: "empty batch", issues: []*types.Issue{}, wantErr: false, - checkFunc: func(t *testing.T, issues []*types.Issue) { - if len(issues) != 0 { - t.Errorf("expected 0 issues, got %d", len(issues)) - } + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) { + h.assertCount(issues, 0) }, }, { name: "single issue", issues: []*types.Issue{ - { - Title: "Single issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("", "Single issue", types.StatusOpen, 1, types.TypeTask, nil), }, wantErr: false, - checkFunc: func(t *testing.T, issues []*types.Issue) { - if len(issues) != 1 { - t.Fatalf("expected 1 issue, got %d", len(issues)) - } - if issues[0].ID == "" { - t.Error("issue ID should be set") - } - if !issues[0].CreatedAt.After(time.Time{}) { - t.Error("CreatedAt should be set") - } - if !issues[0].UpdatedAt.After(time.Time{}) { - t.Error("UpdatedAt should be set") - } + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) { + h.assertCount(issues, 1) + h.assertIDSet(issues[0], 0) + h.assertTimestampSet(issues[0].CreatedAt, "CreatedAt", 0) + h.assertTimestampSet(issues[0].UpdatedAt, "UpdatedAt", 0) }, }, { name: "multiple issues", issues: []*types.Issue{ - { - Title: "Issue 1", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, - { - Title: "Issue 2", - Status: types.StatusInProgress, - Priority: 2, - IssueType: types.TypeBug, - }, - { - Title: "Issue 3", - Status: types.StatusOpen, - Priority: 3, - IssueType: types.TypeFeature, - }, + h.newIssue("", "Issue 1", types.StatusOpen, 1, types.TypeTask, nil), + h.newIssue("", "Issue 2", types.StatusInProgress, 2, types.TypeBug, nil), + h.newIssue("", "Issue 3", types.StatusOpen, 3, types.TypeFeature, nil), }, wantErr: false, - checkFunc: func(t *testing.T, issues []*types.Issue) { - if len(issues) != 3 { - t.Fatalf("expected 3 issues, got %d", len(issues)) - } + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) { + h.assertCount(issues, 3) for i, issue := range issues { - if issue.ID == "" { - t.Errorf("issue %d: ID should be set", i) - } - if !issue.CreatedAt.After(time.Time{}) { - t.Errorf("issue %d: CreatedAt should be set", i) - } - if !issue.UpdatedAt.After(time.Time{}) { - t.Errorf("issue %d: UpdatedAt should be set", i) - } - } - // Verify IDs are unique - ids := make(map[string]bool) - for _, issue := range issues { - if ids[issue.ID] { - t.Errorf("duplicate ID found: %s", issue.ID) - } - ids[issue.ID] = true + h.assertIDSet(issue, i) + h.assertTimestampSet(issue.CreatedAt, "CreatedAt", i) + h.assertTimestampSet(issue.UpdatedAt, "UpdatedAt", i) } + h.assertUniqueIDs(issues) }, }, { name: "mixed ID assignment - explicit and auto-generated", issues: []*types.Issue{ - { - ID: "custom-1", - Title: "Custom ID 1", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, - { - Title: "Auto ID", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, - { - ID: "custom-2", - Title: "Custom ID 2", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("custom-1", "Custom ID 1", types.StatusOpen, 1, types.TypeTask, nil), + h.newIssue("", "Auto ID", types.StatusOpen, 1, types.TypeTask, nil), + h.newIssue("custom-2", "Custom ID 2", types.StatusOpen, 1, types.TypeTask, nil), }, wantErr: false, - checkFunc: func(t *testing.T, issues []*types.Issue) { - if len(issues) != 3 { - t.Fatalf("expected 3 issues, got %d", len(issues)) - } - if issues[0].ID != "custom-1" { - t.Errorf("expected ID 'custom-1', got %s", issues[0].ID) - } + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) { + h.assertCount(issues, 3) + h.assertEqual("custom-1", issues[0].ID, "ID") if issues[1].ID == "" || issues[1].ID == "custom-1" || issues[1].ID == "custom-2" { t.Errorf("expected auto-generated ID, got %s", issues[1].ID) } - if issues[2].ID != "custom-2" { - t.Errorf("expected ID 'custom-2', got %s", issues[2].ID) - } + h.assertEqual("custom-2", issues[2].ID, "ID") }, }, { name: "validation error - missing title", issues: []*types.Issue{ - { - Title: "Valid issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, - { - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("", "Valid issue", types.StatusOpen, 1, types.TypeTask, nil), + h.newIssue("", "", types.StatusOpen, 1, types.TypeTask, nil), }, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { - name: "validation error - invalid priority", - issues: []*types.Issue{ - { - Title: "Test", - Status: types.StatusOpen, - Priority: 10, - IssueType: types.TypeTask, - }, - }, + name: "validation error - invalid priority", + issues: []*types.Issue{h.newIssue("", "Test", types.StatusOpen, 10, types.TypeTask, nil)}, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { - name: "validation error - invalid status", - issues: []*types.Issue{ - { - Title: "Test", - Status: "invalid", - Priority: 1, - IssueType: types.TypeTask, - }, - }, + name: "validation error - invalid status", + issues: []*types.Issue{h.newIssue("", "Test", "invalid", 1, types.TypeTask, nil)}, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { name: "duplicate ID error", issues: []*types.Issue{ - { - ID: "duplicate-id", - Title: "First issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, - { - ID: "duplicate-id", - Title: "Second issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("duplicate-id", "First issue", types.StatusOpen, 1, types.TypeTask, nil), + h.newIssue("duplicate-id", "Second issue", types.StatusOpen, 1, types.TypeTask, nil), }, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { name: "closed_at invariant - open status with closed_at", issues: []*types.Issue{ - { - Title: "Invalid closed_at", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - ClosedAt: &time.Time{}, - }, + h.newIssue("", "Invalid closed_at", types.StatusOpen, 1, types.TypeTask, &time.Time{}), }, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { name: "closed_at invariant - closed status without closed_at", issues: []*types.Issue{ - { - Title: "Missing closed_at", - Status: types.StatusClosed, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("", "Missing closed_at", types.StatusClosed, 1, types.TypeTask, nil), }, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { name: "nil item in batch", issues: []*types.Issue{ - { - Title: "Valid issue", - Status: types.StatusOpen, - Priority: 1, - IssueType: types.TypeTask, - }, + h.newIssue("", "Valid issue", types.StatusOpen, 1, types.TypeTask, nil), nil, }, wantErr: true, - checkFunc: func(t *testing.T, issues []*types.Issue) { - // Should not be called on error - }, + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) {}, }, { name: "valid closed issue with closed_at", issues: []*types.Issue{ - { - Title: "Properly closed", - Status: types.StatusClosed, - Priority: 1, - IssueType: types.TypeTask, - ClosedAt: func() *time.Time { t := time.Now(); return &t }(), - }, + h.newIssue("", "Properly closed", types.StatusClosed, 1, types.TypeTask, func() *time.Time { t := time.Now(); return &t }()), }, wantErr: false, - checkFunc: func(t *testing.T, issues []*types.Issue) { - if len(issues) != 1 { - t.Fatalf("expected 1 issue, got %d", len(issues)) - } - if issues[0].Status != types.StatusClosed { - t.Errorf("expected closed status, got %s", issues[0].Status) - } - if issues[0].ClosedAt == nil { - t.Error("ClosedAt should be set for closed issue") - } + checkFunc: func(t *testing.T, h *createIssuesTestHelper, issues []*types.Issue) { + h.assertCount(issues, 1) + h.assertEqual(types.StatusClosed, issues[0].Status, "status") + h.assertNotNil(issues[0].ClosedAt, "ClosedAt") }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := store.CreateIssues(ctx, tt.issues, "test-user") - if (err != nil) != tt.wantErr { - t.Errorf("CreateIssues() error = %v, wantErr %v", err, tt.wantErr) - return - } - + err := h.createIssues(tt.issues) if tt.wantErr { - // Verify IDs weren't auto-generated on error (timestamps may be set) - for i, issue := range tt.issues { - if issue == nil { - continue - } - // Allow pre-set IDs (custom-1, existing-id, duplicate-id, etc.) - hasCustomID := issue.ID != "" && (issue.ID == "custom-1" || issue.ID == "custom-2" || - issue.ID == "duplicate-id" || issue.ID == "existing-id") - if !hasCustomID && issue.ID != "" { - t.Errorf("issue %d: ID should not be auto-generated on error, got %s", i, issue.ID) - } - } + h.assertError(err) + h.assertNoAutoGenID(tt.issues, tt.wantErr) + } else { + h.assertNoError(err) } - if !tt.wantErr && tt.checkFunc != nil { - tt.checkFunc(t, tt.issues) + tt.checkFunc(t, h, tt.issues) } }) }