From 6bb9a8a66615aeb1455646e6a431518ac626d7e5 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Thu, 4 Apr 2024 23:11:22 +0100 Subject: [PATCH] BUGIFX-60: Increase unit test coverage --- internal/logger/logger_test.go | 4 +- internal/model/host_test.go | 4 +- internal/ui/component/hostedit/hostedit.go | 6 +- internal/ui/component/hostlist/hostlist.go | 6 +- .../ui/component/hostlist/hostlist_test.go | 3 +- internal/ui/model.go | 6 +- internal/ui/model_test.go | 68 +++++++++++++++++++ internal/ui/ui.go | 12 ++-- internal/utils/ssh/cmd_win_test.go | 2 +- 9 files changed, 87 insertions(+), 24 deletions(-) create mode 100644 internal/ui/model_test.go diff --git a/internal/logger/logger_test.go b/internal/logger/logger_test.go index cf3ab29..1f3a176 100644 --- a/internal/logger/logger_test.go +++ b/internal/logger/logger_test.go @@ -10,7 +10,7 @@ import ( "testing" ) -func Test_LoggerConstructor(t *testing.T) { +func TestLoggerConstructor(t *testing.T) { // Create a temporary directory for testing tmpDir, err := os.MkdirTemp("", "testlog") if err != nil { @@ -66,7 +66,7 @@ func Test_LoggerConstructor(t *testing.T) { } } -func Test_LoggerMethods(t *testing.T) { +func TestLoggerMethods(t *testing.T) { // Create a temporary directory for testing tmpDir, err := os.MkdirTemp("", "testlog") if err != nil { diff --git a/internal/model/host_test.go b/internal/model/host_test.go index c144a14..bfeb128 100644 --- a/internal/model/host_test.go +++ b/internal/model/host_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_NewHost(t *testing.T) { +func TestNewHost(t *testing.T) { // Create a new host using the NewHost function expectedHost := Host{ ID: 1, @@ -28,7 +28,7 @@ func Test_NewHost(t *testing.T) { } } -func Test_CloneHost(t *testing.T) { +func TestCloneHost(t *testing.T) { // Create a host to clone originalHost := Host{ ID: 1, diff --git a/internal/ui/component/hostedit/hostedit.go b/internal/ui/component/hostedit/hostedit.go index 3481704..83ccaef 100644 --- a/internal/ui/component/hostedit/hostedit.go +++ b/internal/ui/component/hostedit/hostedit.go @@ -57,7 +57,7 @@ var ( debounceTime = time.Second * 1 ) -type logger interface { +type iLogger interface { Debug(format string, args ...any) Info(format string, args ...any) } @@ -103,7 +103,7 @@ type editModel struct { inputs []labeledInput isNewHost bool keyMap keyMap - logger logger + logger iLogger ready bool title string viewport viewport.Model @@ -111,7 +111,7 @@ type editModel struct { } // New - returns new edit host form. -func New(ctx context.Context, storage storage.HostStorage, state *state.ApplicationState, log logger) *editModel { +func New(ctx context.Context, storage storage.HostStorage, state *state.ApplicationState, log iLogger) *editModel { initialFocusedInput := inputTitle // If we can't cast host id to int, that means we're adding a new host. Ignore the error diff --git a/internal/ui/component/hostlist/hostlist.go b/internal/ui/component/hostlist/hostlist.go index b945e6a..65dda03 100644 --- a/internal/ui/component/hostlist/hostlist.go +++ b/internal/ui/component/hostlist/hostlist.go @@ -28,7 +28,7 @@ var ( defaultListTitle = "press 'n' to add a new host" ) -type logger interface { +type iLogger interface { Debug(format string, args ...any) Info(format string, args ...any) Error(format string, args ...any) @@ -50,7 +50,7 @@ type listModel struct { repo storage.HostStorage keyMap *keyMap appState *state.ApplicationState - logger logger + logger iLogger mode string } @@ -60,7 +60,7 @@ type listModel struct { // appState - is the application state, usually we want to restore previous state when application restarts, // for instance focus previously selected host. // log - application logger. -func New(_ context.Context, storage storage.HostStorage, appState *state.ApplicationState, log logger) *listModel { +func New(_ context.Context, storage storage.HostStorage, appState *state.ApplicationState, log iLogger) *listModel { delegate := list.NewDefaultDelegate() delegateKeys := newDelegateKeyMap() diff --git a/internal/ui/component/hostlist/hostlist_test.go b/internal/ui/component/hostlist/hostlist_test.go index 5233fdd..de4d957 100644 --- a/internal/ui/component/hostlist/hostlist_test.go +++ b/internal/ui/component/hostlist/hostlist_test.go @@ -39,7 +39,6 @@ func Test_listModel_Change_Selection(t *testing.T) { { "Select next using 'j' key", 2, - tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'j'}}, }, { @@ -288,7 +287,7 @@ func Test_exitRemoveItemMode(t *testing.T) { // Reject the action by pressing 'n' (it can be any key apart from 'y') _, cmd := model.Update(tea.KeyMsg{ - Type: -1, + Type: -1, // Type '-1' should be equal to 'KeyRunes' Runes: []rune{'n'}, }) diff --git a/internal/ui/model.go b/internal/ui/model.go index 2ac716c..dd48055 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -20,7 +20,7 @@ import ( "github.com/grafviktor/goto/internal/utils/ssh" ) -type logger interface { +type iLogger interface { Debug(format string, args ...any) Info(format string, args ...any) Error(format string, args ...any) @@ -32,7 +32,7 @@ func New( ctx context.Context, storage storage.HostStorage, appState *state.ApplicationState, - log logger, + log iLogger, ) mainModel { m := mainModel{ modelHostList: hostlist.New(ctx, storage, appState, log), @@ -51,7 +51,7 @@ type mainModel struct { modelHostList tea.Model modelHostEdit tea.Model appState *state.ApplicationState - logger logger + logger iLogger viewport viewport.Model ready bool } diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go new file mode 100644 index 0000000..f5c7ea5 --- /dev/null +++ b/internal/ui/model_test.go @@ -0,0 +1,68 @@ +package ui + +import ( + "context" + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/grafviktor/goto/internal/mock" + "github.com/grafviktor/goto/internal/state" + "github.com/grafviktor/goto/internal/ui/message" + "github.com/grafviktor/goto/internal/utils/ssh" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNew(t *testing.T) { + model := New(context.TODO(), mock.NewMockStorage(true), MockAppState(), &mock.MockLogger{}) + require.NotNil(t, model) +} + +func TestUpdate_KeyMsg(t *testing.T) { + // Random key test - make sure that the app reacts on Ctrl+C + model := New(context.TODO(), mock.NewMockStorage(true), MockAppState(), &mock.MockLogger{}) + _, cmd := model.Update(tea.KeyMsg{ + Type: tea.KeyCtrlC, + }) + + assert.NotNil(t, model) + require.IsType(t, tea.QuitMsg{}, cmd(), "Wrong message type") +} + +func TestUpdate_TerminalSizePolling(t *testing.T) { + // Ensure that when the model receives TerminalSizePolling it autogenerates 'WindowSizeMsg' + model := New(context.TODO(), mock.NewMockStorage(true), MockAppState(), &mock.MockLogger{}) + assert.Equal(t, 0, model.appState.Width) + assert.Equal(t, 0, model.appState.Height) + + _, cmds := model.Update(message.TerminalSizePolling{ + Width: 10, + Height: 10, + }) + + var dst []tea.Msg + cmdToMessage(cmds, &dst) + + require.Contains(t, dst, tea.WindowSizeMsg{ + Width: 10, + Height: 10, + }) +} + +func MockAppState() *state.ApplicationState { + return &state.ApplicationState{ + HostSSHConfig: &ssh.Config{}, + } +} + +func cmdToMessage(cmd tea.Cmd, messages *[]tea.Msg) { + result := cmd() + + if batchMsg, ok := result.(tea.BatchMsg); ok { + for _, msg := range batchMsg { + cmdToMessage(msg, messages) + } + } else { + *messages = append(*messages, result) + } +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index c0e08b7..6f6259a 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -10,14 +10,10 @@ import ( "github.com/grafviktor/goto/internal/storage" ) -type interfaceLogger interface { - Debug(format string, args ...any) - Info(format string, args ...any) - Error(format string, args ...any) -} - -// Start - starts UI subsystem of the application. -func Start(ctx context.Context, storage storage.HostStorage, appState *state.ApplicationState, logger interfaceLogger) { +// Start - starts UI subsystem of the application. *state.ApplicationState should be substituted +// with interface type which would have getters and setters for appropriate fields, without doing it +// it's hard to use mock objects in unit tests of the child components. Search for 'MockAppState'. +func Start(ctx context.Context, storage storage.HostStorage, appState *state.ApplicationState, logger iLogger) { uiComponent := New(ctx, storage, appState, logger) p := tea.NewProgram(&uiComponent, tea.WithAltScreen()) diff --git a/internal/utils/ssh/cmd_win_test.go b/internal/utils/ssh/cmd_win_test.go index 769fa0d..7d9e9e4 100644 --- a/internal/utils/ssh/cmd_win_test.go +++ b/internal/utils/ssh/cmd_win_test.go @@ -4,7 +4,7 @@ package ssh import "testing" -func Test_BaseCMD(t *testing.T) { +func TestBaseCMD(t *testing.T) { expected := "cmd /c ssh" result := BaseCMD()