From da12bd719c4900b8c8120cd668625e4f91021a78 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sun, 31 Dec 2023 21:16:03 +0300 Subject: [PATCH] IMPROVEMENT-24: Increase unit test coverage --- internal/ui/component/edithost/edit_host.go | 12 +- .../ui/component/edithost/edit_host_test.go | 159 ++++++++++++++++++ internal/ui/component/hostlist/list_test.go | 65 +------ 3 files changed, 171 insertions(+), 65 deletions(-) create mode 100644 internal/ui/component/edithost/edit_host_test.go diff --git a/internal/ui/component/edithost/edit_host.go b/internal/ui/component/edithost/edit_host.go index 502567f..3ff7791 100644 --- a/internal/ui/component/edithost/edit_host.go +++ b/internal/ui/component/edithost/edit_host.go @@ -70,7 +70,7 @@ func networkPortValidator(s string) error { auto := 0 // 0 is used to autodetect base, see strconv.ParseUint maxLengthBit := 16 if num, err := strconv.ParseUint(s, auto, maxLengthBit); err != nil || num < 1 { - return fmt.Errorf("network port must be a number which is less than 65 535") + return fmt.Errorf("network port must be a number which is less than 65,535") } return nil @@ -248,9 +248,15 @@ func (m editModel) save(_ tea.Msg) (editModel, tea.Cmd) { } host, _ := m.hostStorage.Save(m.host) - return m, tea.Batch( + // Need to check storage error and update application status: + // if err !=nil { return message.TeaCmd(message.Error{Err: err}) } + // or + // m.title = err + + return m, tea.Sequence( message.TeaCmd(MsgClose{}), - // Order matters here! 'HostListSelectItem' message should be dispatched + // Order matters here! That's why we use tea.Sequence instead of tea.Batch. + // 'HostListSelectItem' message should be dispatched // before 'MsgRepoUpdated'. The reasons of that is because // 'MsgRepoUpdated' handler automatically sets focus on previously selected item. message.TeaCmd(message.HostListSelectItem{HostID: host.ID}), diff --git a/internal/ui/component/edithost/edit_host_test.go b/internal/ui/component/edithost/edit_host_test.go new file mode 100644 index 0000000..cecf6e8 --- /dev/null +++ b/internal/ui/component/edithost/edit_host_test.go @@ -0,0 +1,159 @@ +// Package edithost contains UI components for editing host model attributes. +package edithost + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafviktor/goto/internal/mock" + "github.com/grafviktor/goto/internal/state" +) + +func Test_NotEmptyValidator(t *testing.T) { + tests := []struct { + input string + expected error + }{ + {"", fmt.Errorf("value is required")}, + {"non-empty", nil}, + } + + for _, test := range tests { + result := notEmptyValidator(test.input) + + if (result == nil && test.expected != nil) || (result != nil && test.expected == nil) { + t.Errorf("For input %q, expected error %v but got %v", test.input, test.expected, result) + } + + if result != nil && result.Error() != test.expected.Error() { + t.Errorf("For input %q, expected error %v but got %v", test.input, test.expected, result) + } + } +} + +func Test_NetworkPortValidator(t *testing.T) { + tests := []struct { + input string + expected error + }{ + {"", nil}, + {"abc", fmt.Errorf("network port must be a number which is less than 65,535")}, + {"0", fmt.Errorf("network port must be a number which is less than 65,535")}, + {"65536", fmt.Errorf("network port must be a number which is less than 65,535")}, + {"123", nil}, + } + + for _, test := range tests { + result := networkPortValidator(test.input) + + if (result == nil && test.expected != nil) || (result != nil && test.expected == nil) { + t.Errorf("For input %q, expected error %v but got %v", test.input, test.expected, result) + } + + if result != nil && result.Error() != test.expected.Error() { + t.Errorf("For input %q, expected error %v but got %v", test.input, test.expected, result) + } + } +} + +func Test_GetKeyMap(t *testing.T) { + keyMap := getKeyMap(inputAddress) + require.True(t, keyMap.CopyToTitle.Enabled()) + + keyMap = getKeyMap(inputTitle) + require.False(t, keyMap.CopyToTitle.Enabled()) +} + +func Test_New(t *testing.T) { + state := state.ApplicationState{} + + // If we open host details, but host does not exist, we should focus Address input by default + editHostModel := New(context.TODO(), mock.NewMockStorage(true), &state, &mock.MockLogger{}) + require.Equal(t, inputAddress, editHostModel.focusedInput) + + // If we open host details, but host does not exist, we should focus Title input by default + editHostModel = New(context.TODO(), mock.NewMockStorage(false), &state, &mock.MockLogger{}) + require.Equal(t, inputTitle, editHostModel.focusedInput) +} + +// func (m editModel) save(_ tea.Msg) (editModel, tea.Cmd) { +// for i := range m.inputs { +// if m.inputs[i].Validate != nil { +// if err := m.inputs[i].Validate(m.inputs[i].Value()); err != nil { +// m.inputs[i].Err = err +// m.title = fmt.Sprintf("%s is not valid", m.inputs[i].Label) + +// return m, nil +// } +// } + +// switch i { +// case inputTitle: +// m.host.Title = m.inputs[i].Value() +// case inputAddress: +// m.host.Address = m.inputs[i].Value() +// case inputDescription: +// m.host.Description = m.inputs[i].Value() +// case inputLogin: +// m.host.LoginName = m.inputs[i].Value() +// case inputNetworkPort: +// m.host.RemotePort = m.inputs[i].Value() +// case inputIdentityFile: +// m.host.PrivateKeyPath = m.inputs[i].Value() +// } +// } + +// host, _ := m.hostStorage.Save(m.host) +// return m, tea.Batch( +// message.TeaCmd(MsgClose{}), +// // Order matters here! 'HostListSelectItem' message should be dispatched +// // before 'MsgRepoUpdated'. The reasons of that is because +// // 'MsgRepoUpdated' handler automatically sets focus on previously selected item. +// message.TeaCmd(message.HostListSelectItem{HostID: host.ID}), +// message.TeaCmd(hostlist.MsgRepoUpdated{}), +// ) +// } + +func Test_save(t *testing.T) { + state := state.ApplicationState{} + + editHostModel := New(context.TODO(), mock.NewMockStorage(true), &state, &mock.MockLogger{}) + require.Equal(t, inputAddress, editHostModel.focusedInput) + + editHostModel.inputs[inputDescription].SetValue("test") + editHostModel.inputs[inputLogin].SetValue("root") + editHostModel.inputs[inputNetworkPort].SetValue("2222") + editHostModel.inputs[inputIdentityFile].SetValue("id_rsa") + + // Should fail because mandatory fields are not set + model, messageSequence := editHostModel.save(nil) + + require.Nil(t, messageSequence) + require.Contains(t, model.title, "not valid") + + // model, messageSequence := editHostModel.save(nil) + + editHostModel.inputs[inputTitle].SetValue("test") + editHostModel.inputs[inputAddress].SetValue("localhost") + + model, messageSequence = editHostModel.save(nil) + + require.NotNil(t, messageSequence) + + /* + // Cannot test return values because the function returns an array-like structure of private objects + expectedSequence := tea.Sequence( + message.TeaCmd(MsgClose{}), + message.TeaCmd(message.HostListSelectItem{HostID: model.host.ID}), + message.TeaCmd(hostlist.MsgRepoUpdated{}), + ) + + one := expectedSequence() + two := messageSequence() // returns []tea.sequenceMsg, which is private. Cannot test. + + require.Equal(t, one, two) + */ +} diff --git a/internal/ui/component/hostlist/list_test.go b/internal/ui/component/hostlist/list_test.go index 531de44..4908b2c 100644 --- a/internal/ui/component/hostlist/list_test.go +++ b/internal/ui/component/hostlist/list_test.go @@ -3,7 +3,6 @@ package hostlist import ( "context" - "errors" "os" "testing" @@ -12,7 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafviktor/goto/internal/model" + "github.com/grafviktor/goto/internal/mock" "github.com/grafviktor/goto/internal/utils" ) @@ -308,14 +307,14 @@ func Test_listTitleUpdate(t *testing.T) { // ============================================== List Model func NewMockListModel(storageShouldFail bool) *listModel { - storage := NewMockStorage(storageShouldFail) + storage := mock.NewMockStorage(storageShouldFail) // Create listModel using constructor function (using 'New' is important to preserve hotkeys) lm := New(context.TODO(), storage, nil, nil) items := make([]list.Item, 0) // Wrap hosts into List items - hosts := storage.hosts + hosts := storage.Hosts for _, h := range hosts { items = append(items, ListItemHost{Host: h}) } @@ -324,61 +323,3 @@ func NewMockListModel(storageShouldFail bool) *listModel { return &lm } - -// =============================================== Storage - -func NewMockStorage(shouldFail bool) *mockStorage { - hosts := []model.Host{ - model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"), - model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"), - model.NewHost(0, "Mock Host", "", "localhost", "root", "id_rsa", "2222"), - } - - return &mockStorage{ - shouldFail: shouldFail, - hosts: hosts, - } -} - -type mockStorage struct { - shouldFail bool - hosts []model.Host -} - -// Delete implements storage.HostStorage. -func (ms *mockStorage) Delete(id int) error { - if ms.shouldFail { - return errors.New("Mock error") - } - - ms.hosts = append(ms.hosts[:id], ms.hosts[id+1:]...) - - return nil -} - -// Get implements storage.HostStorage. -func (ms *mockStorage) Get(hostID int) (model.Host, error) { - if ms.shouldFail { - return model.Host{}, errors.New("Mock error") - } - - return model.Host{}, nil -} - -// GetAll implements storage.HostStorage. -func (ms *mockStorage) GetAll() ([]model.Host, error) { - if ms.shouldFail { - return ms.hosts, errors.New("Mock error") - } - - return ms.hosts, nil -} - -// Save implements storage.HostStorage. -func (ms *mockStorage) Save(m model.Host) (model.Host, error) { - if ms.shouldFail { - return m, errors.New("Mock error") - } - - return m, nil -}