IMPROVEMENT-24: Increase unit test coverage

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}),

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)
+	*/
+}

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
-}