From 47c915ef10cb2c76d50ed0f96d11e8f691a5d47b Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sat, 25 Oct 2025 13:33:51 -0700 Subject: [PATCH] Fix goconst linter warnings by converting repeated strings to constants - Added testUserAlice constant for 'alice' in test files - Added windowsOS constant for 'windows' in test files - Added testIssueBD1/testIssueBD2 constants for 'bd-1'/'bd-2' in test files - Added testVersion100 constant for '1.0.0' in version tests - Added testIssueCustom1 constant for 'custom-1' in lazy init tests Closes bd-54 Amp-Thread-ID: https://ampcode.com/threads/T-0a4e5d44-2d95-4948-8f4a-d8facf8657c7 Co-authored-by: Amp --- .beads/beads.jsonl | 2 +- cmd/bd/comments_test.go | 6 ++-- cmd/bd/daemon_test.go | 4 ++- cmd/bd/import_collision_regression_test.go | 35 +++++++++++-------- cmd/bd/list_test.go | 6 ++-- cmd/bd/main_test.go | 2 +- internal/rpc/version_test.go | 36 ++++++++++--------- internal/storage/sqlite/collision_test.go | 40 ++++++++++++---------- internal/storage/sqlite/events_test.go | 6 ++-- internal/storage/sqlite/lazy_init_test.go | 4 ++- 10 files changed, 79 insertions(+), 62 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index 4f245df6..34a89b31 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -81,7 +81,7 @@ {"id":"bd-51","title":"Race condition between git commit and auto-flush debounce","description":"When using MCP/daemon mode, operations trigger a 5-second debounced auto-flush to JSONL. This creates a race condition with git commits, leaving the working tree dirty.\n\n**Example scenario:**\n1. User closes issue via MCP → daemon schedules flush (5 sec delay)\n2. User commits code changes → JSONL appears clean\n3. Daemon flush fires → JSONL modified after commit\n4. Result: dirty working tree showing JSONL changes\n\n**Root cause:**\n- Auto-flush uses 5-second debounce to batch changes\n- Git commits happen immediately\n- No coordination between flush schedule and git operations\n\n**Possible solutions:**\n\n1. **Immediate flush before git operations**\n - Detect git commands (commit, status, push)\n - Force immediate flush if pending\n - Pros: Clean working tree guaranteed\n - Cons: Requires hooking git, may be slow\n\n2. **Commit includes pending flushes**\n - Add `bd sync` to commit workflow\n - Wait for flush to complete before committing\n - Pros: Simple, explicit\n - Cons: Requires user discipline\n\n3. **Git hooks integration**\n - pre-commit hook: `bd sync --wait`\n - Ensures JSONL is up-to-date before commit\n - Pros: Automatic, reliable\n - Cons: Requires hook installation\n\n4. **Reduce debounce delay**\n - Lower from 5s to 1s or 500ms\n - Pros: Faster sync, less likely to race\n - Cons: More frequent I/O, doesn't eliminate race\n\n5. **Lock-based coordination**\n - Daemon holds lock while flush pending\n - Git operations wait for lock\n - Pros: Guarantees ordering\n - Cons: Complex, may block operations\n\n**Recommended approach:**\nCombine #2 and #3:\n- Add `bd sync` command to explicitly flush\n- Provide git hooks in `examples/git-hooks/`\n- Document workflow in AGENTS.md\n- Keep 5s debounce for normal operations\n\n**Related:**\n- bd-50 (daemon version detection)","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-23T23:16:29.502191-07:00","updated_at":"2025-10-24T22:17:18.871457-07:00","closed_at":"2025-10-24T22:17:18.871457-07:00"} {"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-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":"closed","priority":2,"issue_type":"task","created_at":"2025-10-24T01:01:36.9778-07:00","updated_at":"2025-10-25T13:30:00.130626-07:00","closed_at":"2025-10-25T13:30:00.130626-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\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"}]} diff --git a/cmd/bd/comments_test.go b/cmd/bd/comments_test.go index 514504b0..7abf3942 100644 --- a/cmd/bd/comments_test.go +++ b/cmd/bd/comments_test.go @@ -11,6 +11,8 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const testUserAlice = "alice" + func TestCommentsCommand(t *testing.T) { tmpDir, err := os.MkdirTemp("", "bd-test-comments-*") if err != nil { @@ -41,7 +43,7 @@ func TestCommentsCommand(t *testing.T) { } t.Run("add comment", func(t *testing.T) { - comment, err := s.AddIssueComment(ctx, issue.ID, "alice", "This is a test comment") + comment, err := s.AddIssueComment(ctx, issue.ID, testUserAlice, "This is a test comment") if err != nil { t.Fatalf("Failed to add comment: %v", err) } @@ -49,7 +51,7 @@ func TestCommentsCommand(t *testing.T) { if comment.IssueID != issue.ID { t.Errorf("Expected issue ID %s, got %s", issue.ID, comment.IssueID) } - if comment.Author != "alice" { + if comment.Author != testUserAlice { t.Errorf("Expected author alice, got %s", comment.Author) } if comment.Text != "This is a test comment" { diff --git a/cmd/bd/daemon_test.go b/cmd/bd/daemon_test.go index 460fa0b9..74cf2981 100644 --- a/cmd/bd/daemon_test.go +++ b/cmd/bd/daemon_test.go @@ -18,11 +18,13 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const windowsOS = "windows" + func makeSocketTempDir(t testing.TB) string { t.Helper() base := "/tmp" - if runtime.GOOS == "windows" { + if runtime.GOOS == windowsOS { base = os.TempDir() } else if _, err := os.Stat(base); err != nil { base = os.TempDir() diff --git a/cmd/bd/import_collision_regression_test.go b/cmd/bd/import_collision_regression_test.go index fcecd9e1..4452d930 100644 --- a/cmd/bd/import_collision_regression_test.go +++ b/cmd/bd/import_collision_regression_test.go @@ -10,6 +10,11 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const ( + testIssueBD1 = "bd-1" + testIssueBD2 = "bd-2" +) + // TestRemapCollisionsRemapsImportedNotExisting verifies the bug fix where collision // resolution incorrectly modified existing issue dependencies. // @@ -38,7 +43,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { // Step 1: Create existing issues with dependencies existingIssues := []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Existing BD-1", Description: "Original database issue 1, depends on bd-2", Status: types.StatusOpen, @@ -46,7 +51,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { IssueType: types.TypeTask, }, { - ID: "bd-2", + ID: testIssueBD2, Title: "Existing BD-2", Description: "Original database issue 2", Status: types.StatusOpen, @@ -56,7 +61,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { { ID: "bd-3", Title: "Existing BD-3", - Description: "Original database issue 3, depends on bd-1", + Description: "Original database issue 3, depends on " + testIssueBD1, Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask, @@ -71,13 +76,13 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { // Add dependencies between existing issues dep1 := &types.Dependency{ - IssueID: "bd-1", - DependsOnID: "bd-2", + IssueID: testIssueBD1, + DependsOnID: testIssueBD2, Type: types.DepBlocks, } dep2 := &types.Dependency{ IssueID: "bd-3", - DependsOnID: "bd-1", + DependsOnID: testIssueBD1, Type: types.DepBlocks, } if err := store.AddDependency(ctx, dep1, "test"); err != nil { @@ -90,7 +95,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { // Step 2: Simulate importing issues with same IDs but different content importedIssues := []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Imported BD-1", Description: "From import", Status: types.StatusOpen, @@ -98,7 +103,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { IssueType: types.TypeTask, }, { - ID: "bd-2", + ID: testIssueBD2, Title: "Imported BD-2", Description: "From import", Status: types.StatusOpen, @@ -172,7 +177,7 @@ func TestRemapCollisionsRemapsImportedNotExisting(t *testing.T) { t.Errorf("Expected 1 dependency for bd-3, got %d", len(existingDeps3)) } else { // Verify the dependency is correct - if existingDeps3[0].DependsOnID != "bd-1" { + if existingDeps3[0].DependsOnID != testIssueBD1 { t.Errorf("Expected bd-3 → bd-1, got bd-3 → %s", existingDeps3[0].DependsOnID) } } @@ -206,7 +211,7 @@ func TestRemapCollisionsDoesNotUpdateNonexistentDependencies(t *testing.T) { // Step 1: Create existing issue with dependency existing1 := &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Existing BD-1", Description: "Original database issue", Status: types.StatusOpen, @@ -214,7 +219,7 @@ func TestRemapCollisionsDoesNotUpdateNonexistentDependencies(t *testing.T) { IssueType: types.TypeTask, } existing2 := &types.Issue{ - ID: "bd-2", + ID: testIssueBD2, Title: "Existing BD-2", Description: "Original database issue", Status: types.StatusOpen, @@ -230,8 +235,8 @@ func TestRemapCollisionsDoesNotUpdateNonexistentDependencies(t *testing.T) { // Add dependency between existing issues existingDep := &types.Dependency{ - IssueID: "bd-1", - DependsOnID: "bd-2", + IssueID: testIssueBD1, + DependsOnID: testIssueBD2, Type: types.DepBlocks, } if err := store.AddDependency(ctx, existingDep, "test"); err != nil { @@ -241,7 +246,7 @@ func TestRemapCollisionsDoesNotUpdateNonexistentDependencies(t *testing.T) { // Step 2: Import colliding issues (without dependencies in DB) imported := []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Imported BD-1", Description: "From import, will be remapped", Status: types.StatusOpen, @@ -276,7 +281,7 @@ func TestRemapCollisionsDoesNotUpdateNonexistentDependencies(t *testing.T) { if len(existingDeps) != 1 { t.Errorf("Expected 1 dependency for existing bd-1, got %d (dependency should not be touched)", len(existingDeps)) } else { - if existingDeps[0].DependsOnID != "bd-2" { + if existingDeps[0].DependsOnID != testIssueBD2 { t.Errorf("Expected bd-1 → bd-2, got bd-1 → %s", existingDeps[0].DependsOnID) } } diff --git a/cmd/bd/list_test.go b/cmd/bd/list_test.go index e90da0dd..e9008521 100644 --- a/cmd/bd/list_test.go +++ b/cmd/bd/list_test.go @@ -40,7 +40,7 @@ func (h *listTestHelper) createTestIssues() { Priority: 1, IssueType: types.TypeFeature, Status: types.StatusInProgress, - Assignee: "alice", + Assignee: testUserAlice, }, { Title: "Task Issue", @@ -128,10 +128,10 @@ func TestListCommand(t *testing.T) { }) t.Run("filter by assignee", func(t *testing.T) { - assignee := "alice" + assignee := testUserAlice results := h.search(types.IssueFilter{Assignee: &assignee}) h.assertCount(len(results), 1, "issues for alice") - h.assertEqual("alice", results[0].Assignee, "assignee") + h.assertEqual(testUserAlice, results[0].Assignee, "assignee") }) t.Run("filter by issue type", func(t *testing.T) { diff --git a/cmd/bd/main_test.go b/cmd/bd/main_test.go index 5bfaa6f9..cbaabf86 100644 --- a/cmd/bd/main_test.go +++ b/cmd/bd/main_test.go @@ -556,7 +556,7 @@ func TestAutoFlushJSONLContent(t *testing.T) { // TestAutoFlushErrorHandling tests error scenarios in flush operations func TestAutoFlushErrorHandling(t *testing.T) { - if runtime.GOOS == "windows" { + if runtime.GOOS == windowsOS { t.Skip("chmod-based read-only directory behavior is not reliable on Windows") } diff --git a/internal/rpc/version_test.go b/internal/rpc/version_test.go index 0549c751..7b437820 100644 --- a/internal/rpc/version_test.go +++ b/internal/rpc/version_test.go @@ -10,6 +10,8 @@ import ( sqlitestorage "github.com/steveyegge/beads/internal/storage/sqlite" ) +const testVersion100 = "1.0.0" + func TestVersionCompatibility(t *testing.T) { tests := []struct { name string @@ -20,8 +22,8 @@ func TestVersionCompatibility(t *testing.T) { }{ { name: "Exact version match", - serverVersion: "1.0.0", - clientVersion: "1.0.0", + serverVersion: testVersion100, + clientVersion: testVersion100, shouldWork: true, }, { @@ -39,7 +41,7 @@ func TestVersionCompatibility(t *testing.T) { }, { name: "Different major versions - client newer", - serverVersion: "1.0.0", + serverVersion: testVersion100, clientVersion: "2.0.0", shouldWork: false, errorContains: "incompatible major versions", @@ -47,13 +49,13 @@ func TestVersionCompatibility(t *testing.T) { { name: "Different major versions - daemon newer", serverVersion: "2.0.0", - clientVersion: "1.0.0", + clientVersion: testVersion100, shouldWork: false, errorContains: "incompatible major versions", }, { name: "Empty client version (legacy client)", - serverVersion: "1.0.0", + serverVersion: testVersion100, clientVersion: "", shouldWork: true, }, @@ -65,8 +67,8 @@ func TestVersionCompatibility(t *testing.T) { }, { name: "Version without v prefix", - serverVersion: "1.0.0", - clientVersion: "1.0.0", + serverVersion: testVersion100, + clientVersion: testVersion100, shouldWork: true, }, { @@ -182,8 +184,8 @@ func TestHealthCheckIncludesVersionInfo(t *testing.T) { defer store.Close() // Set explicit versions - ServerVersion = "1.0.0" - ClientVersion = "1.0.0" + ServerVersion = testVersion100 + ClientVersion = testVersion100 server := NewServer(socketPath, store) @@ -246,7 +248,7 @@ func TestIncompatibleVersionInHealth(t *testing.T) { defer store.Close() // Set incompatible versions - ServerVersion = "1.0.0" + ServerVersion = testVersion100 ClientVersion = "2.0.0" server := NewServer(socketPath, store) @@ -304,7 +306,7 @@ func TestVersionCheckMessage(t *testing.T) { }{ { name: "Major mismatch - daemon older", - serverVersion: "1.0.0", + serverVersion: testVersion100, clientVersion: "2.0.0", expectError: true, errorContains: "Daemon is older; upgrade and restart daemon", @@ -312,13 +314,13 @@ func TestVersionCheckMessage(t *testing.T) { { name: "Major mismatch - client older", serverVersion: "2.0.0", - clientVersion: "1.0.0", + clientVersion: testVersion100, expectError: true, errorContains: "Client is older; upgrade the bd CLI", }, { name: "Minor mismatch - daemon older", - serverVersion: "1.0.0", + serverVersion: testVersion100, clientVersion: "1.1.0", expectError: true, errorContains: "daemon 1.0.0 is older than client 1.1.0", @@ -326,7 +328,7 @@ func TestVersionCheckMessage(t *testing.T) { { name: "Compatible versions", serverVersion: "1.1.0", - clientVersion: "1.0.0", + clientVersion: testVersion100, expectError: false, }, } @@ -366,7 +368,7 @@ func TestPingAndHealthBypassVersionCheck(t *testing.T) { defer store.Close() // Set incompatible versions - ServerVersion = "1.0.0" + ServerVersion = testVersion100 ClientVersion = "2.0.0" server := NewServer(socketPath, store) @@ -440,8 +442,8 @@ func TestMetricsOperation(t *testing.T) { } defer store.Close() - ServerVersion = "1.0.0" - ClientVersion = "1.0.0" + ServerVersion = testVersion100 + ClientVersion = testVersion100 server := NewServer(socketPath, store) diff --git a/internal/storage/sqlite/collision_test.go b/internal/storage/sqlite/collision_test.go index aa08f588..6f8e8462 100644 --- a/internal/storage/sqlite/collision_test.go +++ b/internal/storage/sqlite/collision_test.go @@ -10,6 +10,8 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const testIssueBD1 = "bd-1" + func TestDetectCollisions(t *testing.T) { // Create temporary database tmpDir, err := os.MkdirTemp("", "collision-test-*") @@ -29,7 +31,7 @@ func TestDetectCollisions(t *testing.T) { // Setup: Create some existing issues in the database existingIssue1 := &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Existing issue 1", Description: "This is an existing issue", Status: types.StatusOpen, @@ -66,7 +68,7 @@ func TestDetectCollisions(t *testing.T) { name: "exact match - idempotent import", incomingIssues: []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Existing issue 1", Description: "This is an existing issue", Status: types.StatusOpen, @@ -98,7 +100,7 @@ func TestDetectCollisions(t *testing.T) { name: "collision - same ID, different title", incomingIssues: []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Modified title", Description: "This is an existing issue", Status: types.StatusOpen, @@ -113,7 +115,7 @@ func TestDetectCollisions(t *testing.T) { if len(collisions) != 1 { t.Fatalf("expected 1 collision, got %d", len(collisions)) } - if collisions[0].ID != "bd-1" { + if collisions[0].ID != testIssueBD1 { t.Errorf("expected collision ID bd-1, got %s", collisions[0].ID) } if len(collisions[0].ConflictingFields) != 1 { @@ -167,7 +169,7 @@ func TestDetectCollisions(t *testing.T) { incomingIssues: []*types.Issue{ { // Exact match - ID: "bd-1", + ID: testIssueBD1, Title: "Existing issue 1", Description: "This is an existing issue", Status: types.StatusOpen, @@ -201,7 +203,7 @@ func TestDetectCollisions(t *testing.T) { name: "collision - estimated_minutes differs", incomingIssues: []*types.Issue{ { - ID: "bd-1", + ID: testIssueBD1, Title: "Existing issue 1", Description: "This is an existing issue", Status: types.StatusOpen, @@ -258,7 +260,7 @@ func TestCompareIssues(t *testing.T) { { name: "identical issues", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test desc", Status: types.StatusOpen, @@ -266,7 +268,7 @@ func TestCompareIssues(t *testing.T) { IssueType: types.TypeTask, }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test desc", Status: types.StatusOpen, @@ -278,7 +280,7 @@ func TestCompareIssues(t *testing.T) { { name: "different title", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Original", Description: "Test", Status: types.StatusOpen, @@ -286,7 +288,7 @@ func TestCompareIssues(t *testing.T) { IssueType: types.TypeTask, }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Modified", Description: "Test", Status: types.StatusOpen, @@ -298,7 +300,7 @@ func TestCompareIssues(t *testing.T) { { name: "different status and priority", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -306,7 +308,7 @@ func TestCompareIssues(t *testing.T) { IssueType: types.TypeTask, }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusClosed, @@ -318,7 +320,7 @@ func TestCompareIssues(t *testing.T) { { name: "estimated_minutes - both nil", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -327,7 +329,7 @@ func TestCompareIssues(t *testing.T) { EstimatedMinutes: nil, }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -340,7 +342,7 @@ func TestCompareIssues(t *testing.T) { { name: "estimated_minutes - existing nil, incoming set", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -349,7 +351,7 @@ func TestCompareIssues(t *testing.T) { EstimatedMinutes: nil, }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -362,7 +364,7 @@ func TestCompareIssues(t *testing.T) { { name: "estimated_minutes - same values", existing: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -371,7 +373,7 @@ func TestCompareIssues(t *testing.T) { EstimatedMinutes: intPtr(60), }, incoming: &types.Issue{ - ID: "bd-1", + ID: testIssueBD1, Title: "Test", Description: "Test", Status: types.StatusOpen, @@ -710,7 +712,7 @@ func TestCountReferencesWordBoundary(t *testing.T) { // Adjust expected based on actual counting logic // countReferences skips the issue itself expected := tt.expectedCount - if tt.issueID == "bd-1" { + if tt.issueID == testIssueBD1 { expected = 1 // only bd-10's description } diff --git a/internal/storage/sqlite/events_test.go b/internal/storage/sqlite/events_test.go index 321d5610..08f7bb99 100644 --- a/internal/storage/sqlite/events_test.go +++ b/internal/storage/sqlite/events_test.go @@ -7,6 +7,8 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const testUserAlice = "alice" + func TestAddComment(t *testing.T) { store, cleanup := setupTestDB(t) defer cleanup() @@ -26,7 +28,7 @@ func TestAddComment(t *testing.T) { } // Add a comment - err = store.AddComment(ctx, issue.ID, "alice", "This is a test comment") + err = store.AddComment(ctx, issue.ID, testUserAlice, "This is a test comment") if err != nil { t.Fatalf("AddComment failed: %v", err) } @@ -55,7 +57,7 @@ func TestAddComment(t *testing.T) { t.Fatal("Comment event not found") } - if commentEvent.Actor != "alice" { + if commentEvent.Actor != testUserAlice { t.Errorf("Expected actor 'alice', got '%s'", commentEvent.Actor) } diff --git a/internal/storage/sqlite/lazy_init_test.go b/internal/storage/sqlite/lazy_init_test.go index 110d8d5d..3390565f 100644 --- a/internal/storage/sqlite/lazy_init_test.go +++ b/internal/storage/sqlite/lazy_init_test.go @@ -9,6 +9,8 @@ import ( "github.com/steveyegge/beads/internal/types" ) +const testIssueCustom1 = "custom-1" + // TestLazyCounterInitialization verifies that counters are initialized lazily // on first use, not by scanning the entire database on every CreateIssue func TestLazyCounterInitialization(t *testing.T) { @@ -163,7 +165,7 @@ func TestLazyCounterInitializationMultiplePrefix(t *testing.T) { t.Fatalf("CreateIssue failed: %v", err) } - if customIssue.ID != "custom-1" { + if customIssue.ID != testIssueCustom1 { t.Errorf("Expected custom-1, got %s", customIssue.ID) }