Files
beads/cmd/bd/version_tracking_test.go
Steve Yegge 4a9d6e6dd7 fix: Code review fixes for auto-migration (bd-jgxi)
Critical fixes from code review:

1. **Moved auto-migration to correct location**
   - Now runs AFTER daemon check but BEFORE opening database
   - Prevents: database opened twice, conflicts with daemon
   - Was: Running too early, before knowing if daemon exists

2. **Fixed context cancellation issue**
   - Check if rootCtx is canceled before using it
   - Fall back to Background() if canceled
   - Fixes: "context canceled" errors in test suite

3. **Updated function signature**
   - Takes dbPath as parameter (no longer searches for it)
   - Simpler, more explicit, easier to test
   - Caller already has dbPath, no need to re-discover

4. **Enhanced test reliability**
   - Save/restore all global state
   - Add debug logging for troubleshooting
   - Verify preconditions before migration

Changes:
- cmd/bd/main.go: Move autoMigrateOnVersionBump call to correct location
- cmd/bd/version_tracking.go: Fix context handling, update signature
- cmd/bd/version_tracking_test.go: Improve test reliability

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-23 18:28:01 -08:00

471 lines
13 KiB
Go

package main
import (
"context"
"os"
"path/filepath"
"testing"
"github.com/steveyegge/beads/internal/configfile"
"github.com/steveyegge/beads/internal/storage/sqlite"
)
func TestGetVersionsSince(t *testing.T) {
tests := []struct {
name string
sinceVersion string
expectedCount int
description string
}{
{
name: "empty version returns all",
sinceVersion: "",
expectedCount: len(versionChanges),
description: "Should return all versions when sinceVersion is empty",
},
{
name: "version not in changelog",
sinceVersion: "0.1.0",
expectedCount: len(versionChanges),
description: "Should return all versions when sinceVersion not found",
},
{
name: "oldest version in changelog",
sinceVersion: "0.21.0",
expectedCount: 3, // 0.22.0, 0.22.1, 0.23.0
description: "Should return versions newer than oldest",
},
{
name: "middle version returns newer versions",
sinceVersion: "0.22.0",
expectedCount: 2, // 0.22.1 and 0.23.0
description: "Should return versions newer than specified",
},
{
name: "latest version returns empty",
sinceVersion: "0.23.0",
expectedCount: 0,
description: "Should return empty slice when already on latest in changelog",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := getVersionsSince(tt.sinceVersion)
if len(result) != tt.expectedCount {
t.Errorf("getVersionsSince(%q) returned %d versions, want %d: %s",
tt.sinceVersion, len(result), tt.expectedCount, tt.description)
}
})
}
}
func TestGetVersionsSinceOrder(t *testing.T) {
// Test that versions are returned in chronological order (oldest first)
// versionChanges array is newest-first, but getVersionsSince returns oldest-first
result := getVersionsSince("0.21.0")
if len(result) != 3 {
t.Fatalf("Expected 3 versions after 0.21.0, got %d", len(result))
}
// Verify chronological order by checking dates increase
// result should be [0.22.0, 0.22.1, 0.23.0]
for i := 1; i < len(result); i++ {
prev := result[i-1]
curr := result[i]
// Simple date comparison (YYYY-MM-DD format)
if curr.Date < prev.Date {
t.Errorf("Versions not in chronological order: %s (%s) should come before %s (%s)",
prev.Version, prev.Date, curr.Version, curr.Date)
}
}
// Check specific order
expectedVersions := []string{"0.22.0", "0.22.1", "0.23.0"}
for i, expected := range expectedVersions {
if result[i].Version != expected {
t.Errorf("Version at index %d = %s, want %s", i, result[i].Version, expected)
}
}
}
func TestTrackBdVersion_NoBeadsDir(t *testing.T) {
// Save original state
origUpgradeDetected := versionUpgradeDetected
origPreviousVersion := previousVersion
defer func() {
versionUpgradeDetected = origUpgradeDetected
previousVersion = origPreviousVersion
}()
// Change to temp directory with no .beads
tmpDir := t.TempDir()
origWd, _ := os.Getwd()
defer os.Chdir(origWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp dir: %v", err)
}
// trackBdVersion should silently succeed
trackBdVersion()
// Should not detect upgrade when no .beads dir exists
if versionUpgradeDetected {
t.Error("Expected no upgrade detection when .beads directory doesn't exist")
}
}
func TestTrackBdVersion_FirstRun(t *testing.T) {
// Create temp .beads directory
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
t.Fatalf("Failed to create .beads: %v", err)
}
// Change to temp directory
origWd, _ := os.Getwd()
defer os.Chdir(origWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp dir: %v", err)
}
// Save original state
origUpgradeDetected := versionUpgradeDetected
origPreviousVersion := previousVersion
defer func() {
versionUpgradeDetected = origUpgradeDetected
previousVersion = origPreviousVersion
}()
// Reset state
versionUpgradeDetected = false
previousVersion = ""
// trackBdVersion should create metadata.json
trackBdVersion()
// Should not detect upgrade on first run
if versionUpgradeDetected {
t.Error("Expected no upgrade detection on first run")
}
// Should have created metadata.json with current version
cfg, err := configfile.Load(beadsDir)
if err != nil {
t.Fatalf("Failed to load config after tracking: %v", err)
}
if cfg.LastBdVersion != Version {
t.Errorf("LastBdVersion = %q, want %q", cfg.LastBdVersion, Version)
}
}
func TestTrackBdVersion_UpgradeDetection(t *testing.T) {
// Create temp .beads directory
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
t.Fatalf("Failed to create .beads: %v", err)
}
// Change to temp directory
origWd, _ := os.Getwd()
defer os.Chdir(origWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp dir: %v", err)
}
// Create metadata.json with old version
cfg := configfile.DefaultConfig()
cfg.LastBdVersion = "0.22.0"
if err := cfg.Save(beadsDir); err != nil {
t.Fatalf("Failed to save config: %v", err)
}
// Save original state
origUpgradeDetected := versionUpgradeDetected
origPreviousVersion := previousVersion
defer func() {
versionUpgradeDetected = origUpgradeDetected
previousVersion = origPreviousVersion
}()
// Reset state
versionUpgradeDetected = false
previousVersion = ""
// trackBdVersion should detect upgrade
trackBdVersion()
// Should detect upgrade
if !versionUpgradeDetected {
t.Error("Expected upgrade detection when version changed")
}
if previousVersion != "0.22.0" {
t.Errorf("previousVersion = %q, want %q", previousVersion, "0.22.0")
}
// Should have updated metadata.json to current version
cfg, err := configfile.Load(beadsDir)
if err != nil {
t.Fatalf("Failed to load config after tracking: %v", err)
}
if cfg.LastBdVersion != Version {
t.Errorf("LastBdVersion = %q, want %q", cfg.LastBdVersion, Version)
}
}
func TestTrackBdVersion_SameVersion(t *testing.T) {
// Create temp .beads directory
tmpDir := t.TempDir()
beadsDir := filepath.Join(tmpDir, ".beads")
if err := os.MkdirAll(beadsDir, 0755); err != nil {
t.Fatalf("Failed to create .beads: %v", err)
}
// Change to temp directory
origWd, _ := os.Getwd()
defer os.Chdir(origWd)
if err := os.Chdir(tmpDir); err != nil {
t.Fatalf("Failed to change to temp dir: %v", err)
}
// Create metadata.json with current version
cfg := configfile.DefaultConfig()
cfg.LastBdVersion = Version
if err := cfg.Save(beadsDir); err != nil {
t.Fatalf("Failed to save config: %v", err)
}
// Save original state
origUpgradeDetected := versionUpgradeDetected
origPreviousVersion := previousVersion
defer func() {
versionUpgradeDetected = origUpgradeDetected
previousVersion = origPreviousVersion
}()
// Reset state
versionUpgradeDetected = false
previousVersion = ""
// trackBdVersion should not detect upgrade
trackBdVersion()
// Should not detect upgrade
if versionUpgradeDetected {
t.Error("Expected no upgrade detection when version is the same")
}
}
func TestMaybeShowUpgradeNotification(t *testing.T) {
// Save original state
origUpgradeDetected := versionUpgradeDetected
origPreviousVersion := previousVersion
origUpgradeAcknowledged := upgradeAcknowledged
defer func() {
versionUpgradeDetected = origUpgradeDetected
previousVersion = origPreviousVersion
upgradeAcknowledged = origUpgradeAcknowledged
}()
// Test: No upgrade detected - should not modify acknowledged flag
versionUpgradeDetected = false
upgradeAcknowledged = false
previousVersion = ""
maybeShowUpgradeNotification()
if upgradeAcknowledged {
t.Error("Should not set acknowledged flag when no upgrade detected")
}
// Test: Upgrade detected but already acknowledged - should not change state
versionUpgradeDetected = true
upgradeAcknowledged = true
previousVersion = "0.22.0"
maybeShowUpgradeNotification()
if !upgradeAcknowledged {
t.Error("Should keep acknowledged flag when already acknowledged")
}
// Test: Upgrade detected and not acknowledged - should set acknowledged flag
versionUpgradeDetected = true
upgradeAcknowledged = false
previousVersion = "0.22.0"
maybeShowUpgradeNotification()
if !upgradeAcknowledged {
t.Error("Should mark as acknowledged after showing notification")
}
// Calling again should keep acknowledged flag set
prevAck := upgradeAcknowledged
maybeShowUpgradeNotification()
if upgradeAcknowledged != prevAck {
t.Error("Should not change acknowledged state on subsequent calls")
}
}
func TestAutoMigrateOnVersionBump_NoUpgrade(t *testing.T) {
// Save original state
origUpgradeDetected := versionUpgradeDetected
defer func() {
versionUpgradeDetected = origUpgradeDetected
}()
// Reset state - no upgrade detected
versionUpgradeDetected = false
// Should return early without doing anything
autoMigrateOnVersionBump("/tmp/test.db")
// Test passes if no panic occurs
}
func TestAutoMigrateOnVersionBump_NoDatabase(t *testing.T) {
// Create temp directory
tmpDir := t.TempDir()
nonExistentDB := filepath.Join(tmpDir, "nonexistent.db")
// Save original state
origUpgradeDetected := versionUpgradeDetected
defer func() {
versionUpgradeDetected = origUpgradeDetected
}()
// Simulate version upgrade
versionUpgradeDetected = true
// Should handle gracefully when database doesn't exist
autoMigrateOnVersionBump(nonExistentDB)
// Test passes if no panic occurs
}
func TestAutoMigrateOnVersionBump_MigratesVersion(t *testing.T) {
// NOTE: Cannot use t.Parallel() because we modify global variables
// Save original state FIRST - critical to avoid test pollution from previous tests
origUpgradeDetected := versionUpgradeDetected
origUpgradeAcknowledged := upgradeAcknowledged
origPreviousVersion := previousVersion
defer func() {
versionUpgradeDetected = origUpgradeDetected
upgradeAcknowledged = origUpgradeAcknowledged
previousVersion = origPreviousVersion
}()
// Create temp directory with unique name to avoid any possible interference
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test-migrate-version-unique.db")
// Create database with old version
ctx := context.Background()
store, err := sqlite.New(ctx, dbPath)
if err != nil {
t.Fatalf("Failed to create database: %v", err)
}
// Set old database version
oldVersion := "0.22.0"
if err := store.SetMetadata(ctx, "bd_version", oldVersion); err != nil {
t.Fatalf("Failed to set old version: %v", err)
}
if err := store.Close(); err != nil {
t.Fatalf("Failed to close database: %v", err)
}
// Simulate version upgrade (must be set to true for migration to run)
// Set this AFTER closing the database to ensure clean state
upgradeAcknowledged = false
previousVersion = oldVersion
// Verify dbPath before migration
if dbPath == "" {
t.Fatalf("dbPath is empty!")
}
if _, err := os.Stat(dbPath); os.IsNotExist(err) {
t.Fatalf("database doesn't exist before migration: %s", dbPath)
}
// Set versionUpgradeDetected immediately before calling to avoid races
versionUpgradeDetected = true
t.Logf("Calling autoMigrateOnVersionBump with dbPath=%s, versionUpgradeDetected=%v", dbPath, versionUpgradeDetected)
// Immediately call migration while flag is still true
autoMigrateOnVersionBump(dbPath)
// Verify the flag is still true after migration
if !versionUpgradeDetected {
t.Fatalf("version Upgrade detected flag was cleared during migration")
}
// Verify database version was updated
store, err = sqlite.New(ctx, dbPath)
if err != nil {
t.Fatalf("Failed to open database: %v", err)
}
defer store.Close()
newVersion, err := store.GetMetadata(ctx, "bd_version")
if err != nil {
t.Fatalf("Failed to read database version: %v", err)
}
if newVersion != Version {
t.Errorf("Database version not updated: got %q, want %q (versionUpgradeDetected=%v)",
newVersion, Version, versionUpgradeDetected)
}
}
func TestAutoMigrateOnVersionBump_AlreadyMigrated(t *testing.T) {
// Create temp directory
tmpDir := t.TempDir()
dbPath := filepath.Join(tmpDir, "test.db")
// Create database with current version
ctx := context.Background()
store, err := sqlite.New(ctx, dbPath)
if err != nil {
t.Fatalf("Failed to create database: %v", err)
}
// Set current database version
if err := store.SetMetadata(ctx, "bd_version", Version); err != nil {
t.Fatalf("Failed to set version: %v", err)
}
_ = store.Close()
// Save original state
origUpgradeDetected := versionUpgradeDetected
defer func() {
versionUpgradeDetected = origUpgradeDetected
}()
// Simulate version upgrade
versionUpgradeDetected = true
// Call auto-migration - should be a no-op
autoMigrateOnVersionBump(dbPath)
// Verify database version is still current
store, err = sqlite.New(ctx, dbPath)
if err != nil {
t.Fatalf("Failed to open database: %v", err)
}
defer store.Close()
currentVersion, err := store.GetMetadata(ctx, "bd_version")
if err != nil {
t.Fatalf("Failed to read database version: %v", err)
}
if currentVersion != Version {
t.Errorf("Database version changed unexpectedly: got %q, want %q", currentVersion, Version)
}
}