From bedcd8a185e4197d087b2166afa79ff4dce25f7c Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Thu, 22 Jan 2026 20:49:06 -1000 Subject: Fix slice reallocation bug in GetBoards The boardMap was storing pointers to Board structs in the boards slice. When the slice grew and reallocated, those pointers became stale, causing cards to be added to old memory locations instead of the current slice. Fixed by storing indices instead of pointers in boardMap. Added tests to verify multiple boards with varying card counts are correctly saved and retrieved. Co-Authored-By: Claude Opus 4.5 --- internal/store/sqlite_test.go | 163 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) diff --git a/internal/store/sqlite_test.go b/internal/store/sqlite_test.go index 6bf7783..c09f3ca 100644 --- a/internal/store/sqlite_test.go +++ b/internal/store/sqlite_test.go @@ -267,3 +267,166 @@ func TestDeleteCard_NonExistent(t *testing.T) { t.Errorf("DeleteCard on non-existent card should not error, got: %v", err) } } + +// TestSaveAndGetBoards_MultipleBoards verifies that all boards and cards are +// correctly saved and retrieved. This tests the fix for slice reallocation bug +// where pointers in boardMap became stale when the boards slice grew. +func TestSaveAndGetBoards_MultipleBoards(t *testing.T) { + store := setupTestStoreWithCards(t) + defer store.Close() + + // Create multiple boards with varying numbers of cards + // Use enough boards to trigger slice reallocation + boards := []models.Board{ + { + ID: "board1", + Name: "Board 1", + Cards: []models.Card{ + {ID: "card1a", Name: "Card 1A", ListID: "list1", ListName: "To Do"}, + {ID: "card1b", Name: "Card 1B", ListID: "list1", ListName: "To Do"}, + }, + }, + { + ID: "board2", + Name: "Board 2", + Cards: []models.Card{ + {ID: "card2a", Name: "Card 2A", ListID: "list2", ListName: "Doing"}, + {ID: "card2b", Name: "Card 2B", ListID: "list2", ListName: "Doing"}, + {ID: "card2c", Name: "Card 2C", ListID: "list2", ListName: "Doing"}, + }, + }, + { + ID: "board3", + Name: "Board 3", + Cards: []models.Card{ + {ID: "card3a", Name: "Card 3A", ListID: "list3", ListName: "Done"}, + }, + }, + { + ID: "board4", + Name: "Board 4", + Cards: []models.Card{ + {ID: "card4a", Name: "Card 4A", ListID: "list4", ListName: "Backlog"}, + {ID: "card4b", Name: "Card 4B", ListID: "list4", ListName: "Backlog"}, + }, + }, + { + ID: "board5", + Name: "Board 5", + Cards: []models.Card{ + {ID: "card5a", Name: "Card 5A", ListID: "list5", ListName: "Ideas"}, + {ID: "card5b", Name: "Card 5B", ListID: "list5", ListName: "Ideas"}, + {ID: "card5c", Name: "Card 5C", ListID: "list5", ListName: "Ideas"}, + {ID: "card5d", Name: "Card 5D", ListID: "list5", ListName: "Ideas"}, + }, + }, + } + + // Calculate expected totals + expectedBoards := len(boards) + expectedCards := 0 + for _, b := range boards { + expectedCards += len(b.Cards) + } + + // Save boards + if err := store.SaveBoards(boards); err != nil { + t.Fatalf("SaveBoards failed: %v", err) + } + + // Retrieve boards + result, err := store.GetBoards() + if err != nil { + t.Fatalf("GetBoards failed: %v", err) + } + + // Verify board count + if len(result) != expectedBoards { + t.Errorf("Expected %d boards, got %d", expectedBoards, len(result)) + } + + // Verify total card count + totalCards := 0 + for _, b := range result { + totalCards += len(b.Cards) + } + if totalCards != expectedCards { + t.Errorf("Expected %d total cards, got %d", expectedCards, totalCards) + } + + // Verify each board has correct card count + expectedCardCounts := map[string]int{ + "board1": 2, + "board2": 3, + "board3": 1, + "board4": 2, + "board5": 4, + } + for _, b := range result { + expected, ok := expectedCardCounts[b.ID] + if !ok { + t.Errorf("Unexpected board ID: %s", b.ID) + continue + } + if len(b.Cards) != expected { + t.Errorf("Board %s: expected %d cards, got %d", b.ID, expected, len(b.Cards)) + } + } +} + +// TestSaveAndGetBoards_ManyBoards verifies correct behavior with many boards +// to ensure slice reallocation is thoroughly tested +func TestSaveAndGetBoards_ManyBoards(t *testing.T) { + store := setupTestStoreWithCards(t) + defer store.Close() + + // Create 20 boards with 5 cards each = 100 cards total + numBoards := 20 + cardsPerBoard := 5 + boards := make([]models.Board, numBoards) + + for i := 0; i < numBoards; i++ { + boards[i] = models.Board{ + ID: string(rune('A' + i)), + Name: "Board " + string(rune('A'+i)), + Cards: make([]models.Card, cardsPerBoard), + } + for j := 0; j < cardsPerBoard; j++ { + boards[i].Cards[j] = models.Card{ + ID: string(rune('A'+i)) + "_card" + string(rune('0'+j)), + Name: "Card " + string(rune('0'+j)), + ListID: "list1", + ListName: "To Do", + } + } + } + + // Save boards + if err := store.SaveBoards(boards); err != nil { + t.Fatalf("SaveBoards failed: %v", err) + } + + // Retrieve boards + result, err := store.GetBoards() + if err != nil { + t.Fatalf("GetBoards failed: %v", err) + } + + // Verify counts + if len(result) != numBoards { + t.Errorf("Expected %d boards, got %d", numBoards, len(result)) + } + + totalCards := 0 + for _, b := range result { + totalCards += len(b.Cards) + if len(b.Cards) != cardsPerBoard { + t.Errorf("Board %s: expected %d cards, got %d", b.ID, cardsPerBoard, len(b.Cards)) + } + } + + expectedTotal := numBoards * cardsPerBoard + if totalCards != expectedTotal { + t.Errorf("Expected %d total cards, got %d", expectedTotal, totalCards) + } +} -- cgit v1.2.3