From 08f8e0216648fa3b6fab25af7532bc282a82ace2 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sun, 19 Nov 2023 01:16:13 +0000 Subject: [PATCH 1/7] IMPROVEMENT-12: Add 'require' library for unit tests --- go.mod | 4 ++++ go.sum | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/go.mod b/go.mod index ed9d4ff..629df2c 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/charmbracelet/bubbletea v0.24.2 github.com/charmbracelet/lipgloss v0.8.0 github.com/samber/lo v1.38.1 + github.com/stretchr/testify v1.8.4 golang.org/x/exp v0.0.0-20230905200255-921286631fa9 golang.org/x/term v0.6.0 gopkg.in/yaml.v2 v2.4.0 @@ -17,6 +18,7 @@ require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.18 // indirect github.com/mattn/go-localereader v0.0.1 // indirect @@ -25,9 +27,11 @@ require ( github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/reflow v0.3.0 // indirect github.com/muesli/termenv v0.15.2 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.2.0 // indirect github.com/sahilm/fuzzy v0.1.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/sys v0.12.0 // indirect golang.org/x/text v0.3.8 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index e72c9b6..3a149f2 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,8 @@ github.com/charmbracelet/lipgloss v0.8.0 h1:IS00fk4XAHcf8uZKc3eHeMUTCxUH6NkaTrdy github.com/charmbracelet/lipgloss v0.8.0/go.mod h1:p4eYUZZJ/0oXTuCQKFF8mqyKCz0ja6y+7DniDDw5KKU= github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81 h1:q2hJAaP1k2wIvVRd/hEHD7lacgqrCPS+k8g1MndzfWY= github.com/containerd/console v1.0.4-0.20230313162750-1ae8d489ac81/go.mod h1:YynlIjWYF8myEu6sdkwKIvGQq+cOckRm6So2avqoYAk= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= @@ -30,6 +32,8 @@ github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s= github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8= github.com/muesli/termenv v0.15.2 h1:GohcuySI0QmI3wN8Ok9PtKGkgkFIk7y6Vpb5PvrY+Wo= github.com/muesli/termenv v0.15.2/go.mod h1:Epx+iuz8sNs7mNKhxzH4fWXGNpZwUaJKRS1noLXviQ8= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= @@ -37,6 +41,8 @@ github.com/sahilm/fuzzy v0.1.0 h1:FzWGaw2Opqyu+794ZQ9SYifWv2EIXpwP4q8dY1kDAwI= github.com/sahilm/fuzzy v0.1.0/go.mod h1:VFvziUEIMCrT6A6tw2RFIXPXXmzXbOsSHF0DOI8ZK9Y= github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM= github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/exp v0.0.0-20230905200255-921286631fa9 h1:GoHiUyI/Tp2nVkLI2mCxVkOjsbSXD66ic0XW0js0R9g= golang.org/x/exp v0.0.0-20230905200255-921286631fa9/go.mod h1:S2oDrQGGwySpoQPVqRShND87VCbxmc6bL1Yd2oYrm6k= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= @@ -53,3 +59,5 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 5346d8bbbdd460a388712f3ad24811c06742dce5 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sun, 19 Nov 2023 01:17:04 +0000 Subject: [PATCH 2/7] IMPROVEMENT-12: Add unit tests for utils package --- Makefile | 6 ++++ internal/utils/utils_test.go | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 internal/utils/utils_test.go diff --git a/Makefile b/Makefile index 5c96c5d..067f079 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,12 @@ test: @echo 'Running unit tests' go test -race -vet=off -count=1 -coverprofile unit.txt -covermode atomic ./... +## unit-test-report: display unit coverage report in html format +.PHONY: unit-test-report +unit-test-report: + @echo 'The report will be opened in the browser' + go tool cover -html unit.txt + ## run: delete logs and run debug .PHONY: run run: diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go new file mode 100644 index 0000000..7a45a2b --- /dev/null +++ b/internal/utils/utils_test.go @@ -0,0 +1,64 @@ +package utils + +import ( + "os" + "path" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_stringEmpty(t *testing.T) { + require.True(t, stringEmpty("")) + require.True(t, stringEmpty(" ")) + require.False(t, stringEmpty("test")) +} + +func Test_CreateAppDirIfNotExists(t *testing.T) { + tmpFile, _ := os.CreateTemp("", "unit_test_tmp*") + defer os.RemoveAll(tmpFile.Name()) // clean up + err := CreateAppDirIfNotExists(tmpFile.Name()) + require.Error(t, err, "CreateAppDirIfNotExists should return an error when home path exists and it's not a directory") + + err = CreateAppDirIfNotExists(" ") + require.Error(t, err, "CreateAppDirIfNotExists should return an error when argument is empty") + + tmpDir, _ := os.MkdirTemp("", "unit_test_tmp*") + defer os.RemoveAll(tmpDir) // clean up + err = CreateAppDirIfNotExists(tmpDir) + require.NoError(t, err, "CreateAppDirIfNotExists should not return an error when app home exists") + + tmpDir = path.Join(os.TempDir(), "test") + defer os.RemoveAll(tmpDir) // clean up + err = CreateAppDirIfNotExists(tmpDir) + require.NoError(t, err, "CreateAppDirIfNotExists should create app home folder if not exists") +} + +func Test_GetAppDir(t *testing.T) { + userConfigDir, _ := os.UserConfigDir() + + expected := path.Join(userConfigDir, "test") + got, _ := AppDir("test", "") + require.Equal(t, got, expected, "Should create a subfolder with a certain name in user config directory") + + expected, _ = filepath.Abs(".") + got, _ = AppDir("test", ".") + require.Equal(t, got, expected, "Should ignore application name and use a user-defined folder") + + tmp, _ := os.CreateTemp("", "unit_test_tmp*") + defer os.RemoveAll(tmp.Name()) + _, err := AppDir("", tmp.Name()) + require.Error(t, err, "Should not accept file as a user dir") + + _, err = AppDir("", "") + require.Error(t, err, "App home folder should not be empty 1") + + _, err = AppDir(" ", "") + require.Error(t, err, "App home folder should not be empty 2") +} + +func Test_GetCurrentOSUser(t *testing.T) { + username := CurrentOSUsername() + require.NotEmpty(t, username, "GetCurrentOSUser should return a non-empty string") +} From 81246484025946a81c9ccc308836653c5b249f59 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sun, 19 Nov 2023 01:17:51 +0000 Subject: [PATCH 3/7] IMPROVEMENT-12: Refactor and fix a couple minor bugs --- cmd/goto/main.go | 2 +- internal/constant/constant.go | 5 ++- internal/ui/component/edithost/edit_host.go | 2 +- internal/utils/utils.go | 40 +++++++++++++++------ 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/cmd/goto/main.go b/cmd/goto/main.go index 8ee717e..f4ddd4b 100644 --- a/cmd/goto/main.go +++ b/cmd/goto/main.go @@ -46,7 +46,7 @@ func main() { flag.Parse() var err error - commandLineParams.AppHome, err = utils.GetAppDir(appName, commandLineParams.AppHome) + commandLineParams.AppHome, err = utils.AppDir(appName, commandLineParams.AppHome) if err != nil { log.Fatalf("Can't get application home folder: %v", err) } diff --git a/internal/constant/constant.go b/internal/constant/constant.go index 6588a51..7151764 100644 --- a/internal/constant/constant.go +++ b/internal/constant/constant.go @@ -2,7 +2,10 @@ package constant import "errors" -var ErrNotFound = errors.New("not found") +var ( + ErrNotFound = errors.New("not found") + ErrBadArgument = errors.New("bad argument") +) // ErrDuplicateRecord = errors.New("duplicate record") // ErrDeleted = errors.New("deleted") diff --git a/internal/ui/component/edithost/edit_host.go b/internal/ui/component/edithost/edit_host.go index 57c6887..c9e588a 100644 --- a/internal/ui/component/edithost/edit_host.go +++ b/internal/ui/component/edithost/edit_host.go @@ -77,7 +77,7 @@ func New(ctx context.Context, storage storage.HostStorage, state *state.Applicat case 3: t.Label = "Login" t.CharLimit = 128 - t.Placeholder = fmt.Sprintf("default: %s", utils.GetCurrentOSUser()) + t.Placeholder = fmt.Sprintf("default: %s", utils.CurrentOSUsername()) t.SetValue(host.LoginName) case 4: t.Label = "Port" diff --git a/internal/utils/utils.go b/internal/utils/utils.go index fc93b8c..28937dd 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -6,29 +6,45 @@ import ( "os/user" "path" "path/filepath" + "strings" + + "github.com/grafviktor/goto/internal/constant" ) type Logger interface { Debug(format string, args ...any) } +func stringEmpty(s string) bool { + return len(strings.TrimSpace(s)) == 0 +} + func CreateAppDirIfNotExists(appConfigDir string) error { - _, err := os.Stat(appConfigDir) + if stringEmpty(appConfigDir) { + return constant.ErrBadArgument + } + stat, err := os.Stat(appConfigDir) if os.IsNotExist(err) { - err = os.MkdirAll(appConfigDir, 0o700) - if err != nil { - return err - } + return os.MkdirAll(appConfigDir, 0o700) } else if err != nil { return err } + if !stat.IsDir() { + return errors.New("app home path exists and it is not a directory") + } + return nil } -func GetAppDir(appName, userDefinedPath string) (string, error) { - if len(userDefinedPath) > 0 { +// AppDir - returns application home folder where all all files are stored. +// appName is application name which will be used as folder name. +// userDefinedPath allows you to set a custom path to application home folder, can be relative or absolute. +// If userDefinedPath is not empty, it will be used as application home folder +// Else, userConfigDir will be used, which is system dependent. +func AppDir(appName, userDefinedPath string) (string, error) { + if !stringEmpty(userDefinedPath) { absolutePath, err := filepath.Abs(userDefinedPath) if err != nil { return "", err @@ -46,9 +62,12 @@ func GetAppDir(appName, userDefinedPath string) (string, error) { return absolutePath, nil } - // FIXME: -------- DEBUG ------------- / + if stringEmpty(appName) { + return "", errors.New("application home folder name is not provided") + } + + // Left for debugging purposes // userConfigDir, err := os.Getwd() - // -------- RELEASE ----------- / userConfigDir, err := os.UserConfigDir() if err != nil { return "", err @@ -57,7 +76,8 @@ func GetAppDir(appName, userDefinedPath string) (string, error) { return path.Join(userConfigDir, appName), nil } -func GetCurrentOSUser() string { +// CurrentOSUsername - returns current OS username or "n/a" if it can't be determined. +func CurrentOSUsername() string { user, err := user.Current() if err != nil { return "n/a" From 8549f44358796cd44fc3f364f45f73d73864deba Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sun, 19 Nov 2023 01:28:26 +0000 Subject: [PATCH 4/7] IMPROVEMENT-12: Refactor and add documentation to utils package --- internal/utils/utils.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 28937dd..f5d00ad 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -11,14 +11,14 @@ import ( "github.com/grafviktor/goto/internal/constant" ) -type Logger interface { - Debug(format string, args ...any) -} - +// stringEmpty - checks if string is empty or contains only spaces. +// s is string to check. func stringEmpty(s string) bool { return len(strings.TrimSpace(s)) == 0 } +// CreateAppDirIfNotExists - creates application home folder if it doesn't exist. +// appConfigDir is application home folder path. func CreateAppDirIfNotExists(appConfigDir string) error { if stringEmpty(appConfigDir) { return constant.ErrBadArgument From 829d62c07ae2c2d2d2af362221fc6fe11de712b4 Mon Sep 17 00:00:00 2001 From: Cyberfox <149542080+lika8701@users.noreply.github.com> Date: Sat, 25 Nov 2023 00:49:27 +0000 Subject: [PATCH 5/7] Fix linting rules (#15) --- internal/utils/utils_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 8c59cb2..584eb79 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -7,9 +7,10 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/require" + "github.com/grafviktor/goto/internal/model" "github.com/grafviktor/goto/internal/utils/ssh" - "github.com/stretchr/testify/require" ) func Test_stringEmpty(t *testing.T) { From 6198320f3f31202bf03589915a407cb0b31a307d Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sat, 25 Nov 2023 01:04:43 +0000 Subject: [PATCH 6/7] IMPROVEMENT-12: Add codecode badge --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5930b9a..78d688d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # GOTO - A simple SSH manager # -[![License](https://img.shields.io/badge/License-MIT-blue.svg?style=flat-square)](https://raw.githubusercontent.com/grafviktor/goto/master/LICENSE) +[![License](https://img.shields.io/badge/license-MIT-blue.svg?style=flat-square)](https://raw.githubusercontent.com/grafviktor/goto/master/LICENSE) +[![Codecov](https://codecov.io/gh/grafviktor/goto/branch/develop/graph/badge.svg?token=wrIL0tyQ5q)](https://codecov.io/gh/grafviktor/goto) This utility helps to maintain a list of ssh servers. Unlike PuTTY it doesn't incorporate any connection logic, but relying on `ssh` utility which should be installed on your system. From 6351ae57c2d129f4f1423061309a055c00486990 Mon Sep 17 00:00:00 2001 From: Roman Leonenkov <6890447+grafviktor@users.noreply.github.com> Date: Sat, 25 Nov 2023 01:05:08 +0000 Subject: [PATCH 7/7] IMPROVEMENT-12: Refactor --- internal/ui/component/edithost/edit_host.go | 2 +- internal/utils/utils.go | 4 ++-- internal/utils/utils_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/ui/component/edithost/edit_host.go b/internal/ui/component/edithost/edit_host.go index 44089a2..0fa8aaf 100644 --- a/internal/ui/component/edithost/edit_host.go +++ b/internal/ui/component/edithost/edit_host.go @@ -77,7 +77,7 @@ func New(ctx context.Context, storage storage.HostStorage, state *state.Applicat case 3: t.Label = "Login" t.CharLimit = 128 - t.Placeholder = fmt.Sprintf("default: %s", utils.CurrentOSUsername()) + t.Placeholder = fmt.Sprintf("default: %s", utils.CurrentUsername()) t.SetValue(host.LoginName) case 4: t.Label = "Port" diff --git a/internal/utils/utils.go b/internal/utils/utils.go index ea9b2b0..fd43a82 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -79,8 +79,8 @@ func AppDir(appName, userDefinedPath string) (string, error) { return path.Join(userConfigDir, appName), nil } -// CurrentOSUsername - returns current OS username or "n/a" if it can't be determined. -func CurrentOSUsername() string { +// CurrentUsername - returns current OS username or "n/a" if it can't be determined. +func CurrentUsername() string { user, err := user.Current() if err != nil { return "n/a" diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 584eb79..a92bdeb 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -63,7 +63,7 @@ func Test_GetAppDir(t *testing.T) { } func Test_GetCurrentOSUser(t *testing.T) { - username := CurrentOSUsername() + username := CurrentUsername() require.NotEmpty(t, username, "GetCurrentOSUser should return a non-empty string") }