improve test performance

Author: Thomas Stromberg

Makefile

@@ -16,7 +16,7 @@ build-registrar:
 
 # Run tests with race detection and coverage
 test:
-	go test -v -race -cover ./...
+	go test -race -cover ./...
 
 # Format code
 fmt:

pkg/bot/coordinator_test_helpers.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sync"
 	"time"
 
 	ghmailto "github.com/codeGROOVE-dev/gh-mailto/pkg/gh-mailto"
@@ -15,6 +16,7 @@ import (
 
 // mockStateStore implements StateStore interface from bot package.
 type mockStateStore struct {
+	mu                sync.Mutex
 	threads           map[string]ThreadInfo
 	dmTimes           map[string]time.Time
 	dmUsers           map[string][]string
@@ -25,6 +27,8 @@ type mockStateStore struct {
 }
 
 func (m *mockStateStore) Thread(owner, repo string, number int, channelID string) (ThreadInfo, bool) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	key := fmt.Sprintf("%s/%s#%d:%s", owner, repo, number, channelID)
 	if m.threads != nil {
 		if info, ok := m.threads[key]; ok {
@@ -35,6 +39,8 @@ func (m *mockStateStore) Thread(owner, repo string, number int, channelID string
 }
 
 func (m *mockStateStore) SaveThread(owner, repo string, number int, channelID string, info ThreadInfo) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.saveThreadErr != nil {
 		return m.saveThreadErr
 	}
@@ -47,6 +53,8 @@ func (m *mockStateStore) SaveThread(owner, repo string, number int, channelID st
 }
 
 func (m *mockStateStore) LastDM(userID, prURL string) (time.Time, bool) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	key := userID + ":" + prURL
 	if m.dmTimes != nil {
 		if t, ok := m.dmTimes[key]; ok {
@@ -57,6 +65,8 @@ func (m *mockStateStore) LastDM(userID, prURL string) (time.Time, bool) {
 }
 
 func (m *mockStateStore) RecordDM(userID, prURL string, sentAt time.Time) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	key := userID + ":" + prURL
 	if m.dmTimes == nil {
 		m.dmTimes = make(map[string]time.Time)
@@ -66,6 +76,8 @@ func (m *mockStateStore) RecordDM(userID, prURL string, sentAt time.Time) error
 }
 
 func (m *mockStateStore) ListDMUsers(prURL string) []string {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.dmUsers != nil {
 		if users, ok := m.dmUsers[prURL]; ok {
 			return users
@@ -75,13 +87,17 @@ func (m *mockStateStore) ListDMUsers(prURL string) []string {
 }
 
 func (m *mockStateStore) WasProcessed(eventKey string) bool {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.processedEvents != nil {
 		return m.processedEvents[eventKey]
 	}
 	return false
 }
 
 func (m *mockStateStore) MarkProcessed(eventKey string, _ time.Duration) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.markProcessedErr != nil {
 		return m.markProcessedErr
 	}
@@ -93,6 +109,8 @@ func (m *mockStateStore) MarkProcessed(eventKey string, _ time.Duration) error {
 }
 
 func (m *mockStateStore) LastNotification(prURL string) time.Time {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.lastNotifications != nil {
 		if t, ok := m.lastNotifications[prURL]; ok {
 			return t
@@ -102,6 +120,8 @@ func (m *mockStateStore) LastNotification(prURL string) time.Time {
 }
 
 func (m *mockStateStore) RecordNotification(prURL string, notifiedAt time.Time) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.lastNotifications == nil {
 		m.lastNotifications = make(map[string]time.Time)
 	}
@@ -130,6 +150,7 @@ func (*mockStateStore) Close() error {
 //
 //nolint:govet // fieldalignment optimization would reduce test readability
 type mockSlackClient struct {
+	mu                  sync.Mutex
 	postThreadFunc      func(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error)
 	updateMessageFunc   func(ctx context.Context, channelID, timestamp, text string) error
 	updateDMMessageFunc func(ctx context.Context, userID, timestamp, text string) error
@@ -170,35 +191,41 @@ type mockUpdatedDMMessage struct {
 }
 
 func (m *mockSlackClient) PostThread(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error) {
+	m.mu.Lock()
 	m.postedMessages = append(m.postedMessages, mockPostedMessage{
 		ChannelID:   channelID,
 		Text:        text,
 		Attachments: attachments,
 	})
+	m.mu.Unlock()
 	if m.postThreadFunc != nil {
 		return m.postThreadFunc(ctx, channelID, text, attachments)
 	}
 	return "1234567890.123456", nil
 }
 
 func (m *mockSlackClient) UpdateMessage(ctx context.Context, channelID, timestamp, text string) error {
+	m.mu.Lock()
 	m.updatedMessages = append(m.updatedMessages, mockUpdatedMessage{
 		ChannelID: channelID,
 		Timestamp: timestamp,
 		Text:      text,
 	})
+	m.mu.Unlock()
 	if m.updateMessageFunc != nil {
 		return m.updateMessageFunc(ctx, channelID, timestamp, text)
 	}
 	return nil
 }
 
 func (m *mockSlackClient) UpdateDMMessage(ctx context.Context, userID, prURL, text string) error {
+	m.mu.Lock()
 	m.updatedDMMessage = append(m.updatedDMMessage, mockUpdatedDMMessage{
 		UserID: userID,
 		PRURL:  prURL,
 		Text:   text,
 	})
+	m.mu.Unlock()
 	if m.updateDMMessageFunc != nil {
 		return m.updateDMMessageFunc(ctx, userID, prURL, text)
 	}
@@ -363,6 +390,7 @@ func (m *mockUserMapper) FormatUserMentions(ctx context.Context, githubUsers []s
 
 // mockTracker is a simple mock for notification tracking in tests.
 type mockTracker struct {
+	mu              sync.Mutex
 	channelNotified bool
 	userTags        []mockUserTag
 	tagInfoByUser   map[string]TagInfo // Map from slackUserID to TagInfo for testing
@@ -378,10 +406,14 @@ type mockUserTag struct {
 }
 
 func (m *mockTracker) UpdateChannelNotification(workspaceID, owner, repo string, prNumber int) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.channelNotified = true
 }
 
 func (m *mockTracker) UpdateUserPRChannelTag(workspaceID, slackUserID, channelID, owner, repo string, prNumber int) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	m.userTags = append(m.userTags, mockUserTag{
 		workspaceID: workspaceID,
 		slackUserID: slackUserID,
@@ -393,6 +425,8 @@ func (m *mockTracker) UpdateUserPRChannelTag(workspaceID, slackUserID, channelID
 }
 
 func (m *mockTracker) LastUserPRChannelTag(workspaceID, slackUserID, owner, repo string, prNumber int) TagInfo {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.tagInfoByUser != nil {
 		if tagInfo, ok := m.tagInfoByUser[slackUserID]; ok {
 			return tagInfo
@@ -403,6 +437,7 @@ func (m *mockTracker) LastUserPRChannelTag(workspaceID, slackUserID, owner, repo
 
 // mockNotifier is a simple mock for notification manager in tests.
 type mockNotifier struct {
+	mu              sync.Mutex
 	Tracker         *mockTracker
 	notifyUserError error
 	notifyCalls     []notifyUserCall
@@ -417,12 +452,14 @@ type notifyUserCall struct {
 
 // NotifyUser mocks the notify.Manager.NotifyUser method.
 func (m *mockNotifier) NotifyUser(ctx context.Context, workspaceID, userID, channelID, channelName string, pr interface{}) error {
+	m.mu.Lock()
 	m.notifyCalls = append(m.notifyCalls, notifyUserCall{
 		workspaceID: workspaceID,
 		userID:      userID,
 		channelID:   channelID,
 		channelName: channelName,
 	})
+	m.mu.Unlock()
 	return m.notifyUserError
 }
 

pkg/slack/additional_functions_test.go

@@ -10,6 +10,8 @@ import (
 
 // TestPostThreadReply tests posting a reply to a thread.
 func TestPostThreadReply(t *testing.T) {
+	t.Parallel()
+
 	mockSlack := slacktest.New()
 	defer mockSlack.Close()
 
@@ -46,6 +48,8 @@ func TestPostThreadReply(t *testing.T) {
 
 // TestHasRecentDMAboutPR_NoRecent tests when no recent DM exists.
 func TestHasRecentDMAboutPR_NoRecent(t *testing.T) {
+	t.Parallel()
+
 	mockSlack := slacktest.New()
 	defer mockSlack.Close()
 
@@ -70,6 +74,8 @@ func TestHasRecentDMAboutPR_NoRecent(t *testing.T) {
 
 // TestSaveDMMessageInfo tests saving DM message information.
 func TestSaveDMMessageInfo(t *testing.T) {
+	t.Parallel()
+
 	mockSlack := slacktest.New()
 	defer mockSlack.Close()
 

pkg/slack/api_test.go

@@ -9,6 +9,8 @@ import (
 )
 
 func TestSlackAPIWrapper(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("RawClient", func(t *testing.T) {

pkg/slack/api_wrapper_test.go

@@ -11,6 +11,8 @@ import (
 
 // TestSlackAPIWrapperIntegration tests the actual slackAPIWrapper with a mock HTTP server.
 func TestSlackAPIWrapperIntegration(t *testing.T) {
+	t.Parallel()
+
 	// Create a mock HTTP server that responds to Slack API calls
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Return simple successful responses for all endpoints

pkg/slack/client_additional_test.go

@@ -11,6 +11,8 @@ import (
 )
 
 func TestUpdateDMMessage(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("no_state_store", func(t *testing.T) {
@@ -28,6 +30,8 @@ func TestUpdateDMMessage(t *testing.T) {
 }
 
 func TestSearchMessages(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("success", func(t *testing.T) {
@@ -83,6 +87,8 @@ func TestSearchMessages(t *testing.T) {
 }
 
 func TestAPI(t *testing.T) {
+	t.Parallel()
+
 	t.Run("wrapper_returns_raw_client", func(t *testing.T) {
 		rawClient := slack.New("test-token")
 		wrapper := newSlackAPIWrapper(rawClient)
@@ -112,6 +118,8 @@ func TestAPI(t *testing.T) {
 }
 
 func TestResolveChannelID(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("cached_channel", func(t *testing.T) {
@@ -194,6 +202,8 @@ func TestResolveChannelID(t *testing.T) {
 }
 
 func TestIsUserInChannel(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("user_in_channel", func(t *testing.T) {
@@ -259,6 +269,8 @@ func TestIsUserInChannel(t *testing.T) {
 }
 
 func TestPublishHomeView(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("success", func(t *testing.T) {
@@ -307,6 +319,8 @@ func TestPublishHomeView(t *testing.T) {
 }
 
 func TestChannelHistory(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("success", func(t *testing.T) {
@@ -412,6 +426,8 @@ func (m *programmableMockStateStore) SaveDMMessage(userID, prURL string, info st
 }
 
 func TestUpdateDMMessage_Complete(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	prURL := "https://github.com/test/repo/pull/123"
 

pkg/slack/client_coverage_test.go

@@ -12,6 +12,8 @@ import (
 
 // TestPostThreadReply_ErrorCases tests error handling in PostThreadReply.
 func TestPostThreadReply_ErrorCases(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("channel_not_found", func(t *testing.T) {
@@ -123,6 +125,8 @@ func TestPostThreadReply_ErrorCases(t *testing.T) {
 
 // TestHasRecentDMAboutPR_WithStateStore tests HasRecentDMAboutPR with a state store.
 func TestHasRecentDMAboutPR_WithStateStore(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 	prURL := "https://github.com/test/repo/pull/123"
 
@@ -244,6 +248,8 @@ func TestHasRecentDMAboutPR_WithStateStore(t *testing.T) {
 
 // TestSendDirectMessage_Errors tests error handling in SendDirectMessage.
 func TestSendDirectMessage_Errors(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("open_conversation_fails", func(t *testing.T) {
@@ -323,6 +329,8 @@ func TestSendDirectMessage_Errors(t *testing.T) {
 
 // TestSaveDMMessageInfo_WithStore tests SaveDMMessageInfo with a state store.
 func TestSaveDMMessageInfo_WithStore(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("saves_to_store", func(t *testing.T) {
@@ -376,6 +384,8 @@ func TestSaveDMMessageInfo_WithStore(t *testing.T) {
 
 // TestPostThread_Errors tests error handling in PostThread.
 func TestPostThread_Errors(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("channel_not_found_during_check", func(t *testing.T) {
@@ -495,6 +505,8 @@ func TestPostThread_Errors(t *testing.T) {
 
 // TestUpdateMessage_EdgeCases tests edge cases in UpdateMessage.
 func TestUpdateMessage_EdgeCases(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
 
 	t.Run("message_not_found", func(t *testing.T) {