From 06e029660fc991a5a34dd9600ad1547ef4488afe Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sat, 8 Nov 2025 15:38:47 +0000 Subject: [PATCH 1/5] test: Add unit tests for UI Model Update logic Introduce comprehensive unit tests for the main Bubbletea model (`internal/ui/Model`), ensuring correct state transitions and command generation upon receiving various messages. Includes mock implementations for: * Git client interaction * LLM provider API calls * Sub-models (`CommitView`, `PromptView`) The dependency `github.com/stretchr/objx` was added to support the test utilities. --- go.mod | 1 + go.sum | 2 + internal/ui/model_test.go | 322 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 325 insertions(+) create mode 100644 internal/ui/model_test.go diff --git a/go.mod b/go.mod index 16afa49..bc6845a 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/spf13/pflag v1.0.10 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.2.0 // indirect github.com/tidwall/pretty v1.2.1 // indirect diff --git a/go.sum b/go.sum index 0afbf4f..0b818dc 100644 --- a/go.sum +++ b/go.sum @@ -146,6 +146,8 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go new file mode 100644 index 0000000..2cf2c45 --- /dev/null +++ b/internal/ui/model_test.go @@ -0,0 +1,322 @@ +package ui + +import ( + "context" + "testing" + + "github.com/charmbracelet/bubbles/spinner" + "github.com/charmbracelet/bubbles/textarea" + tea "github.com/charmbracelet/bubbletea" + "github.com/cockroachdb/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/rm-hull/git-commit-summary/internal/interfaces" + llmprovider "github.com/rm-hull/git-commit-summary/internal/llm_provider" +) + +// MockLLMProvider is a mock implementation of llmprovider.Provider +type MockLLMProvider struct { + mock.Mock +} + +func (m *MockLLMProvider) Call(ctx context.Context, model string, prompt string) (string, error) { + args := m.Called(ctx, model, prompt) + return args.String(0), args.Error(1) +} + +func (m *MockLLMProvider) Model() string { + args := m.Called() + return args.String(0) +} + +// MockGitClient is a mock implementation of interfaces.GitClient +type MockGitClient struct { + mock.Mock +} + +func (m *MockGitClient) IsInWorkTree() error { + args := m.Called() + return args.Error(0) +} + +func (m *MockGitClient) StagedFiles() ([]string, error) { + args := m.Called() + return args.Get(0).([]string), args.Error(1) +} + +func (m *MockGitClient) Diff() (string, error) { + args := m.Called() + return args.String(0), args.Error(1) +} + +func (m *MockGitClient) Commit(message string) error { + args := m.Called(message) + return args.Error(0) +} + +func TestModel_Update(t *testing.T) { + ctx := context.Background() + mockLLM := new(MockLLMProvider) + mockGit := new(MockGitClient) + + // Common setup for InitialModel + initialModel := func() *Model { + // Explicitly use the types to avoid "imported and not used" warnings + var _ interfaces.GitClient = mockGit + var _ llmprovider.Provider = mockLLM + return InitialModel(ctx, mockLLM, mockGit, "system prompt", "user message") + } + + t.Run("tea.KeyMsg - CtrlC in showSpinner state", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + + updatedModel, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + + assert.Equal(t, Abort, updatedModel.(*Model).action) + assert.NotNil(t, cmd) + assert.IsType(t, tea.QuitMsg{}, cmd()) + }) + + t.Run("tea.KeyMsg - CtrlC in other states", func(t *testing.T) { + m := initialModel() + m.state = showCommitView // Set to a state other than showSpinner + + // Mock the sub-model's Update method + mockCommitView := new(mockTeaModel) + mockCommitView.On("Update", mock.Anything).Return(mockCommitView, (tea.Cmd)(nil)) + m.commitView = mockCommitView + + updatedModel, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + + assert.Equal(t, None, updatedModel.(*Model).action) // Action should not be Abort + assert.Nil(t, cmd) // No tea.Quit command + mockCommitView.AssertCalled(t, "Update", tea.KeyMsg{Type: tea.KeyCtrlC}) + }) + + t.Run("gitCheckMsg - empty (no staged changes)", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + + updatedModel, cmd := m.Update(gitCheckMsg{}) + + assert.NotNil(t, updatedModel.(*Model).err) + assert.NotNil(t, cmd) + assert.IsType(t, tea.QuitMsg{}, cmd()) + }) + + t.Run("gitCheckMsg - non-empty (staged changes)", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + + mockGit.On("Diff").Return("mocked diff content", nil).Once() + + updatedModel, cmd := m.Update(gitCheckMsg{"file1.go", "file2.go"}) + + assert.Nil(t, updatedModel.(*Model).err) + assert.NotNil(t, cmd) + msg := cmd() + assert.IsType(t, gitDiffMsg(""), msg) + assert.Equal(t, gitDiffMsg("mocked diff content"), msg) + mockGit.AssertExpectations(t) + }) + + t.Run("gitDiffMsg", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + mockLLM.On("Model").Return("test-model").Once() + // The command returned by Update will execute llmProvider.Call later. + // No need to set mockLLM.On("Call") here. + + diffContent := "diff --git a/file.go b/file.go" + updatedModel, cmd := m.Update(gitDiffMsg(diffContent)) + + assert.Equal(t, diffContent, updatedModel.(*Model).diff) + assert.Contains(t, updatedModel.(*Model).spinnerMessage, "Generating commit summary (using: test-model)") + assert.IsType(t, tea.Batch(nil), cmd) + mockLLM.AssertExpectations(t) + }) + + t.Run("llmResultMsg - with user message", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + llmResult := "This is a summary from LLM." + userMsg := "Additional user message." + + m.userMessage = userMsg // Set user message for this test case + + mockCommitView := new(mockTeaModel) + // Only expect Init to be called by Update. View is called by Model.View() + mockCommitView.On("Init").Return(textarea.Blink).Once() + m.commitView = mockCommitView + + updatedModel, cmd := m.Update(llmResultMsg(llmResult)) + + assert.Equal(t, showCommitView, updatedModel.(*Model).state) + // Assert that the commitView is set, but not its content directly from Update + assert.NotNil(t, updatedModel.(*Model).commitView) + assert.IsType(t, (tea.Cmd)(nil), cmd) + // mockCommitView.AssertExpectations(t) + }) + + t.Run("llmResultMsg - without user message", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure initial state is showSpinner + llmResult := "This is a summary from LLM." + m.userMessage = "" // Ensure no user message + + mockCommitView := new(mockTeaModel) + // Only expect Init to be called by Update. View is called by Model.View() + mockCommitView.On("Init").Return(textarea.Blink).Once() + m.commitView = mockCommitView + + updatedModel, cmd := m.Update(llmResultMsg(llmResult)) + + assert.Equal(t, showCommitView, updatedModel.(*Model).state) + // Assert that the commitView is set, but not its content directly from Update + assert.NotNil(t, updatedModel.(*Model).commitView) + assert.NotNil(t, cmd) + // mockCommitView.AssertExpectations(t) + }) + + t.Run("commitMsg", func(t *testing.T) { + m := initialModel() + m.state = showCommitView // Ensure state is showCommitView + + commitContent := "feat: new feature" + updatedModel, cmd := m.Update(commitMsg(commitContent)) + + assert.Equal(t, Commit, updatedModel.(*Model).action) + assert.Equal(t, commitContent, updatedModel.(*Model).commitMessage) + assert.NotNil(t, cmd) + assert.IsType(t, tea.QuitMsg{}, cmd()) + }) + + t.Run("regenerateMsg", func(t *testing.T) { + m := initialModel() + m.state = showCommitView // Ensure state is showCommitView + + updatedModel, cmd := m.Update(regenerateMsg{}) + + assert.Equal(t, showRegeneratePrompt, updatedModel.(*Model).state) + assert.NotNil(t, updatedModel.(*Model).promptView) // promptView should be initialized + assert.IsType(t, (tea.Cmd)(nil), cmd) // Should return m.promptView.Init() + }) + + t.Run("userResponseMsg", func(t *testing.T) { + m := initialModel() + m.state = showRegeneratePrompt // Ensure state is showRegeneratePrompt + mockLLM.On("Model").Return("test-model").Once() + // The command returned by Update will execute llmProvider.Call later. + // No need to set mockLLM.On("Call") here. + + userResponse := "make it shorter" + updatedModel, cmd := m.Update(userResponseMsg(userResponse)) + + assert.Equal(t, showSpinner, updatedModel.(*Model).state) + assert.Contains(t, updatedModel.(*Model).spinnerMessage, "Re-generating commit summary (using: test-model)") + assert.IsType(t, tea.Batch(nil), cmd) // Should return tea.Batch(m.spinner.Tick, m.generateSummary) + mockLLM.AssertExpectations(t) + }) + + t.Run("cancelRegenPromptMsg", func(t *testing.T) { + m := initialModel() + m.state = showRegeneratePrompt // Ensure state is showRegeneratePrompt + + // Mock the sub-model's Init method + mockCommitView := new(mockTeaModel) + mockCommitView.On("Init").Return((tea.Cmd)(nil)).Once() + m.commitView = mockCommitView + + updatedModel, cmd := m.Update(cancelRegenPromptMsg{}) + + assert.Equal(t, showCommitView, updatedModel.(*Model).state) + assert.Nil(t, cmd) // Should return m.commitView.Init() which is mocked to return nil + mockCommitView.AssertExpectations(t) + }) + + t.Run("errMsg", func(t *testing.T) { + m := initialModel() + m.state = showSpinner // Ensure state is showSpinner + + testErr := errors.New("something went wrong") + updatedModel, cmd := m.Update(errMsg{err: testErr}) + + assert.Equal(t, testErr, updatedModel.(*Model).err) + assert.NotNil(t, cmd) + assert.IsType(t, tea.QuitMsg{}, cmd()) + }) + + t.Run("abortMsg", func(t *testing.T) { + m := initialModel() + m.state = showCommitView // Ensure state is showCommitView + + updatedModel, cmd := m.Update(abortMsg{}) + + assert.Equal(t, Abort, updatedModel.(*Model).action) + assert.NotNil(t, cmd) + assert.IsType(t, tea.QuitMsg{}, cmd()) + }) + + t.Run("spinner.Update for showSpinner state", func(t *testing.T) { + m := initialModel() + m.state = showSpinner + // Spinner's Update method is tested by charmbracelet/bubbles, + // here we just ensure it's called and returns its cmd. + // We can't easily mock spinner.Model directly, so we'll check the cmd. + updatedModel, cmd := m.Update(spinner.TickMsg{}) + assert.NotNil(t, updatedModel) + assert.IsType(t, spinner.TickMsg{}, cmd()) + }) + + t.Run("commitView.Update for showCommitView state", func(t *testing.T) { + m := initialModel() + m.state = showCommitView + mockCommitView := new(mockTeaModel) + mockCommitView.On("Update", mock.Anything).Return(mockCommitView, (tea.Cmd)(nil)).Once() + m.commitView = mockCommitView + + testMsg := tea.KeyMsg{Type: tea.KeyEnter} + updatedModel, cmd := m.Update(testMsg) + + assert.NotNil(t, updatedModel) + assert.Nil(t, cmd) // Mock returns nil cmd + mockCommitView.AssertCalled(t, "Update", testMsg) + }) + + t.Run("promptView.Update for showRegeneratePrompt state", func(t *testing.T) { + m := initialModel() + m.state = showRegeneratePrompt + mockPromptView := new(mockTeaModel) + mockPromptView.On("Update", mock.Anything).Return(mockPromptView, (tea.Cmd)(nil)).Once() + m.promptView = mockPromptView + + testMsg := tea.KeyMsg{Type: tea.KeyEnter} + updatedModel, cmd := m.Update(testMsg) + + assert.NotNil(t, updatedModel) + assert.Nil(t, cmd) // Mock returns nil cmd + mockPromptView.AssertCalled(t, "Update", testMsg) + }) +} + +// mockTeaModel is a generic mock for tea.Model interface +type mockTeaModel struct { + mock.Mock +} + +func (m *mockTeaModel) Init() tea.Cmd { + args := m.Called() + return args.Get(0).(tea.Cmd) +} + +func (m *mockTeaModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + args := m.Called(msg) + return args.Get(0).(tea.Model), args.Get(1).(tea.Cmd) +} + +func (m *mockTeaModel) View() string { + args := m.Called() + return args.String(0) +} From 07f933fac1d3a6e9ee6f12ce5f497dbb4ac818fe Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 9 Nov 2025 14:30:21 +0000 Subject: [PATCH 2/5] test: Check textinput Blink command in model Updates the assertion in the `TestModel_Update` for the `regenerateMsg` handler to verify that the command returned is the expected `textinput.Blink()` initialization command. --- internal/ui/model_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 2cf2c45..7ea43fc 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -6,6 +6,7 @@ import ( "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/textarea" + "github.com/charmbracelet/bubbles/textinput" tea "github.com/charmbracelet/bubbletea" "github.com/cockroachdb/errors" "github.com/stretchr/testify/assert" @@ -200,8 +201,9 @@ func TestModel_Update(t *testing.T) { updatedModel, cmd := m.Update(regenerateMsg{}) assert.Equal(t, showRegeneratePrompt, updatedModel.(*Model).state) - assert.NotNil(t, updatedModel.(*Model).promptView) // promptView should be initialized - assert.IsType(t, (tea.Cmd)(nil), cmd) // Should return m.promptView.Init() + assert.NotNil(t, updatedModel.(*Model).promptView) + assert.NotNil(t, cmd) + assert.IsType(t, textinput.Blink(), cmd()) }) t.Run("userResponseMsg", func(t *testing.T) { From 4ac3204ecd4177a0c89472c0e193c01c8f1feb38 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 9 Nov 2025 14:34:07 +0000 Subject: [PATCH 3/5] test: Remove unnecessary commit view mocking The tests for `Model.Update` handling `llmResultMsg` no longer require setting up and asserting calls on a mocked commit view. Instead, we now assert directly that the resulting command is of type `textarea.Blink()`, which confirms the state transition and command generation correctly occurred. This simplifies the test implementation. --- internal/ui/model_test.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 7ea43fc..003bd95 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -147,18 +147,13 @@ func TestModel_Update(t *testing.T) { m.userMessage = userMsg // Set user message for this test case - mockCommitView := new(mockTeaModel) - // Only expect Init to be called by Update. View is called by Model.View() - mockCommitView.On("Init").Return(textarea.Blink).Once() - m.commitView = mockCommitView - updatedModel, cmd := m.Update(llmResultMsg(llmResult)) assert.Equal(t, showCommitView, updatedModel.(*Model).state) // Assert that the commitView is set, but not its content directly from Update assert.NotNil(t, updatedModel.(*Model).commitView) - assert.IsType(t, (tea.Cmd)(nil), cmd) - // mockCommitView.AssertExpectations(t) + assert.NotNil(t, cmd) + assert.IsType(t, textarea.Blink(), cmd()) }) t.Run("llmResultMsg - without user message", func(t *testing.T) { @@ -167,18 +162,13 @@ func TestModel_Update(t *testing.T) { llmResult := "This is a summary from LLM." m.userMessage = "" // Ensure no user message - mockCommitView := new(mockTeaModel) - // Only expect Init to be called by Update. View is called by Model.View() - mockCommitView.On("Init").Return(textarea.Blink).Once() - m.commitView = mockCommitView - updatedModel, cmd := m.Update(llmResultMsg(llmResult)) assert.Equal(t, showCommitView, updatedModel.(*Model).state) // Assert that the commitView is set, but not its content directly from Update assert.NotNil(t, updatedModel.(*Model).commitView) assert.NotNil(t, cmd) - // mockCommitView.AssertExpectations(t) + assert.IsType(t, textarea.Blink(), cmd()) }) t.Run("commitMsg", func(t *testing.T) { From 4cd50a0a57b27963ad74d2442f630875ed577df0 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 9 Nov 2025 14:46:31 +0000 Subject: [PATCH 4/5] test: Refine mock expectations in model tests Updates tests within `internal/ui/model_test.go`: * Makes the mocked `commitView` update expectation explicit when testing `Ctrl+C` handling. * Removes redundant assertion on the updated model within the spinner tick message test, focusing solely on validating that a command is returned. --- internal/ui/model_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 003bd95..4db2cf6 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -86,7 +86,7 @@ func TestModel_Update(t *testing.T) { // Mock the sub-model's Update method mockCommitView := new(mockTeaModel) - mockCommitView.On("Update", mock.Anything).Return(mockCommitView, (tea.Cmd)(nil)) + mockCommitView.On("Update", tea.KeyMsg{Type: tea.KeyCtrlC}).Return(mockCommitView, (tea.Cmd)(nil)) m.commitView = mockCommitView updatedModel, cmd := m.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) @@ -257,8 +257,8 @@ func TestModel_Update(t *testing.T) { // Spinner's Update method is tested by charmbracelet/bubbles, // here we just ensure it's called and returns its cmd. // We can't easily mock spinner.Model directly, so we'll check the cmd. - updatedModel, cmd := m.Update(spinner.TickMsg{}) - assert.NotNil(t, updatedModel) + _, cmd := m.Update(spinner.TickMsg{}) + assert.NotNil(t, cmd) assert.IsType(t, spinner.TickMsg{}, cmd()) }) From 75871fcab665a988491c3494fd0a867677625b45 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 9 Nov 2025 14:50:49 +0000 Subject: [PATCH 5/5] test: Handle missing args in mockTeaModel Refactor the `mockTeaModel` methods to safely retrieve return values from the underlying mocking framework (`m.Called`). This ensures the mock does not panic if expected arguments are missing or of the incorrect type, defaulting to `nil` or the mock receiver itself where appropriate. --- internal/ui/model_test.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 4db2cf6..2891853 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -300,15 +300,36 @@ type mockTeaModel struct { func (m *mockTeaModel) Init() tea.Cmd { args := m.Called() - return args.Get(0).(tea.Cmd) + if len(args) > 0 { + if cmd, ok := args.Get(0).(tea.Cmd); ok { + return cmd + } + } + return nil } func (m *mockTeaModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { args := m.Called(msg) - return args.Get(0).(tea.Model), args.Get(1).(tea.Cmd) + var model tea.Model = m + var cmd tea.Cmd + + if len(args) > 0 { + if m, ok := args.Get(0).(tea.Model); ok { + model = m + } + } + if len(args) > 1 { + if c, ok := args.Get(1).(tea.Cmd); ok { + cmd = c + } + } + return model, cmd } func (m *mockTeaModel) View() string { args := m.Called() - return args.String(0) + if len(args) > 0 { + return args.String(0) + } + return "" }