From f86369e75c0a06e5f1f7dc39163cf8a94a9a3532 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Sun, 22 Jun 2025 17:07:35 -0400 Subject: [PATCH 01/22] Add more tests for InstalledVersion --- plugin/utils_test.go | 65 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/plugin/utils_test.go b/plugin/utils_test.go index c63ba47..784e789 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -33,19 +33,70 @@ var twoEntryJson string = `[ } ]` -func TestInstalledVersionNoPluginFile(t *testing.T) { +func TestInstalledVersionNoPluginsJson(t *testing.T) { _, err := InstalledVersion("loremIpsum", afero.NewMemMapFs()) checkIfErrNotFound(t, err) } -func TestInstalledVersionEmptyPluginFile(t *testing.T) { - fs := afero.NewMemMapFs() - createAndWritePluginsJson(t, []byte("[]"), fs) - _, err := InstalledVersion("loremIpsum", fs) - checkIfErrNotFound(t, err) +func TestInstalledVersionWithPluginsJson(t *testing.T) { + testFuncErrNotFound := func(t *testing.T, _ string, err error) { + checkIfErrNotFound(t, err) + } + tests := map[string]struct { + pluginsJsonContent []byte + pluginName string + testFunc func(t *testing.T, version string, err error) + }{ + "empty plugins.json": { + pluginsJsonContent: []byte(`[]`), + pluginName: "loremIpsum", + testFunc: testFuncErrNotFound, + }, + "non-existent entry": { + pluginsJsonContent: []byte(oneEntryJson), + pluginName: "dolorSitAmet", + testFunc: testFuncErrNotFound, + }, + "invalid json": { + pluginsJsonContent: []byte(""), + pluginName: "loremIpsum", + testFunc: func(t *testing.T, version string, err error) { + if err == nil { + t.Fatal("want error, got nil") + } + if _, ok := err.(*json.SyntaxError); !ok { + t.Fatalf("want JSON syntax error, got %v", err) + } + if version != "" { + t.Fatalf("want version to be empty, got %v", version) + } + }, + }, + "existing entry": { + pluginsJsonContent: []byte(oneEntryJson), + pluginName: "loremIpsum", + testFunc: func(t *testing.T, version string, err error) { + if err != nil { + t.Fatalf("want no error, got %v", err) + } + if version != "0.0.0" { + t.Fatalf("want version to be '0.0.0', got %v", version) + } + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + createAndWritePluginsJson(t, tt.pluginsJsonContent, fs) + version, err := InstalledVersion(tt.pluginName, fs) + tt.testFunc(t, version, err) + }) + } } -func TestAddFirstEntryNoPluginFile(t *testing.T) { +func TestAddFirstEntryNoPluginsJson(t *testing.T) { fs := afero.NewMemMapFs() err := UpdateEntries(Entry{ Name: "loremIpsum", From bb35948df86216e4a216dd9d65b2444da3208793 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Mon, 23 Jun 2025 17:56:10 -0400 Subject: [PATCH 02/22] Table test for get plugin info and replace ErrNotFound with a shared type named NotFoundError --- cmd/plugincmds/install.go | 9 +++ cmd/plugincmds/install_test.go | 115 +++++++++++++++++++++------------ plugin/commands.go | 8 +-- plugin/errors.go | 5 -- plugin/utils.go | 47 +++++++++++--- plugin/utils_test.go | 50 +++++++------- shared/errors.go | 14 ++++ shared/testUtils.go | 23 +++++++ shared/types.go | 12 ++++ shared/{shared.go => utils.go} | 16 ++--- shared/variables.go | 11 ++++ 11 files changed, 217 insertions(+), 93 deletions(-) delete mode 100644 plugin/errors.go create mode 100644 shared/errors.go create mode 100644 shared/testUtils.go create mode 100644 shared/types.go rename shared/{shared.go => utils.go} (68%) create mode 100644 shared/variables.go diff --git a/cmd/plugincmds/install.go b/cmd/plugincmds/install.go index 81b8e5c..eb1b9b1 100644 --- a/cmd/plugincmds/install.go +++ b/cmd/plugincmds/install.go @@ -83,6 +83,15 @@ func getPluginInfoCmd(metadata plugin.Metadata) tea.Cmd { url = v.Url } } + if url == "" { + return shared.NotFoundError{ + Thing: "download", + Source: shared.Source{ + File: "cmd/plugincmds/install.go", + Function: "getPluginInfoCmd(metadata plugin.Metadata) tea.Cmd", + }, + } + } return pluginDownloadInfo{ Url: url, Name: metadata.Name, diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index 51972d3..e89c09d 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -14,52 +14,83 @@ import ( tea "github.com/charmbracelet/bubbletea" ) -func TestGetPluginInfo(t *testing.T) { - msg := getPluginInfoCmd(plugin.Metadata{ - Name: "loremIpsum", - Version: "0.0.0", - Downloads: []plugin.Download{ - { - OS: runtime.GOOS, - Arch: runtime.GOARCH, - Url: "https://example.com", +func TestGetPluginInfoTemp(t *testing.T) { + type test struct { + metadata plugin.Metadata + testFunc func(t *testing.T, msg tea.Msg) + } + tests := map[string]test{ + "existing download": { + metadata: plugin.Metadata{ + Name: "loremIpsum", + Version: "0.0.0", + Downloads: []plugin.Download{ + { + OS: runtime.GOOS, + Arch: runtime.GOARCH, + Url: "https://example.com", + }, + }, + }, + testFunc: func(t *testing.T, msg tea.Msg) { + if downloadInfo, ok := msg.(pluginDownloadInfo); ok { + if downloadInfo.Name != "loremIpsum" { + t.Fatalf("want name to be 'loremIpsum', got name '%v'", downloadInfo.Name) + } + if downloadInfo.Url != "https://example.com" { + t.Fatalf("want url to be 'https://example.com', got url '%v'", downloadInfo.Url) + } + compareVersionTo := semver.New(0, 0, 0, "", "") + if !compareVersionTo.Equal(downloadInfo.Version) { + t.Fatalf("Want version 0.0.0 got %v", downloadInfo.Version.String()) + } + } else if err, ok := msg.(error); ok { + t.Fatalf("want no error, got %v", err) + } else { + t.Fatalf("want pluginDownloadInfo returned, got %T with content %v", msg, msg) + } }, }, - })() - if downloadInfo, ok := msg.(pluginDownloadInfo); ok { - if downloadInfo.Name != "loremIpsum" { - t.Fatalf("want name to be 'loremIpsum', got name '%v'", downloadInfo.Name) - } - if downloadInfo.Url != "https://example.com" { - t.Fatalf("want url to be 'https://example.com', got url '%v'", downloadInfo.Url) - } - compareVersionTo := semver.New(0, 0, 0, "", "") - if !compareVersionTo.Equal(downloadInfo.Version) { - t.Fatalf("Want version 0.0.0 got %v", downloadInfo.Version.String()) - } - } else if err, ok := msg.(error); ok { - t.Fatalf("want no error, got %v", err) - } else { - t.Fatalf("want pluginDownloadInfo returned, got %T with content %v", msg, msg) - } -} - -func TestGetPluginInfoInvalidVersion(t *testing.T) { - msg := getPluginInfoCmd(plugin.Metadata{ - Name: "loremIpsum", - Version: "loremIpsum", - Downloads: []plugin.Download{ - { - OS: runtime.GOOS, - Arch: runtime.GOARCH, - Url: "https://example.com", + "no download": { + metadata: plugin.Metadata{ + Name: "loremIpsum", + Version: "0.0.0", + Downloads: []plugin.Download{ + { + OS: func() string { + if runtime.GOOS == "imaginaryOS" { + return "fakeOS" + } + return "imaginaryOS" + }(), + Arch: func() string { + if runtime.GOARCH == "imaginaryArch" { + return "fakeArch" + } + return "imaginaryArch" + }(), + Url: "https://example.com", + }, + }, + }, + testFunc: func(t *testing.T, msg tea.Msg) { + if err, ok := msg.(error); ok { + shared.AssertIsNotFoundError(t, err, "download", shared.Source{ + File: "cmd/plugincmds/install.go", + Function: "getPluginInfoCmd(metadata plugin.Metadata) tea.Cmd", + }) + } else { + t.Fatalf("want error, got %T with content %v", msg, msg) + } }, }, - })() - if err, ok := msg.(error); !ok { - t.Fatalf("want error, got %T with contents %v", msg, msg) - } else if !errors.Is(err, semver.ErrInvalidSemVer) { - t.Fatalf("want error containing ErrInvalidSemVer, got %v", err) + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + msg := getPluginInfoCmd(tt.metadata)() + tt.testFunc(t, msg) + }) } } diff --git a/plugin/commands.go b/plugin/commands.go index 31bf2ce..8b3c1d6 100644 --- a/plugin/commands.go +++ b/plugin/commands.go @@ -1,8 +1,6 @@ package plugin import ( - "errors" - "github.com/MTVersionManager/mtvm/shared" tea "github.com/charmbracelet/bubbletea" "github.com/spf13/afero" @@ -25,7 +23,7 @@ func InstalledVersionCmd(pluginName string, fs afero.Fs) tea.Cmd { return func() tea.Msg { version, err := InstalledVersion(pluginName, fs) if err != nil { - if errors.Is(err, ErrNotFound) { + if shared.IsNotFound(err) { return NotFoundMsg{ PluginName: pluginName, Source: "InstalledVersion", @@ -41,7 +39,7 @@ func RemoveEntryCmd(pluginName string, fs afero.Fs) tea.Cmd { return func() tea.Msg { err := RemoveEntry(pluginName, fs) if err != nil { - if errors.Is(err, ErrNotFound) { + if shared.IsNotFound(err) { return NotFoundMsg{ PluginName: pluginName, Source: "RemoveEntry", @@ -57,7 +55,7 @@ func RemoveCmd(pluginName string, fs afero.Fs) tea.Cmd { return func() tea.Msg { err := Remove(pluginName, fs) if err != nil { - if errors.Is(err, ErrNotFound) { + if shared.IsNotFound(err) { return NotFoundMsg{ PluginName: pluginName, Source: "Remove", diff --git a/plugin/errors.go b/plugin/errors.go deleted file mode 100644 index 7db2c3e..0000000 --- a/plugin/errors.go +++ /dev/null @@ -1,5 +0,0 @@ -package plugin - -import "errors" - -var ErrNotFound = errors.New("plugin not found") diff --git a/plugin/utils.go b/plugin/utils.go index af58248..8de4823 100644 --- a/plugin/utils.go +++ b/plugin/utils.go @@ -12,7 +12,7 @@ import ( "github.com/MTVersionManager/mtvm/config" ) -// UpdateEntries updates the data of an entry if it exists, and adds an entry if it doesn't +// UpdateEntries updates the data of an entry if it exists and adds an entry if it doesn't func UpdateEntries(entry Entry, fs afero.Fs) error { configDir, err := config.GetConfigDir() if err != nil { @@ -52,7 +52,7 @@ func UpdateEntries(entry Entry, fs afero.Fs) error { } // InstalledVersion returns the current version of a plugin that is installed. -// Returns an ErrNotFound if the version is not found. +// Returns a NotFoundError if the version is not found. func InstalledVersion(pluginName string, fs afero.Fs) (string, error) { configDir, err := config.GetConfigDir() if err != nil { @@ -61,7 +61,13 @@ func InstalledVersion(pluginName string, fs afero.Fs) (string, error) { data, err := afero.ReadFile(fs, filepath.Join(configDir, "plugins.json")) if err != nil { if os.IsNotExist(err) { - return "", ErrNotFound + return "", shared.NotFoundError{ + Thing: "plugins.json", + Source: shared.Source{ + File: "plugin/utils.go", + Function: "InstalledVersion(pluginName string, fs afero.Fs) (string, error)", + }, + } } return "", err } @@ -75,7 +81,13 @@ func InstalledVersion(pluginName string, fs afero.Fs) (string, error) { return v.Version, nil } } - return "", fmt.Errorf("%w: %s", ErrNotFound, pluginName) + return "", fmt.Errorf("%w: %s", shared.NotFoundError{ + Thing: "entry", + Source: shared.Source{ + File: "plugin/utils.go", + Function: "InstalledVersion(pluginName string, fs afero.Fs) (string, error)", + }, + }, pluginName) } // GetEntries returns a list of installed plugins and an error @@ -107,7 +119,13 @@ func RemoveEntry(pluginName string, fs afero.Fs) error { data, err := afero.ReadFile(fs, filepath.Join(configDir, "plugins.json")) if err != nil { if os.IsNotExist(err) { - return ErrNotFound + return shared.NotFoundError{ + Thing: "plugins.json", + Source: shared.Source{ + File: "plugin/utils.go", + Function: "RemoveEntry(pluginName string, fs afero.Fs) error", + }, + } } return err } @@ -116,8 +134,15 @@ func RemoveEntry(pluginName string, fs afero.Fs) error { if err != nil { return err } + notFound := shared.NotFoundError{ + Thing: "entry", + Source: shared.Source{ + File: "plugin/utils.go", + Function: "RemoveEntry(pluginName string, fs afero.Fs) error", + }, + } if len(entries) == 0 { - return ErrNotFound + return notFound } removed := make([]Entry, 0, len(entries)-1) for _, v := range entries { @@ -126,7 +151,7 @@ func RemoveEntry(pluginName string, fs afero.Fs) error { } } if len(removed) == len(entries) { - return ErrNotFound + return notFound } data, err = json.MarshalIndent(removed, "", " ") if err != nil { @@ -138,7 +163,13 @@ func RemoveEntry(pluginName string, fs afero.Fs) error { func Remove(pluginName string, fs afero.Fs) error { err := fs.Remove(filepath.Join(shared.Configuration.PluginDir, pluginName+shared.LibraryExtension)) if os.IsNotExist(err) { - return ErrNotFound + return shared.NotFoundError{ + Thing: "plugin", + Source: shared.Source{ + File: "plugin/utils.go", + Function: "Remove(pluginName string, fs afero.Fs) error", + }, + } } return err } diff --git a/plugin/utils_test.go b/plugin/utils_test.go index 784e789..b431dcc 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -12,7 +12,7 @@ import ( "github.com/spf13/afero" ) -var oneEntryJson string = `[ +var oneEntryJson = `[ { "name": "loremIpsum", "version": "0.0.0", @@ -20,7 +20,7 @@ var oneEntryJson string = `[ } ]` -var twoEntryJson string = `[ +var twoEntryJson = `[ { "name": "loremIpsum", "version": "0.0.0", @@ -35,12 +35,18 @@ var twoEntryJson string = `[ func TestInstalledVersionNoPluginsJson(t *testing.T) { _, err := InstalledVersion("loremIpsum", afero.NewMemMapFs()) - checkIfErrNotFound(t, err) + shared.AssertIsNotFoundError(t, err, "plugins.json", shared.Source{ + File: "plugin/utils.go", + Function: "InstalledVersion(pluginName string, fs afero.Fs) (string, error)", + }) } func TestInstalledVersionWithPluginsJson(t *testing.T) { testFuncErrNotFound := func(t *testing.T, _ string, err error) { - checkIfErrNotFound(t, err) + shared.AssertIsNotFoundError(t, err, "entry", shared.Source{ + File: "plugin/utils.go", + Function: "InstalledVersion(pluginName string, fs afero.Fs) (string, error)", + }) } tests := map[string]struct { pluginsJsonContent []byte @@ -61,12 +67,7 @@ func TestInstalledVersionWithPluginsJson(t *testing.T) { pluginsJsonContent: []byte(""), pluginName: "loremIpsum", testFunc: func(t *testing.T, version string, err error) { - if err == nil { - t.Fatal("want error, got nil") - } - if _, ok := err.(*json.SyntaxError); !ok { - t.Fatalf("want JSON syntax error, got %v", err) - } + checkIfJsonSyntaxError(t, err) if version != "" { t.Fatalf("want version to be empty, got %v", version) } @@ -233,12 +234,18 @@ func TestGetEntriesWithPluginsJson(t *testing.T) { func TestRemoveEntryWithoutPluginsJson(t *testing.T) { fs := afero.NewMemMapFs() err := RemoveEntry("loremIpsum", fs) - checkIfErrNotFound(t, err) + shared.AssertIsNotFoundError(t, err, "plugins.json", shared.Source{ + File: "plugin/utils.go", + Function: "RemoveEntry(pluginName string, fs afero.Fs) error", + }) } func TestRemoveEntryWithPluginsJson(t *testing.T) { testFuncErrNotFound := func(t *testing.T, _ afero.Fs, err error) { - checkIfErrNotFound(t, err) + shared.AssertIsNotFoundError(t, err, "entry", shared.Source{ + File: "plugin/utils.go", + Function: "RemoveEntry(pluginName string, fs afero.Fs) error", + }) } tests := map[string]struct { pluginToRemove string @@ -272,12 +279,7 @@ func TestRemoveEntryWithPluginsJson(t *testing.T) { pluginToRemove: "loremIpsum", pluginsJsonContent: []byte(""), testFunc: func(t *testing.T, _ afero.Fs, err error) { - if err == nil { - t.Fatal("want error, got nil") - } - if _, ok := err.(*json.SyntaxError); !ok { - t.Fatalf("want JSON syntax error, got %v", err) - } + checkIfJsonSyntaxError(t, err) }, }, } @@ -324,7 +326,10 @@ func TestRemoveExisting(t *testing.T) { func TestRemoveNonExistent(t *testing.T) { fs := afero.NewMemMapFs() err := Remove("loremIpsum", fs) - checkIfErrNotFound(t, err) + shared.AssertIsNotFoundError(t, err, "plugin", shared.Source{ + File: "plugin/utils.go", + Function: "Remove(pluginName string, fs afero.Fs) error", + }) } func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { @@ -360,11 +365,12 @@ func readPluginsJson(t *testing.T, fs afero.Fs) []byte { return data } -func checkIfErrNotFound(t *testing.T, err error) { +func checkIfJsonSyntaxError(t *testing.T, err error) { if err == nil { t.Fatal("want error, got nil") } - if !errors.Is(err, ErrNotFound) { - t.Fatalf("want error to contain ErrNotFound, got error not containing ErrNotFound") + var syntaxError *json.SyntaxError + if !errors.As(err, &syntaxError) { + t.Fatalf("want JSON syntax error, got %v", err) } } diff --git a/shared/errors.go b/shared/errors.go new file mode 100644 index 0000000..4a6ce1e --- /dev/null +++ b/shared/errors.go @@ -0,0 +1,14 @@ +package shared + +import "fmt" + +type NotFoundError struct { + // The thing that couldn't be found + Thing string + // The source of the error (the function that returned it and what file that function is in) + Source Source +} + +func (e NotFoundError) Error() string { + return fmt.Sprintf("%v could not find %v", e.Source, e.Thing) +} diff --git a/shared/testUtils.go b/shared/testUtils.go new file mode 100644 index 0000000..27a089d --- /dev/null +++ b/shared/testUtils.go @@ -0,0 +1,23 @@ +package shared + +import ( + "errors" + "testing" +) + +func AssertIsNotFoundError(t *testing.T, err error, thing string, source Source) { + if err == nil { + t.Fatal("want error, got nil") + } + var notFoundError NotFoundError + errors.As(err, ¬FoundError) + if !errors.As(err, ¬FoundError) { + t.Fatalf("want error to be NotFoundError, got error not containing NotFoundError") + } + if notFoundError.Thing != thing { + t.Fatalf("want error to contain thing %v, got %v", thing, notFoundError.Thing) + } + if notFoundError.Source != source { + t.Fatalf("want error to contain source %v, got %v", source, notFoundError.Source) + } +} diff --git a/shared/types.go b/shared/types.go new file mode 100644 index 0000000..851a534 --- /dev/null +++ b/shared/types.go @@ -0,0 +1,12 @@ +package shared + +type SuccessMsg string + +type Source struct { + File string + Function string +} + +func (s Source) String() string { + return s.File + ":" + s.Function +} diff --git a/shared/shared.go b/shared/utils.go similarity index 68% rename from shared/shared.go rename to shared/utils.go index 2b81565..cdc3207 100644 --- a/shared/shared.go +++ b/shared/utils.go @@ -4,21 +4,10 @@ import ( "errors" "os" "path/filepath" - // "strings" - // "github.com/MTVersionManager/goplugin" "github.com/MTVersionManager/mtvmplugin" - "github.com/charmbracelet/lipgloss" - - "github.com/MTVersionManager/mtvm/config" ) -var Configuration config.Config - -type SuccessMsg string - -var CheckMark = lipgloss.NewStyle().Foreground(lipgloss.Color("2")).SetString("✓").String() - func IsVersionInstalled(tool, version string) (bool, error) { _, err := os.Stat(filepath.Join(Configuration.InstallDir, tool, version)) if err != nil { @@ -39,3 +28,8 @@ func LoadPlugin(tool string) (mtvmplugin.Plugin, error) { // } // return plugin, nil } + +func IsNotFound(err error) bool { + var notFound NotFoundError + return errors.As(err, ¬Found) +} diff --git a/shared/variables.go b/shared/variables.go new file mode 100644 index 0000000..53deed4 --- /dev/null +++ b/shared/variables.go @@ -0,0 +1,11 @@ +package shared + +import ( + "github.com/MTVersionManager/mtvm/config" + "github.com/charmbracelet/lipgloss" +) + +var ( + Configuration config.Config + CheckMark = lipgloss.NewStyle().Foreground(lipgloss.Color("2")).SetString("✓").String() +) From 47f2b1faac47b2475619b5d8c4b55b709ef3b321 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:00:58 -0400 Subject: [PATCH 03/22] Add invalid version test for getplugininfo and remove temporary function name --- cmd/plugincmds/install_test.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index e89c09d..8dea7c0 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -14,7 +14,7 @@ import ( tea "github.com/charmbracelet/bubbletea" ) -func TestGetPluginInfoTemp(t *testing.T) { +func TestGetPluginInfo(t *testing.T) { type test struct { metadata plugin.Metadata testFunc func(t *testing.T, msg tea.Msg) @@ -84,6 +84,28 @@ func TestGetPluginInfoTemp(t *testing.T) { } }, }, + "invalid version": { + metadata: plugin.Metadata{ + Name: "loremIpsum", + Version: "IAmAnInvalidVersion", + Downloads: []plugin.Download{ + { + OS: runtime.GOOS, + Arch: runtime.GOARCH, + Url: "https://example.com", + }, + }, + }, + testFunc: func(t *testing.T, msg tea.Msg) { + if err, ok := msg.(error); ok { + if !errors.Is(err, semver.ErrInvalidSemVer) { + t.Fatalf("want error to be semver.ErrInvalidSemVer, got %v", err) + } + } else { + t.Fatalf("want error, got %T with content %v", msg, msg) + } + }, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 300b6e93e059bd5bc24afeb1ba592158cbdfd42e Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:19:20 -0400 Subject: [PATCH 04/22] Tests for shared.NotFoundError.Error() and shared.Source.String() --- shared/errors_test.go | 18 ++++++++++++++++++ shared/types_test.go | 15 +++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 shared/errors_test.go create mode 100644 shared/types_test.go diff --git a/shared/errors_test.go b/shared/errors_test.go new file mode 100644 index 0000000..1e2a7f1 --- /dev/null +++ b/shared/errors_test.go @@ -0,0 +1,18 @@ +package shared + +import "testing" + +func TestNotFoundError_Error(t *testing.T) { + err := NotFoundError{ + Thing: "lorem", + Source: Source{ + File: "ipsum", + Function: "dolor", + }, + } + expected := "ipsum:dolor could not find lorem" + output := err.Error() + if output != expected { + t.Fatalf("want %v, got %v", expected, output) + } +} diff --git a/shared/types_test.go b/shared/types_test.go new file mode 100644 index 0000000..5a8ab8c --- /dev/null +++ b/shared/types_test.go @@ -0,0 +1,15 @@ +package shared + +import "testing" + +func TestSource_String(t *testing.T) { + source := Source{ + File: "lorem", + Function: "ipsum", + } + expected := "lorem:ipsum" + output := source.String() + if output != expected { + t.Fatalf("want %v, got %v", expected, output) + } +} From a36a04c6730fc4d708adc2971a001679043d829f Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:55:18 -0400 Subject: [PATCH 05/22] Use testify assertations --- cmd/plugincmds/install_test.go | 77 +++++++------------ go.mod | 3 + plugin/utils_test.go | 132 +++++++++------------------------ shared/errors_test.go | 9 ++- 4 files changed, 70 insertions(+), 151 deletions(-) diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index 8dea7c0..b53ab72 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -3,10 +3,11 @@ package plugincmds import ( "context" "errors" - "fmt" "runtime" "testing" + "github.com/stretchr/testify/assert" + "github.com/MTVersionManager/mtvm/components/downloader" "github.com/MTVersionManager/mtvm/plugin" "github.com/MTVersionManager/mtvm/shared" @@ -34,12 +35,8 @@ func TestGetPluginInfo(t *testing.T) { }, testFunc: func(t *testing.T, msg tea.Msg) { if downloadInfo, ok := msg.(pluginDownloadInfo); ok { - if downloadInfo.Name != "loremIpsum" { - t.Fatalf("want name to be 'loremIpsum', got name '%v'", downloadInfo.Name) - } - if downloadInfo.Url != "https://example.com" { - t.Fatalf("want url to be 'https://example.com', got url '%v'", downloadInfo.Url) - } + assert.Equalf(t, "loremIpsum", downloadInfo.Name, "want name to be 'loremIpsum', got name '%v'", downloadInfo.Name) + assert.Equalf(t, "https://example.com", downloadInfo.Url, "want url to be 'https://example.com', got url '%v'", downloadInfo.Url) compareVersionTo := semver.New(0, 0, 0, "", "") if !compareVersionTo.Equal(downloadInfo.Version) { t.Fatalf("Want version 0.0.0 got %v", downloadInfo.Version.String()) @@ -116,50 +113,28 @@ func TestGetPluginInfo(t *testing.T) { } } -func TestInstallUpdateCancelQ(t *testing.T) { - err := CancelTest(tea.KeyMsg{ - Type: tea.KeyRunes, - Runes: []rune{'q'}, - }) - if err != nil { - t.Fatal(err) - } -} - -func TestPluginInstallUpdateCancelCtrlC(t *testing.T) { - err := CancelTest(tea.KeyMsg{ - Type: tea.KeyCtrlC, - }) - if err != nil { - t.Fatal(err) - } -} - -func CancelTest(keyPress tea.KeyMsg) error { - model := initialInstallModel("https://example.com") - _, cancel := context.WithCancel(context.Background()) - modelUpdated, _ := model.Update(downloader.DownloadStartedMsg{ - Cancel: cancel, - }) - _, cmd := modelUpdated.Update(keyPress) - if cmd == nil { - return errors.New("want not nil command, got nil") - } - msg := cmd() - if _, ok := msg.(downloader.DownloadCanceledMsg); !ok { - return fmt.Errorf("expected returned command to return downloader.DownloadCanceledMsg, returned %v with type %T", msg, msg) - } - return nil -} - -func TestPluginInstallUpdateEntriesSuccess(t *testing.T) { - model := initialInstallModel("https://example.com") - _, cmd := model.Update(shared.SuccessMsg("UpdateEntries")) - if cmd == nil { - t.Fatal("want not nil command, got nil") +func TestInstallUpdateCancel(t *testing.T) { + tests := map[string]tea.KeyMsg{ + "ctrl+c": { + Type: tea.KeyCtrlC, + }, + "q": { + Type: tea.KeyRunes, + Runes: []rune{'q'}, + }, } - msg := cmd() - if _, ok := msg.(tea.QuitMsg); !ok { - t.Fatalf("want command to return tea.QuitMsg, returned %T with content %v", msg, msg) + for name, keyPress := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + model := initialInstallModel("https://example.com") + _, cancel := context.WithCancel(context.Background()) + modelUpdated, _ := model.Update(downloader.DownloadStartedMsg{ + Cancel: cancel, + }) + _, cmd := modelUpdated.Update(keyPress) + assert.NotNil(t, cmd, "want not nil command, got nil") + msg := cmd() + assert.IsType(t, downloader.DownloadCanceledMsg{}, msg) + }) } } diff --git a/go.mod b/go.mod index 7d3e60f..0852589 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/spf13/afero v1.14.0 github.com/spf13/cobra v1.9.1 github.com/spf13/viper v1.20.1 + github.com/stretchr/testify v1.10.0 ) require ( @@ -22,6 +23,7 @@ require ( github.com/charmbracelet/x/ansi v0.8.0 // indirect github.com/charmbracelet/x/cellbuf v0.0.13 // indirect github.com/charmbracelet/x/term v0.2.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/gabriel-vasile/mimetype v1.4.8 // indirect @@ -39,6 +41,7 @@ require ( github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/termenv v0.16.0 // indirect github.com/pelletier/go-toml/v2 v2.2.3 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/sagikazarmark/locafero v0.9.0 // indirect github.com/sourcegraph/conc v0.3.0 // indirect diff --git a/plugin/utils_test.go b/plugin/utils_test.go index b431dcc..3f80be8 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -2,11 +2,12 @@ package plugin import ( "encoding/json" - "errors" "os" "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "github.com/MTVersionManager/mtvm/config" "github.com/MTVersionManager/mtvm/shared" "github.com/spf13/afero" @@ -68,21 +69,15 @@ func TestInstalledVersionWithPluginsJson(t *testing.T) { pluginName: "loremIpsum", testFunc: func(t *testing.T, version string, err error) { checkIfJsonSyntaxError(t, err) - if version != "" { - t.Fatalf("want version to be empty, got %v", version) - } + assert.Emptyf(t, version, "want version to be empty, got %v", version) }, }, "existing entry": { pluginsJsonContent: []byte(oneEntryJson), pluginName: "loremIpsum", testFunc: func(t *testing.T, version string, err error) { - if err != nil { - t.Fatalf("want no error, got %v", err) - } - if version != "0.0.0" { - t.Fatalf("want version to be '0.0.0', got %v", version) - } + assert.Nilf(t, err, "want no error, got %v", err) + assert.Equal(t, "0.0.0", version) }, }, } @@ -104,13 +99,9 @@ func TestAddFirstEntryNoPluginsJson(t *testing.T) { Version: "0.0.0", MetadataUrl: "https://example.com", }, fs) - if err != nil { - t.Fatalf("want no error, got %v", err) - } + assert.Nilf(t, err, "want no error, got %v", err) data := readPluginsJson(t, fs) - if string(data) != oneEntryJson { - t.Fatalf("want plugins.json to contain\n%v\ngot plugins.json containing\n%v", oneEntryJson, string(data)) - } + assert.Equal(t, oneEntryJson, string(data)) } func TestUpdateEntryWithPluginsJson(t *testing.T) { @@ -130,9 +121,7 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { wantsError: false, testFunc: func(t *testing.T, fs afero.Fs, err error) { data := readPluginsJson(t, fs) - if string(data) != twoEntryJson { - t.Fatalf("want plugins.json to contain\n%v\ngot plugins.json containing\n%v", twoEntryJson, string(data)) - } + assert.Equal(t, twoEntryJson, string(data)) }, }, "update existing": { @@ -152,9 +141,7 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { "metadataUrl": "https://example.com" } ]` - if string(data) != expected { - t.Fatalf("want plugins.json to contain\n%v\ngot plugins.json containing\n%v", expected, string(data)) - } + assert.Equal(t, expected, string(data)) }, }, } @@ -164,11 +151,11 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { fs := afero.NewMemMapFs() createAndWritePluginsJson(t, tt.pluginsJsonContent, fs) err := UpdateEntries(tt.entry, fs) - if tt.wantsError && err == nil { - t.Fatal("want error, got nil") + if tt.wantsError { + assert.NotNil(t, err, "want error, got nil") } - if !tt.wantsError && err != nil { - t.Fatalf("want no error, got %v", err) + if !tt.wantsError { + assert.Nilf(t, err, "want no error, got %v", err) } tt.testFunc(t, fs, err) }) @@ -178,12 +165,8 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { func TestGetEntriesWithNoPluginsJson(t *testing.T) { fs := afero.NewMemMapFs() entries, err := GetEntries(fs) - if err != nil { - t.Fatalf("want no error, got %v", err) - } - if entries != nil { - t.Fatalf("want entries to be nil, got %v", entries) - } + assert.Nilf(t, err, "want no error, got %v", err) + assert.Nilf(t, entries, "want entries to be nil, got %v", err) } func TestGetEntriesWithPluginsJson(t *testing.T) { @@ -194,29 +177,17 @@ func TestGetEntriesWithPluginsJson(t *testing.T) { "empty": { pluginsJsonContent: []byte(`[]`), testFunc: func(t *testing.T, entries []Entry, err error) { - if err != nil { - t.Fatalf("want no error, got %v", err) - } - if len(entries) != 0 { - t.Fatalf("want entries to be empty, got %v", entries) - } + assert.Nilf(t, err, "want no error, got %v", err) + assert.Lenf(t, entries, 0, "want entries to be empty, got %v", entries) }, }, "two entries": { pluginsJsonContent: []byte(twoEntryJson), testFunc: func(t *testing.T, entries []Entry, err error) { - if err != nil { - t.Fatalf("want no error, got %v", err) - } - if len(entries) != 2 { - t.Fatalf("want 2 entries, got %v entries containing %v", len(entries), entries) - } - if entries[0].Name != "loremIpsum" { - t.Fatalf("wanted first entry name to be 'loremIpsum', got %v", entries[0].Name) - } - if entries[1].Name != "dolorSitAmet" { - t.Fatalf("wanted second entry name to be 'dolorSitAmet', got %v", entries[1].Name) - } + assert.Nilf(t, err, "want no error, got %v", err) + assert.Len(t, entries, 2, "want 2 entries") + assert.Equalf(t, "loremIpsum", entries[0].Name, "want first entry name to be 'loremIpsum', got %v", entries[0].Name) + assert.Equalf(t, "dolorSitAmet", entries[1].Name, "want second entry name to be 'dolorSitAmet', got %v", entries[1].Name) }, }, } @@ -256,13 +227,9 @@ func TestRemoveEntryWithPluginsJson(t *testing.T) { pluginToRemove: "dolorSitAmet", pluginsJsonContent: []byte(twoEntryJson), testFunc: func(t *testing.T, fs afero.Fs, err error) { - if err != nil { - t.Fatalf("want no error, got %v", err) - } + assert.Nilf(t, err, "want no error, got %v", err) data := readPluginsJson(t, fs) - if string(data) != oneEntryJson { - t.Fatalf("want plugins.json to contain\n%v\ngot plugins.json containing\n%v", oneEntryJson, string(data)) - } + assert.Equal(t, oneEntryJson, string(data)) }, }, "non-existent entry": { @@ -298,26 +265,16 @@ func TestRemoveExisting(t *testing.T) { fs := afero.NewMemMapFs() var err error shared.Configuration, err = config.GetConfig() - if err != nil { - t.Fatalf("want no error when getting configuration, got %v", err) - } + assert.Nilf(t, err, "want no error when getting configuration, got %v", err) err = fs.MkdirAll(shared.Configuration.PluginDir, 0o777) - if err != nil { - t.Fatalf("want no error when creating plugin directory, got %v", err) - } + assert.Nilf(t, err, "want no error when creating plugin directory, got %v", err) pluginPath := filepath.Join(shared.Configuration.PluginDir, "loremIpsum"+shared.LibraryExtension) _, err = fs.Create(pluginPath) - if err != nil { - t.Fatalf("want no error when creating plugin file, got %v", err) - } + assert.Nilf(t, err, "want no error when creating plugin file, got %v", err) err = Remove("loremIpsum", fs) - if err != nil { - t.Fatalf("want no error, got %v", err) - } + assert.Nilf(t, err, "want no error, got %v", err) _, err = fs.Stat(pluginPath) - if err == nil { - t.Fatal("want error, got nil (stat)") - } + assert.NotNil(t, err, "want error, got nil (stat)") if !os.IsNotExist(err) { t.Fatalf("want file does not exist error, got %v (stat)", err) } @@ -334,43 +291,26 @@ func TestRemoveNonExistent(t *testing.T) { func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { configDir, err := config.GetConfigDir() - if err != nil { - t.Fatalf("want no error when getting config directory, got %v", err) - } + assert.Nilf(t, err, "want no error when getting config directory, got %v", err) err = fs.MkdirAll(configDir, 0o666) - if err != nil { - t.Fatalf("want no error when creating config directory, got %v", err) - } + assert.Nilf(t, err, "want no error when creating config directory, got %v", err) file, err := fs.Create(filepath.Join(configDir, "plugins.json")) - if err != nil { - t.Fatalf("want no error when creating plugins.json, got %v", err) - } + assert.Nilf(t, err, "want no error when creating plugins.json, got %v", err) defer file.Close() _, err = file.Write(content) - if err != nil { - t.Fatalf("want no error when writing to plugins.json, got %v", err) - } + assert.Nilf(t, err, "want no error when writing to plugins.json, got %v", err) } func readPluginsJson(t *testing.T, fs afero.Fs) []byte { configDir, err := config.GetConfigDir() - if err != nil { - t.Fatalf("want no error when getting config directory, got %v", err) - return nil - } + assert.Nilf(t, err, "want no error when getting config directory, got %v", err) data, err := afero.ReadFile(fs, filepath.Join(configDir, "plugins.json")) - if err != nil { - t.Fatalf("want no error when reading plugins.json, got %v", err) - } + assert.Nilf(t, err, "want no error when reading plugins.json, got %v", err) return data } func checkIfJsonSyntaxError(t *testing.T, err error) { - if err == nil { - t.Fatal("want error, got nil") - } + assert.NotNil(t, err, "want error, got nil") var syntaxError *json.SyntaxError - if !errors.As(err, &syntaxError) { - t.Fatalf("want JSON syntax error, got %v", err) - } + assert.ErrorAsf(t, err, &syntaxError, "want JSON syntax error, got %v", err) } diff --git a/shared/errors_test.go b/shared/errors_test.go index 1e2a7f1..92227e5 100644 --- a/shared/errors_test.go +++ b/shared/errors_test.go @@ -1,6 +1,9 @@ package shared -import "testing" +import ( + "github.com/stretchr/testify/assert" + "testing" +) func TestNotFoundError_Error(t *testing.T) { err := NotFoundError{ @@ -12,7 +15,5 @@ func TestNotFoundError_Error(t *testing.T) { } expected := "ipsum:dolor could not find lorem" output := err.Error() - if output != expected { - t.Fatalf("want %v, got %v", expected, output) - } + assert.Equal(t, expected, output) } From 2639f08a9ee7ed7250d32ee2d55aaf4d0acd6e6f Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 13:48:16 -0400 Subject: [PATCH 06/22] Use fatal and error when they should be used and use more testify when possible --- cmd/plugincmds/install_test.go | 19 +++++++++--------- components/downloader/downloader_test.go | 21 ++++++++------------ plugin/utils_test.go | 25 ++++++++++++------------ shared/errors_test.go | 7 +++---- shared/testUtils.go | 2 +- shared/types_test.go | 10 ++++++---- 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index b53ab72..5860474 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -2,10 +2,11 @@ package plugincmds import ( "context" - "errors" "runtime" "testing" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" "github.com/MTVersionManager/mtvm/components/downloader" @@ -39,12 +40,12 @@ func TestGetPluginInfo(t *testing.T) { assert.Equalf(t, "https://example.com", downloadInfo.Url, "want url to be 'https://example.com', got url '%v'", downloadInfo.Url) compareVersionTo := semver.New(0, 0, 0, "", "") if !compareVersionTo.Equal(downloadInfo.Version) { - t.Fatalf("Want version 0.0.0 got %v", downloadInfo.Version.String()) + t.Errorf("Want version 0.0.0 got %v", downloadInfo.Version.String()) } } else if err, ok := msg.(error); ok { - t.Fatalf("want no error, got %v", err) + t.Errorf("want no error, got %v", err) } else { - t.Fatalf("want pluginDownloadInfo returned, got %T with content %v", msg, msg) + t.Errorf("want pluginDownloadInfo returned, got %T with content %v", msg, msg) } }, }, @@ -77,7 +78,7 @@ func TestGetPluginInfo(t *testing.T) { Function: "getPluginInfoCmd(metadata plugin.Metadata) tea.Cmd", }) } else { - t.Fatalf("want error, got %T with content %v", msg, msg) + t.Errorf("want error, got %T with content %v", msg, msg) } }, }, @@ -95,11 +96,9 @@ func TestGetPluginInfo(t *testing.T) { }, testFunc: func(t *testing.T, msg tea.Msg) { if err, ok := msg.(error); ok { - if !errors.Is(err, semver.ErrInvalidSemVer) { - t.Fatalf("want error to be semver.ErrInvalidSemVer, got %v", err) - } + assert.ErrorIs(t, err, semver.ErrInvalidSemVer) } else { - t.Fatalf("want error, got %T with content %v", msg, msg) + t.Errorf("want error, got %T with content %v", msg, msg) } }, }, @@ -132,7 +131,7 @@ func TestInstallUpdateCancel(t *testing.T) { Cancel: cancel, }) _, cmd := modelUpdated.Update(keyPress) - assert.NotNil(t, cmd, "want not nil command, got nil") + require.NotNil(t, cmd, "want not nil command, got nil") msg := cmd() assert.IsType(t, downloader.DownloadCanceledMsg{}, msg) }) diff --git a/components/downloader/downloader_test.go b/components/downloader/downloader_test.go index 650d9b1..d45e37d 100644 --- a/components/downloader/downloader_test.go +++ b/components/downloader/downloader_test.go @@ -1,6 +1,9 @@ package downloader -import "testing" +import ( + "github.com/stretchr/testify/assert" + "testing" +) func TestDownloadWriter_Write(t *testing.T) { dw := downloadWriter{ @@ -22,22 +25,14 @@ func TestDownloadWriter_Write(t *testing.T) { for i := 0; i < 2; i++ { select { case progress := <-dw.progressChannel: - if progress != 0.5 { - t.Fatalf("want 0.5 progress, got %v progress", progress) - } + assert.Equalf(t, 0.5, progress, "want 0.5 progress, got %v progress", progress) case returnedData := <-returnDataChannel: if returnedData.error != nil { t.Fatal(returnedData.error) } - if returnedData.int != 50 { - t.Fatalf("want 50 bytes written, got %v bytes written", returnedData.int) - } + assert.Equalf(t, 50, returnedData.int, "want 50 bytes written, got %v bytes written", returnedData.int) } } - if dw.downloadedSize != 50 { - t.Fatalf("want total witten size 50, got %v", dw.downloadedSize) - } - if len(dw.downloadedData) != 50 { - t.Fatalf("want 50 bytes of content, got %v bytes of content", len(dw.downloadedData)) - } + assert.Equalf(t, int64(50), dw.downloadedSize, "want total witten size 50, got %v", dw.downloadedSize) + assert.Equalf(t, 50, len(dw.downloadedData), "want 50 bytes of content, got %v bytes of content", len(dw.downloadedData)) } diff --git a/plugin/utils_test.go b/plugin/utils_test.go index 3f80be8..012c3f8 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/MTVersionManager/mtvm/config" "github.com/MTVersionManager/mtvm/shared" @@ -265,18 +266,18 @@ func TestRemoveExisting(t *testing.T) { fs := afero.NewMemMapFs() var err error shared.Configuration, err = config.GetConfig() - assert.Nilf(t, err, "want no error when getting configuration, got %v", err) + require.Nilf(t, err, "want no error when getting configuration, got %v", err) err = fs.MkdirAll(shared.Configuration.PluginDir, 0o777) - assert.Nilf(t, err, "want no error when creating plugin directory, got %v", err) + require.Nilf(t, err, "want no error when creating plugin directory, got %v", err) pluginPath := filepath.Join(shared.Configuration.PluginDir, "loremIpsum"+shared.LibraryExtension) _, err = fs.Create(pluginPath) - assert.Nilf(t, err, "want no error when creating plugin file, got %v", err) + require.Nilf(t, err, "want no error when creating plugin file, got %v", err) err = Remove("loremIpsum", fs) - assert.Nilf(t, err, "want no error, got %v", err) + require.Nilf(t, err, "want no error, got %v", err) _, err = fs.Stat(pluginPath) assert.NotNil(t, err, "want error, got nil (stat)") if !os.IsNotExist(err) { - t.Fatalf("want file does not exist error, got %v (stat)", err) + t.Errorf("want file does not exist error, got %v (stat)", err) } } @@ -291,26 +292,26 @@ func TestRemoveNonExistent(t *testing.T) { func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { configDir, err := config.GetConfigDir() - assert.Nilf(t, err, "want no error when getting config directory, got %v", err) + require.Nilf(t, err, "want no error when getting config directory, got %v", err) err = fs.MkdirAll(configDir, 0o666) - assert.Nilf(t, err, "want no error when creating config directory, got %v", err) + require.Nilf(t, err, "want no error when creating config directory, got %v", err) file, err := fs.Create(filepath.Join(configDir, "plugins.json")) - assert.Nilf(t, err, "want no error when creating plugins.json, got %v", err) + require.Nilf(t, err, "want no error when creating plugins.json, got %v", err) defer file.Close() _, err = file.Write(content) - assert.Nilf(t, err, "want no error when writing to plugins.json, got %v", err) + require.Nilf(t, err, "want no error when writing to plugins.json, got %v", err) } func readPluginsJson(t *testing.T, fs afero.Fs) []byte { configDir, err := config.GetConfigDir() - assert.Nilf(t, err, "want no error when getting config directory, got %v", err) + require.Nilf(t, err, "want no error when getting config directory, got %v", err) data, err := afero.ReadFile(fs, filepath.Join(configDir, "plugins.json")) - assert.Nilf(t, err, "want no error when reading plugins.json, got %v", err) + require.Nilf(t, err, "want no error when reading plugins.json, got %v", err) return data } func checkIfJsonSyntaxError(t *testing.T, err error) { - assert.NotNil(t, err, "want error, got nil") + require.NotNil(t, err, "want error, got nil") var syntaxError *json.SyntaxError assert.ErrorAsf(t, err, &syntaxError, "want JSON syntax error, got %v", err) } diff --git a/shared/errors_test.go b/shared/errors_test.go index 92227e5..5123b06 100644 --- a/shared/errors_test.go +++ b/shared/errors_test.go @@ -1,8 +1,9 @@ package shared import ( - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestNotFoundError_Error(t *testing.T) { @@ -13,7 +14,5 @@ func TestNotFoundError_Error(t *testing.T) { Function: "dolor", }, } - expected := "ipsum:dolor could not find lorem" - output := err.Error() - assert.Equal(t, expected, output) + assert.Equal(t, "ipsum:dolor could not find lorem", err.Error()) } diff --git a/shared/testUtils.go b/shared/testUtils.go index 27a089d..0f47787 100644 --- a/shared/testUtils.go +++ b/shared/testUtils.go @@ -5,7 +5,7 @@ import ( "testing" ) -func AssertIsNotFoundError(t *testing.T, err error, thing string, source Source) { +func AssertIsNotFoundError(t testing.TB, err error, thing string, source Source) { if err == nil { t.Fatal("want error, got nil") } diff --git a/shared/types_test.go b/shared/types_test.go index 5a8ab8c..bc82e10 100644 --- a/shared/types_test.go +++ b/shared/types_test.go @@ -1,6 +1,10 @@ package shared -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" +) func TestSource_String(t *testing.T) { source := Source{ @@ -9,7 +13,5 @@ func TestSource_String(t *testing.T) { } expected := "lorem:ipsum" output := source.String() - if output != expected { - t.Fatalf("want %v, got %v", expected, output) - } + assert.Equal(t, expected, output) } From 85dcd075cf0928b66f357f7d4480f90ae3a6b64a Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 14:44:57 -0400 Subject: [PATCH 07/22] Use testify for AssertIsNotFoundError --- shared/testUtils.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/shared/testUtils.go b/shared/testUtils.go index 0f47787..0a91760 100644 --- a/shared/testUtils.go +++ b/shared/testUtils.go @@ -1,7 +1,8 @@ package shared import ( - "errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "testing" ) @@ -10,14 +11,7 @@ func AssertIsNotFoundError(t testing.TB, err error, thing string, source Source) t.Fatal("want error, got nil") } var notFoundError NotFoundError - errors.As(err, ¬FoundError) - if !errors.As(err, ¬FoundError) { - t.Fatalf("want error to be NotFoundError, got error not containing NotFoundError") - } - if notFoundError.Thing != thing { - t.Fatalf("want error to contain thing %v, got %v", thing, notFoundError.Thing) - } - if notFoundError.Source != source { - t.Fatalf("want error to contain source %v, got %v", source, notFoundError.Source) - } + require.ErrorAs(t, err, ¬FoundError) + assert.Equalf(t, thing, notFoundError.Thing, "want error to contain thing %v, got %v", thing, notFoundError.Thing) + assert.Equalf(t, source, notFoundError.Source, "want error to contain source %v, got %v", source, notFoundError.Source) } From ac9814d7c0ad20c341d6eaf6541392d5e62f9685 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 14:45:38 -0400 Subject: [PATCH 08/22] Forgot to format --- components/downloader/downloader_test.go | 3 ++- shared/testUtils.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/downloader/downloader_test.go b/components/downloader/downloader_test.go index d45e37d..fb444b6 100644 --- a/components/downloader/downloader_test.go +++ b/components/downloader/downloader_test.go @@ -1,8 +1,9 @@ package downloader import ( - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestDownloadWriter_Write(t *testing.T) { diff --git a/shared/testUtils.go b/shared/testUtils.go index 0a91760..6e31780 100644 --- a/shared/testUtils.go +++ b/shared/testUtils.go @@ -1,9 +1,10 @@ package shared import ( + "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" ) func AssertIsNotFoundError(t testing.TB, err error, thing string, source Source) { From 37eb9146a1d8712250f11f9f674b1fc28eaeac7a Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 14:48:42 -0400 Subject: [PATCH 09/22] Use testify mock for TestingT in AssertIsNotFoundError --- go.mod | 1 + go.sum | 2 ++ shared/testUtils.go | 9 +++------ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 0852589..5ef69c7 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/cast v1.7.1 // indirect github.com/spf13/pflag v1.0.6 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect go.uber.org/multierr v1.11.0 // indirect diff --git a/go.sum b/go.sum index 15e6132..0b3f53b 100644 --- a/go.sum +++ b/go.sum @@ -93,6 +93,8 @@ github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.20.1 h1:ZMi+z/lvLyPSCoNtFCpqjy0S4kPbirhpTMwl8BkW9X4= github.com/spf13/viper v1.20.1/go.mod h1:P9Mdzt1zoHIG8m2eZQinpiBjo6kCmZSKBClNNqjJvu4= +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.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= diff --git a/shared/testUtils.go b/shared/testUtils.go index 6e31780..b198d5d 100644 --- a/shared/testUtils.go +++ b/shared/testUtils.go @@ -1,16 +1,13 @@ package shared import ( - "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -func AssertIsNotFoundError(t testing.TB, err error, thing string, source Source) { - if err == nil { - t.Fatal("want error, got nil") - } +func AssertIsNotFoundError(t mock.TestingT, err error, thing string, source Source) { + require.NotNil(t, err, "want not nil error, got nil") var notFoundError NotFoundError require.ErrorAs(t, err, ¬FoundError) assert.Equalf(t, thing, notFoundError.Thing, "want error to contain thing %v, got %v", thing, notFoundError.Thing) From f11e7d59d5f7af6e58f83e04a8ecb2a6c9cffb70 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:16:51 -0400 Subject: [PATCH 10/22] Tests for AssertIsNotFoundError --- shared/testUtils_test.go | 197 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 197 insertions(+) create mode 100644 shared/testUtils_test.go diff --git a/shared/testUtils_test.go b/shared/testUtils_test.go new file mode 100644 index 0000000..c965dcd --- /dev/null +++ b/shared/testUtils_test.go @@ -0,0 +1,197 @@ +package shared + +import ( + "errors" + "fmt" + "runtime" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +type mockTestingT struct { + mock.TestingT + logs []string + errors []string + failed bool +} + +func (m *mockTestingT) Logf(format string, args ...interface{}) { + m.logs = append(m.logs, fmt.Sprintf(format, args...)) +} + +func (m *mockTestingT) Errorf(format string, args ...interface{}) { + m.errors = append(m.errors, fmt.Sprintf(format, args...)) +} + +func (m *mockTestingT) FailNow() { + m.failed = true + runtime.Goexit() +} + +func newMockTestingT() *mockTestingT { + return &mockTestingT{ + logs: []string{}, + errors: []string{}, + failed: false, + } +} + +func TestAssertIsNotFoundError(t *testing.T) { + tests := map[string]struct { + err error + thing string + source Source + testFunc func(mockT *mockTestingT) + }{ + "matching": { + err: NotFoundError{ + Thing: "lorem", + Source: Source{ + File: "ipsum", + Function: "dolor", + }, + }, + thing: "lorem", + source: Source{ + File: "ipsum", + Function: "dolor", + }, + testFunc: func(mockT *mockTestingT) { + assert.False(t, mockT.failed, "want not failed, got failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assert.Len(t, mockT.errors, 0, "want no errors, got errors") + }, + }, + "thing mismatch": { + err: NotFoundError{ + Thing: "sit", + Source: Source{ + File: "ipsum", + Function: "dolor", + }, + }, + thing: "lorem", + source: Source{ + File: "ipsum", + Function: "dolor", + }, + testFunc: func(mockT *mockTestingT) { + assert.False(t, mockT.failed, "want not failed, got failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") + require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) + var expected string + { + mockForExpected := newMockTestingT() + assert.Equal(mockForExpected, "lorem", "sit", "want error to contain thing lorem, got sit") + require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) + expected = removeErrorTrace(mockForExpected.errors[0]) + } + assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }, + }, + "source mismatch": { + err: NotFoundError{ + Thing: "lorem", + Source: Source{ + File: "sit", + Function: "amet", + }, + }, + thing: "lorem", + source: Source{ + File: "ipsum", + Function: "dolor", + }, + testFunc: func(mockT *mockTestingT) { + assert.False(t, mockT.failed, "want not failed, got failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") + require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) + var expected string + { + mockForExpected := newMockTestingT() + expectedSource := Source{ + File: "ipsum", + Function: "dolor", + } + gotSource := Source{ + File: "sit", + Function: "amet", + } + assert.Equalf(mockForExpected, expectedSource, gotSource, "want error to contain source %v, got %v", expectedSource, gotSource) + require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) + expected = removeErrorTrace(mockForExpected.errors[0]) + } + assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }, + }, + "wrong error type": { + err: errors.New("loremIpsum"), + thing: "lorem", + source: Source{ + File: "ipsum", + Function: "dolor", + }, + testFunc: func(mockT *mockTestingT) { + assert.True(t, mockT.failed, "want failed, got not failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") + require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) + var expected string + { + mockForExpected := newMockTestingT() + assert.ErrorAs(mockForExpected, errors.New("loremIpsum"), &NotFoundError{}) + require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) + expected = removeErrorTrace(mockForExpected.errors[0]) + } + assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }, + }, + "nil error": { + err: nil, + thing: "lorem", + source: Source{ + File: "ipsum", + Function: "dolor", + }, + testFunc: func(mockT *mockTestingT) { + assert.True(t, mockT.failed, "want failed, got not failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") + require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) + var expected string + { + mockForExpected := newMockTestingT() + assert.NotNil(mockForExpected, nil, "want not nil error, got nil") + require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) + expected = removeErrorTrace(mockForExpected.errors[0]) + } + assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + mockT := newMockTestingT() + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + AssertIsNotFoundError(mockT, tt.err, tt.thing, tt.source) + }() + wg.Wait() + // go AssertIsNotFoundError(mockT, tt.err, tt.thing, tt.source) + tt.testFunc(mockT) + }) + } +} + +func removeErrorTrace(error string) string { + if i := strings.Index(error, "Error:"); i != -1 { + return strings.TrimSpace(error[i:]) + } + return error +} From 1986313952ba3ac0939b71ec3854db0ca2d2f56d Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:26:19 -0400 Subject: [PATCH 11/22] Reduce duplication in tests for AssertIsNotFoundError part 1 --- shared/testUtils_test.go | 78 ++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 51 deletions(-) diff --git a/shared/testUtils_test.go b/shared/testUtils_test.go index c965dcd..bfda8a7 100644 --- a/shared/testUtils_test.go +++ b/shared/testUtils_test.go @@ -43,6 +43,14 @@ func newMockTestingT() *mockTestingT { } func TestAssertIsNotFoundError(t *testing.T) { + normalTestSource := Source{ + File: "ipsum", + Function: "dolor", + } + alteredTestSource := Source{ + File: "sit", + Function: "amet", + } tests := map[string]struct { err error thing string @@ -51,17 +59,11 @@ func TestAssertIsNotFoundError(t *testing.T) { }{ "matching": { err: NotFoundError{ - Thing: "lorem", - Source: Source{ - File: "ipsum", - Function: "dolor", - }, - }, - thing: "lorem", - source: Source{ - File: "ipsum", - Function: "dolor", + Thing: "lorem", + Source: normalTestSource, }, + thing: "lorem", + source: normalTestSource, testFunc: func(mockT *mockTestingT) { assert.False(t, mockT.failed, "want not failed, got failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") @@ -70,17 +72,11 @@ func TestAssertIsNotFoundError(t *testing.T) { }, "thing mismatch": { err: NotFoundError{ - Thing: "sit", - Source: Source{ - File: "ipsum", - Function: "dolor", - }, - }, - thing: "lorem", - source: Source{ - File: "ipsum", - Function: "dolor", + Thing: "sit", + Source: normalTestSource, }, + thing: "lorem", + source: normalTestSource, testFunc: func(mockT *mockTestingT) { assert.False(t, mockT.failed, "want not failed, got failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") @@ -97,17 +93,11 @@ func TestAssertIsNotFoundError(t *testing.T) { }, "source mismatch": { err: NotFoundError{ - Thing: "lorem", - Source: Source{ - File: "sit", - Function: "amet", - }, - }, - thing: "lorem", - source: Source{ - File: "ipsum", - Function: "dolor", + Thing: "lorem", + Source: alteredTestSource, }, + thing: "lorem", + source: normalTestSource, testFunc: func(mockT *mockTestingT) { assert.False(t, mockT.failed, "want not failed, got failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") @@ -115,15 +105,7 @@ func TestAssertIsNotFoundError(t *testing.T) { var expected string { mockForExpected := newMockTestingT() - expectedSource := Source{ - File: "ipsum", - Function: "dolor", - } - gotSource := Source{ - File: "sit", - Function: "amet", - } - assert.Equalf(mockForExpected, expectedSource, gotSource, "want error to contain source %v, got %v", expectedSource, gotSource) + assert.Equalf(mockForExpected, normalTestSource, alteredTestSource, "want error to contain source %v, got %v", normalTestSource, alteredTestSource) require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) expected = removeErrorTrace(mockForExpected.errors[0]) } @@ -131,12 +113,9 @@ func TestAssertIsNotFoundError(t *testing.T) { }, }, "wrong error type": { - err: errors.New("loremIpsum"), - thing: "lorem", - source: Source{ - File: "ipsum", - Function: "dolor", - }, + err: errors.New("loremIpsum"), + thing: "lorem", + source: normalTestSource, testFunc: func(mockT *mockTestingT) { assert.True(t, mockT.failed, "want failed, got not failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") @@ -152,12 +131,9 @@ func TestAssertIsNotFoundError(t *testing.T) { }, }, "nil error": { - err: nil, - thing: "lorem", - source: Source{ - File: "ipsum", - Function: "dolor", - }, + err: nil, + thing: "lorem", + source: normalTestSource, testFunc: func(mockT *mockTestingT) { assert.True(t, mockT.failed, "want failed, got not failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") From d7f25c2b2ffd5276e0de9191f95dac930bc0a284 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:42:39 -0400 Subject: [PATCH 12/22] Reduce duplication in tests for AssertIsNotFoundError part 2 --- shared/testUtils_test.go | 48 ++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/shared/testUtils_test.go b/shared/testUtils_test.go index bfda8a7..929dd61 100644 --- a/shared/testUtils_test.go +++ b/shared/testUtils_test.go @@ -80,15 +80,9 @@ func TestAssertIsNotFoundError(t *testing.T) { testFunc: func(mockT *mockTestingT) { assert.False(t, mockT.failed, "want not failed, got failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") - require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) - var expected string - { - mockForExpected := newMockTestingT() + getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.Equal(mockForExpected, "lorem", "sit", "want error to contain thing lorem, got sit") - require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) - expected = removeErrorTrace(mockForExpected.errors[0]) - } - assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }) }, }, "source mismatch": { @@ -101,15 +95,9 @@ func TestAssertIsNotFoundError(t *testing.T) { testFunc: func(mockT *mockTestingT) { assert.False(t, mockT.failed, "want not failed, got failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") - require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) - var expected string - { - mockForExpected := newMockTestingT() + getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.Equalf(mockForExpected, normalTestSource, alteredTestSource, "want error to contain source %v, got %v", normalTestSource, alteredTestSource) - require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) - expected = removeErrorTrace(mockForExpected.errors[0]) - } - assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }) }, }, "wrong error type": { @@ -119,15 +107,9 @@ func TestAssertIsNotFoundError(t *testing.T) { testFunc: func(mockT *mockTestingT) { assert.True(t, mockT.failed, "want failed, got not failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") - require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) - var expected string - { - mockForExpected := newMockTestingT() + getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.ErrorAs(mockForExpected, errors.New("loremIpsum"), &NotFoundError{}) - require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) - expected = removeErrorTrace(mockForExpected.errors[0]) - } - assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }) }, }, "nil error": { @@ -137,15 +119,9 @@ func TestAssertIsNotFoundError(t *testing.T) { testFunc: func(mockT *mockTestingT) { assert.True(t, mockT.failed, "want failed, got not failed") assert.Len(t, mockT.logs, 0, "want no logs, got logs") - require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) - var expected string - { - mockForExpected := newMockTestingT() + getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.NotNil(mockForExpected, nil, "want not nil error, got nil") - require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) - expected = removeErrorTrace(mockForExpected.errors[0]) - } - assert.Equal(t, expected, removeErrorTrace(mockT.errors[0]), "unexpected error") + }) }, }, } @@ -171,3 +147,11 @@ func removeErrorTrace(error string) string { } return error } + +func getExpectedErrorAndCompare(t *testing.T, mockT mockTestingT, function func(mockForExpected *mockTestingT)) { + require.Lenf(t, mockT.errors, 1, "want 1 error, got %v errors", len(mockT.errors)) + mockForExpected := newMockTestingT() + function(mockForExpected) + require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) + assert.Equal(t, removeErrorTrace(mockForExpected.errors[0]), removeErrorTrace(mockT.errors[0]), "unexpected error") +} From 0d5dfd74755d07602d4c3af36f1ffed31a7bd3c6 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 25 Jun 2025 17:57:17 -0400 Subject: [PATCH 13/22] Reduce duplication part 3 --- shared/testUtils_test.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/shared/testUtils_test.go b/shared/testUtils_test.go index 929dd61..6b5659d 100644 --- a/shared/testUtils_test.go +++ b/shared/testUtils_test.go @@ -65,8 +65,7 @@ func TestAssertIsNotFoundError(t *testing.T) { thing: "lorem", source: normalTestSource, testFunc: func(mockT *mockTestingT) { - assert.False(t, mockT.failed, "want not failed, got failed") - assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assertNotFailedNoLogs(t, *mockT) assert.Len(t, mockT.errors, 0, "want no errors, got errors") }, }, @@ -78,8 +77,7 @@ func TestAssertIsNotFoundError(t *testing.T) { thing: "lorem", source: normalTestSource, testFunc: func(mockT *mockTestingT) { - assert.False(t, mockT.failed, "want not failed, got failed") - assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assertNotFailedNoLogs(t, *mockT) getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.Equal(mockForExpected, "lorem", "sit", "want error to contain thing lorem, got sit") }) @@ -93,8 +91,7 @@ func TestAssertIsNotFoundError(t *testing.T) { thing: "lorem", source: normalTestSource, testFunc: func(mockT *mockTestingT) { - assert.False(t, mockT.failed, "want not failed, got failed") - assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assertNotFailedNoLogs(t, *mockT) getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.Equalf(mockForExpected, normalTestSource, alteredTestSource, "want error to contain source %v, got %v", normalTestSource, alteredTestSource) }) @@ -105,8 +102,7 @@ func TestAssertIsNotFoundError(t *testing.T) { thing: "lorem", source: normalTestSource, testFunc: func(mockT *mockTestingT) { - assert.True(t, mockT.failed, "want failed, got not failed") - assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assertFailedNoLogs(t, *mockT) getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.ErrorAs(mockForExpected, errors.New("loremIpsum"), &NotFoundError{}) }) @@ -117,8 +113,7 @@ func TestAssertIsNotFoundError(t *testing.T) { thing: "lorem", source: normalTestSource, testFunc: func(mockT *mockTestingT) { - assert.True(t, mockT.failed, "want failed, got not failed") - assert.Len(t, mockT.logs, 0, "want no logs, got logs") + assertFailedNoLogs(t, *mockT) getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { assert.NotNil(mockForExpected, nil, "want not nil error, got nil") }) @@ -127,6 +122,7 @@ func TestAssertIsNotFoundError(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { + t.Parallel() mockT := newMockTestingT() var wg sync.WaitGroup wg.Add(1) @@ -155,3 +151,13 @@ func getExpectedErrorAndCompare(t *testing.T, mockT mockTestingT, function func( require.Lenf(t, mockForExpected.errors, 1, "want 1 error when getting expected, got %v errors", len(mockForExpected.errors)) assert.Equal(t, removeErrorTrace(mockForExpected.errors[0]), removeErrorTrace(mockT.errors[0]), "unexpected error") } + +func assertNotFailedNoLogs(t *testing.T, mockT mockTestingT) { + assert.False(t, mockT.failed, "want not failed, got failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") +} + +func assertFailedNoLogs(t *testing.T, mockT mockTestingT) { + assert.True(t, mockT.failed, "want failed, got not failed") + assert.Len(t, mockT.logs, 0, "want no logs, got logs") +} From 3a0800bf3ba17304d6d061aea5a726d7bdf2dee6 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 13:27:07 -0400 Subject: [PATCH 14/22] Some tests for startDownload and other changes --- cmd/plugincmds/install_test.go | 31 +++---- components/downloader/downloader.go | 3 +- components/downloader/downloader_test.go | 104 +++++++++++++++++++++++ 3 files changed, 118 insertions(+), 20 deletions(-) diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index 5860474..8dac701 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -21,18 +21,19 @@ func TestGetPluginInfo(t *testing.T) { metadata plugin.Metadata testFunc func(t *testing.T, msg tea.Msg) } + exampleUsableDownloads := []plugin.Download{ + { + OS: runtime.GOOS, + Arch: runtime.GOARCH, + Url: "https://example.com", + }, + } tests := map[string]test{ "existing download": { metadata: plugin.Metadata{ - Name: "loremIpsum", - Version: "0.0.0", - Downloads: []plugin.Download{ - { - OS: runtime.GOOS, - Arch: runtime.GOARCH, - Url: "https://example.com", - }, - }, + Name: "loremIpsum", + Version: "0.0.0", + Downloads: exampleUsableDownloads, }, testFunc: func(t *testing.T, msg tea.Msg) { if downloadInfo, ok := msg.(pluginDownloadInfo); ok { @@ -84,15 +85,9 @@ func TestGetPluginInfo(t *testing.T) { }, "invalid version": { metadata: plugin.Metadata{ - Name: "loremIpsum", - Version: "IAmAnInvalidVersion", - Downloads: []plugin.Download{ - { - OS: runtime.GOOS, - Arch: runtime.GOARCH, - Url: "https://example.com", - }, - }, + Name: "loremIpsum", + Version: "IAmAnInvalidVersion", + Downloads: exampleUsableDownloads, }, testFunc: func(t *testing.T, msg tea.Msg) { if err, ok := msg.(error); ok { diff --git a/components/downloader/downloader.go b/components/downloader/downloader.go index 60739b2..7348a35 100644 --- a/components/downloader/downloader.go +++ b/components/downloader/downloader.go @@ -43,8 +43,7 @@ func (dw *downloadWriter) Start() { // This sends a signal to the update function that it is safe to close the response body dw.copyDone <- true if err != nil && !errors.Is(err, context.Canceled) { - fmt.Println("Error from copying") - log.Fatal(err) + log.Fatal(fmt.Errorf("from copying: %v", err)) } } diff --git a/components/downloader/downloader_test.go b/components/downloader/downloader_test.go index fb444b6..df861d3 100644 --- a/components/downloader/downloader_test.go +++ b/components/downloader/downloader_test.go @@ -1,11 +1,19 @@ package downloader import ( + "net/http" + "net/http/httptest" "testing" + "github.com/stretchr/testify/require" + + tea "github.com/charmbracelet/bubbletea" + "github.com/stretchr/testify/assert" ) +var hundredByteLongLoremIpsum = []byte("Lorem ipsum dolor sit amet consectetur adipiscing elit. Quisque faucibus ex sapien vitae pellentesqu") + func TestDownloadWriter_Write(t *testing.T) { dw := downloadWriter{ totalSize: 100, @@ -37,3 +45,99 @@ func TestDownloadWriter_Write(t *testing.T) { assert.Equalf(t, int64(50), dw.downloadedSize, "want total witten size 50, got %v", dw.downloadedSize) assert.Equalf(t, 50, len(dw.downloadedData), "want 50 bytes of content, got %v bytes of content", len(dw.downloadedData)) } + +func TestModel_StartDownload(t *testing.T) { + tests := map[string]struct { + headers map[string]string + options []Option + statusCode int + testFuncBeforeFinish func(t *testing.T, model Model, msg tea.Msg) + testFuncAfterFinish func(t *testing.T, model Model, msg tea.Msg) + shouldDownload bool + chunked bool + }{ + "content length present status 200": { + headers: map[string]string{ + "Content-Length": "100", + }, + statusCode: 200, + testFuncBeforeFinish: func(t *testing.T, model Model, msg tea.Msg) { + if err, ok := msg.(error); ok { + assert.NoError(t, err) + } + assert.IsType(t, DownloadStartedMsg{}, msg) + downloadStartedMsg := msg.(DownloadStartedMsg) + assert.True(t, downloadStartedMsg.contentLengthKnown, "want content length to be known, got not known") + assert.EqualValuesf(t, 100, model.writer.totalSize, "want 100 bytes total size, got %v bytes total size", model.writer.totalSize) + }, + testFuncAfterFinish: func(t *testing.T, model Model, msg tea.Msg) { + assert.Equal(t, hundredByteLongLoremIpsum, model.writer.downloadedData) + }, + shouldDownload: true, + chunked: false, + }, + "content length not present status 200": { + statusCode: 200, + shouldDownload: true, + chunked: true, + headers: map[string]string{ + "Transfer-Encoding": "chunked", + }, + testFuncBeforeFinish: func(t *testing.T, model Model, msg tea.Msg) { + if err, ok := msg.(error); ok { + assert.NoError(t, err) + } + assert.IsType(t, DownloadStartedMsg{}, msg) + downloadStartedMsg := msg.(DownloadStartedMsg) + assert.False(t, downloadStartedMsg.contentLengthKnown, "want content length to be not known, got known") + }, + testFuncAfterFinish: func(t *testing.T, model Model, msg tea.Msg) { + assert.Equal(t, hundredByteLongLoremIpsum, model.writer.downloadedData) + }, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + for header, value := range tt.headers { + writer.Header().Set(header, value) + } + writer.WriteHeader(tt.statusCode) + var written int + if tt.chunked { + writtenChunk1, err := writer.Write(hundredByteLongLoremIpsum[:50]) + assert.NoError(t, err) + assert.Equalf(t, 50, writtenChunk1, "want 50 bytes written from server for chunk 1, got %v bytes written", writtenChunk1) + written += writtenChunk1 + writtenChunk2, err := writer.Write(hundredByteLongLoremIpsum[50:]) + assert.NoError(t, err) + assert.Equalf(t, 50, writtenChunk2, "want 50 bytes written from server for chunk 2, got %v bytes written", writtenChunk2) + written += writtenChunk2 + } else { + var err error + written, err = writer.Write(hundredByteLongLoremIpsum) + assert.NoError(t, err) + } + assert.Equalf(t, 100, written, "want 100 bytes written from server, got %v bytes written", written) + })) + defer server.Close() + model := New(server.URL, tt.options...) + msg := model.startDownload() + require.NotNil(t, tt.testFuncBeforeFinish) + tt.testFuncBeforeFinish(t, model, msg) + if tt.shouldDownload { + go func() { + for range model.writer.progressChannel { + <-model.writer.progressChannel + } + }() + waitForResponseFinish(model.writer.copyDone)() + err := model.writer.resp.Body.Close() + assert.NoErrorf(t, err, "want no error closing response body, got %v", err) + assert.EqualValuesf(t, 100, model.writer.downloadedSize, "want 100 bytes downloaded, got %v bytes downloaded", model.writer.downloadedSize) + require.NotNil(t, tt.testFuncAfterFinish) + tt.testFuncAfterFinish(t, model, msg) + } + }) + } +} From 3457e8f2b29910efaf8ba652e44f2133fc5d1aaf Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 13:46:40 -0400 Subject: [PATCH 15/22] Use testify Error and NoError functions when appropriate --- cmd/plugincmds/install_test.go | 2 +- components/downloader/downloader_test.go | 4 +-- plugin/utils_test.go | 42 ++++++++++++------------ shared/testUtils.go | 2 +- shared/testUtils_test.go | 2 +- 5 files changed, 25 insertions(+), 27 deletions(-) diff --git a/cmd/plugincmds/install_test.go b/cmd/plugincmds/install_test.go index 8dac701..93e0909 100644 --- a/cmd/plugincmds/install_test.go +++ b/cmd/plugincmds/install_test.go @@ -44,7 +44,7 @@ func TestGetPluginInfo(t *testing.T) { t.Errorf("Want version 0.0.0 got %v", downloadInfo.Version.String()) } } else if err, ok := msg.(error); ok { - t.Errorf("want no error, got %v", err) + assert.NoError(t, err) } else { t.Errorf("want pluginDownloadInfo returned, got %T with content %v", msg, msg) } diff --git a/components/downloader/downloader_test.go b/components/downloader/downloader_test.go index df861d3..6400fef 100644 --- a/components/downloader/downloader_test.go +++ b/components/downloader/downloader_test.go @@ -36,9 +36,7 @@ func TestDownloadWriter_Write(t *testing.T) { case progress := <-dw.progressChannel: assert.Equalf(t, 0.5, progress, "want 0.5 progress, got %v progress", progress) case returnedData := <-returnDataChannel: - if returnedData.error != nil { - t.Fatal(returnedData.error) - } + assert.NoError(t, returnedData.error) assert.Equalf(t, 50, returnedData.int, "want 50 bytes written, got %v bytes written", returnedData.int) } } diff --git a/plugin/utils_test.go b/plugin/utils_test.go index 012c3f8..2f580e7 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -77,7 +77,7 @@ func TestInstalledVersionWithPluginsJson(t *testing.T) { pluginsJsonContent: []byte(oneEntryJson), pluginName: "loremIpsum", testFunc: func(t *testing.T, version string, err error) { - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) assert.Equal(t, "0.0.0", version) }, }, @@ -100,7 +100,7 @@ func TestAddFirstEntryNoPluginsJson(t *testing.T) { Version: "0.0.0", MetadataUrl: "https://example.com", }, fs) - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) data := readPluginsJson(t, fs) assert.Equal(t, oneEntryJson, string(data)) } @@ -153,10 +153,10 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { createAndWritePluginsJson(t, tt.pluginsJsonContent, fs) err := UpdateEntries(tt.entry, fs) if tt.wantsError { - assert.NotNil(t, err, "want error, got nil") + assert.Error(t, err) } if !tt.wantsError { - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) } tt.testFunc(t, fs, err) }) @@ -166,7 +166,7 @@ func TestUpdateEntryWithPluginsJson(t *testing.T) { func TestGetEntriesWithNoPluginsJson(t *testing.T) { fs := afero.NewMemMapFs() entries, err := GetEntries(fs) - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) assert.Nilf(t, entries, "want entries to be nil, got %v", err) } @@ -178,14 +178,14 @@ func TestGetEntriesWithPluginsJson(t *testing.T) { "empty": { pluginsJsonContent: []byte(`[]`), testFunc: func(t *testing.T, entries []Entry, err error) { - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) assert.Lenf(t, entries, 0, "want entries to be empty, got %v", entries) }, }, "two entries": { pluginsJsonContent: []byte(twoEntryJson), testFunc: func(t *testing.T, entries []Entry, err error) { - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) assert.Len(t, entries, 2, "want 2 entries") assert.Equalf(t, "loremIpsum", entries[0].Name, "want first entry name to be 'loremIpsum', got %v", entries[0].Name) assert.Equalf(t, "dolorSitAmet", entries[1].Name, "want second entry name to be 'dolorSitAmet', got %v", entries[1].Name) @@ -228,7 +228,7 @@ func TestRemoveEntryWithPluginsJson(t *testing.T) { pluginToRemove: "dolorSitAmet", pluginsJsonContent: []byte(twoEntryJson), testFunc: func(t *testing.T, fs afero.Fs, err error) { - assert.Nilf(t, err, "want no error, got %v", err) + assert.NoError(t, err) data := readPluginsJson(t, fs) assert.Equal(t, oneEntryJson, string(data)) }, @@ -266,16 +266,16 @@ func TestRemoveExisting(t *testing.T) { fs := afero.NewMemMapFs() var err error shared.Configuration, err = config.GetConfig() - require.Nilf(t, err, "want no error when getting configuration, got %v", err) + require.NoError(t, err, "when getting configuration") err = fs.MkdirAll(shared.Configuration.PluginDir, 0o777) - require.Nilf(t, err, "want no error when creating plugin directory, got %v", err) + require.NoError(t, err, "when creating plugin directory") pluginPath := filepath.Join(shared.Configuration.PluginDir, "loremIpsum"+shared.LibraryExtension) _, err = fs.Create(pluginPath) - require.Nilf(t, err, "want no error when creating plugin file, got %v", err) + require.NoError(t, err, "when creating plugin file") err = Remove("loremIpsum", fs) - require.Nilf(t, err, "want no error, got %v", err) + require.NoError(t, err) _, err = fs.Stat(pluginPath) - assert.NotNil(t, err, "want error, got nil (stat)") + assert.Error(t, err, "when statting plugin file") if !os.IsNotExist(err) { t.Errorf("want file does not exist error, got %v (stat)", err) } @@ -292,26 +292,26 @@ func TestRemoveNonExistent(t *testing.T) { func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { configDir, err := config.GetConfigDir() - require.Nilf(t, err, "want no error when getting config directory, got %v", err) + require.NoError(t, err, "when getting config directory") err = fs.MkdirAll(configDir, 0o666) - require.Nilf(t, err, "want no error when creating config directory, got %v", err) + require.NoError(t, err, "when creating config directory") file, err := fs.Create(filepath.Join(configDir, "plugins.json")) - require.Nilf(t, err, "want no error when creating plugins.json, got %v", err) + require.NoError(t, err, "when creating plugins.json") defer file.Close() _, err = file.Write(content) - require.Nilf(t, err, "want no error when writing to plugins.json, got %v", err) + require.NoError(t, err, "when writing to plugins.json") } func readPluginsJson(t *testing.T, fs afero.Fs) []byte { configDir, err := config.GetConfigDir() - require.Nilf(t, err, "want no error when getting config directory, got %v", err) + require.NoError(t, err, "when getting config directory") data, err := afero.ReadFile(fs, filepath.Join(configDir, "plugins.json")) - require.Nilf(t, err, "want no error when reading plugins.json, got %v", err) + require.NoError(t, err, "when reading plugins.json") return data } func checkIfJsonSyntaxError(t *testing.T, err error) { - require.NotNil(t, err, "want error, got nil") + require.Error(t, err) var syntaxError *json.SyntaxError - assert.ErrorAsf(t, err, &syntaxError, "want JSON syntax error, got %v", err) + assert.ErrorAs(t, err, &syntaxError) } diff --git a/shared/testUtils.go b/shared/testUtils.go index b198d5d..8aa4ecc 100644 --- a/shared/testUtils.go +++ b/shared/testUtils.go @@ -7,7 +7,7 @@ import ( ) func AssertIsNotFoundError(t mock.TestingT, err error, thing string, source Source) { - require.NotNil(t, err, "want not nil error, got nil") + require.Error(t, err) var notFoundError NotFoundError require.ErrorAs(t, err, ¬FoundError) assert.Equalf(t, thing, notFoundError.Thing, "want error to contain thing %v, got %v", thing, notFoundError.Thing) diff --git a/shared/testUtils_test.go b/shared/testUtils_test.go index 6b5659d..c4f8da8 100644 --- a/shared/testUtils_test.go +++ b/shared/testUtils_test.go @@ -115,7 +115,7 @@ func TestAssertIsNotFoundError(t *testing.T) { testFunc: func(mockT *mockTestingT) { assertFailedNoLogs(t, *mockT) getExpectedErrorAndCompare(t, *mockT, func(mockForExpected *mockTestingT) { - assert.NotNil(mockForExpected, nil, "want not nil error, got nil") + assert.Error(mockForExpected, nil) }) }, }, From 82e5d32427fbb640f298f124564765cd369d2a7c Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 14:14:16 -0400 Subject: [PATCH 16/22] Invalid URL and no content length tests, and switch content length check to a switch statement --- components/downloader/downloader.go | 17 +++++++---- components/downloader/downloader_test.go | 39 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/components/downloader/downloader.go b/components/downloader/downloader.go index 7348a35..cb0fa4a 100644 --- a/components/downloader/downloader.go +++ b/components/downloader/downloader.go @@ -131,13 +131,18 @@ func (m Model) startDownload() tea.Msg { return fmt.Errorf("%v %v", resp.StatusCode, http.StatusText(resp.StatusCode)) } contentLengthKnown := true - if resp.ContentLength <= 0 { - if resp.ContentLength == -1 { - contentLengthKnown = false - } else { - cancel() - return errors.New("error when getting content length") + switch resp.ContentLength { + case -1: + contentLengthKnown = false + case 0: + cancel() + return errors.New("content length is 0") + default: + if resp.ContentLength > 0 { + break } + cancel() + return errors.New("unexpected error when getting content length") } m.writer.totalSize = resp.ContentLength m.writer.resp = resp diff --git a/components/downloader/downloader_test.go b/components/downloader/downloader_test.go index 6400fef..e1bdba3 100644 --- a/components/downloader/downloader_test.go +++ b/components/downloader/downloader_test.go @@ -3,6 +3,7 @@ package downloader import ( "net/http" "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/require" @@ -93,6 +94,19 @@ func TestModel_StartDownload(t *testing.T) { assert.Equal(t, hundredByteLongLoremIpsum, model.writer.downloadedData) }, }, + "status 400": { + statusCode: 400, + shouldDownload: false, + chunked: false, + testFuncBeforeFinish: func(t *testing.T, model Model, msg tea.Msg) { + if err, ok := msg.(error); ok { + assert.Error(t, err) + assert.EqualError(t, err, "400 Bad Request") + } else { + t.Errorf("want error, got %T with content %v", msg, msg) + } + }, + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { @@ -138,4 +152,29 @@ func TestModel_StartDownload(t *testing.T) { } }) } + t.Run("invalid url", func(t *testing.T) { + model := New("loremIpsum") + msg := model.startDownload() + if err, ok := msg.(error); ok { + assert.Error(t, err) + var urlError *url.Error + assert.ErrorAs(t, err, &urlError) + } else { + t.Errorf("want error, got %T with content %v", msg, msg) + } + }) + t.Run("no content", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + written, err := writer.Write([]byte{}) + assert.NoError(t, err) + assert.Equalf(t, 0, written, "want 0 bytes written from server, got %v bytes written", written) + })) + defer server.Close() + model := New(server.URL) + msg := model.startDownload() + if err, ok := msg.(error); ok { + assert.Error(t, err) + assert.EqualError(t, err, "content length is 0") + } + }) } From a4abf829f3fe48008e07550cd765ee9e7d3139b1 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 14:33:58 -0400 Subject: [PATCH 17/22] Make shared.IsVersionInstalled use afero --- cmd/install.go | 5 +++-- cmd/remove.go | 4 +++- cmd/use.go | 2 +- shared/utils.go | 5 +++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/install.go b/cmd/install.go index 84594f9..48f34de 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -63,7 +63,8 @@ If you run "mtvm install go latest" it will install the latest version of go`, Args: cobra.ExactArgs(2), Aliases: []string{"i", "in"}, Run: func(cmd *cobra.Command, args []string) { - err := createInstallDir(afero.NewOsFs()) + fs := afero.NewOsFs() + err := createInstallDir(fs) if err != nil { log.Fatal(err) } @@ -79,7 +80,7 @@ If you run "mtvm install go latest" it will install the latest version of go`, log.Fatal(err) } } - installed, err := shared.IsVersionInstalled(args[0], version) + installed, err := shared.IsVersionInstalled(args[0], version, fs) if err != nil { log.Fatal(err) } diff --git a/cmd/remove.go b/cmd/remove.go index 1c0a731..4d252b8 100644 --- a/cmd/remove.go +++ b/cmd/remove.go @@ -2,6 +2,7 @@ package cmd import ( "fmt" + "github.com/spf13/afero" "log" "os" "path/filepath" @@ -94,7 +95,8 @@ For example: log.Fatal(err) } } - installed, err := shared.IsVersionInstalled(args[0], version) + fs := afero.NewOsFs() + installed, err := shared.IsVersionInstalled(args[0], version, fs) if err != nil { log.Fatal(err) } diff --git a/cmd/use.go b/cmd/use.go index 07357d0..3e22819 100644 --- a/cmd/use.go +++ b/cmd/use.go @@ -117,7 +117,7 @@ So if you run go version it will print the version number 1.23.3`, log.Fatal(err) } } - versionInstalled, err := shared.IsVersionInstalled(args[0], version) + versionInstalled, err := shared.IsVersionInstalled(args[0], version, fs) if err != nil { log.Fatal(err) } diff --git a/shared/utils.go b/shared/utils.go index cdc3207..39f9738 100644 --- a/shared/utils.go +++ b/shared/utils.go @@ -2,14 +2,15 @@ package shared import ( "errors" + "github.com/spf13/afero" "os" "path/filepath" "github.com/MTVersionManager/mtvmplugin" ) -func IsVersionInstalled(tool, version string) (bool, error) { - _, err := os.Stat(filepath.Join(Configuration.InstallDir, tool, version)) +func IsVersionInstalled(tool, version string, fs afero.Fs) (bool, error) { + _, err := fs.Stat(filepath.Join(Configuration.InstallDir, tool, version)) if err != nil { if os.IsNotExist(err) { return false, nil From 0c2d9aa57cad7c51c77573b94023f79d811ef24e Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 15:41:23 -0400 Subject: [PATCH 18/22] Format and tests for IsVersionInstalled --- cmd/remove.go | 3 +- shared/utils.go | 3 +- shared/utils_test.go | 95 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 shared/utils_test.go diff --git a/cmd/remove.go b/cmd/remove.go index 4d252b8..bcdad52 100644 --- a/cmd/remove.go +++ b/cmd/remove.go @@ -2,11 +2,12 @@ package cmd import ( "fmt" - "github.com/spf13/afero" "log" "os" "path/filepath" + "github.com/spf13/afero" + "github.com/MTVersionManager/mtvm/shared" "github.com/MTVersionManager/mtvmplugin" "github.com/charmbracelet/bubbles/spinner" diff --git a/shared/utils.go b/shared/utils.go index 39f9738..38805e4 100644 --- a/shared/utils.go +++ b/shared/utils.go @@ -2,10 +2,11 @@ package shared import ( "errors" - "github.com/spf13/afero" "os" "path/filepath" + "github.com/spf13/afero" + "github.com/MTVersionManager/mtvmplugin" ) diff --git a/shared/utils_test.go b/shared/utils_test.go new file mode 100644 index 0000000..078e238 --- /dev/null +++ b/shared/utils_test.go @@ -0,0 +1,95 @@ +package shared + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/stretchr/testify/assert" + + "github.com/spf13/afero" + "github.com/spf13/viper" +) + +type mockMemMapFs struct { + *afero.MemMapFs + openError error +} + +func (m *mockMemMapFs) Stat(name string) (os.FileInfo, error) { + if m.openError != nil { + return nil, m.openError + } + return m.MemMapFs.Stat(name) +} + +func newMockMemMapFs(openError error) afero.Fs { + return &mockMemMapFs{ + MemMapFs: &afero.MemMapFs{}, + openError: openError, + } +} + +func TestIsVersionInstalled(t *testing.T) { + viper.Reset() + viper.Set("installDir", "/test/install/") + err := viper.Unmarshal(&Configuration) + assert.NoError(t, err) + tests := map[string]struct { + tool, version string + fs afero.Fs + want bool + errorCheck func(t *testing.T, err error) + }{ + "installed": { + tool: "test", + version: "0.0.0", + want: true, + errorCheck: func(t *testing.T, err error) { + assert.NoError(t, err) + }, + fs: func() afero.Fs { + fs := afero.NewMemMapFs() + err := fs.MkdirAll("/test/install/test/0.0.0", 0o777) + assert.NoError(t, err) + return fs + }(), + }, + "not installed": { + tool: "test", + version: "0.0.0", + want: false, + errorCheck: func(t *testing.T, err error) { + assert.NoError(t, err) + }, + fs: afero.NewMemMapFs(), + }, + "permission error": { + tool: "test", + version: "0.0.0", + want: false, + errorCheck: func(t *testing.T, err error) { + assert.Error(t, err) + if !os.IsPermission(err) { + t.Errorf("want permission error, got %v", err) + } + }, + fs: func() afero.Fs { + fs := newMockMemMapFs(os.ErrPermission) + err := fs.MkdirAll("/test/install/test/0.0.0", 0o000) + assert.NoError(t, err) + return fs + }(), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + require.NotNil(t, tt.fs) + installed, err := IsVersionInstalled(tt.tool, tt.version, tt.fs) + tt.errorCheck(t, err) + assert.Equal(t, tt.want, installed) + }) + } +} From b6d8fad6416bba9f4ec83de78db6343292c58d49 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 15:48:52 -0400 Subject: [PATCH 19/22] Tests for IsNotFound --- shared/utils.go | 3 +-- shared/utils_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/shared/utils.go b/shared/utils.go index 38805e4..fa341f6 100644 --- a/shared/utils.go +++ b/shared/utils.go @@ -32,6 +32,5 @@ func LoadPlugin(tool string) (mtvmplugin.Plugin, error) { } func IsNotFound(err error) bool { - var notFound NotFoundError - return errors.As(err, ¬Found) + return errors.As(err, &NotFoundError{}) } diff --git a/shared/utils_test.go b/shared/utils_test.go index 078e238..d7ddb93 100644 --- a/shared/utils_test.go +++ b/shared/utils_test.go @@ -93,3 +93,35 @@ func TestIsVersionInstalled(t *testing.T) { }) } } + +func TestIsNotFound(t *testing.T) { + tests := map[string]struct { + err error + want bool + }{ + "not found": { + err: NotFoundError{ + Thing: "lorem", + Source: Source{ + File: "ipsum", + Function: "dolor", + }, + }, + want: true, + }, + "nil": { + err: nil, + want: false, + }, + "other error": { + err: &os.PathError{}, + want: false, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, IsNotFound(tt.err)) + }) + } +} From ed8e7ecfad23772d96515adcf326c1bda05ec7d4 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 15:56:54 -0400 Subject: [PATCH 20/22] Remove ancient commented code and remove unused parameter in loadplugin --- shared/utils.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/shared/utils.go b/shared/utils.go index fa341f6..076159a 100644 --- a/shared/utils.go +++ b/shared/utils.go @@ -21,14 +21,8 @@ func IsVersionInstalled(tool, version string, fs afero.Fs) (bool, error) { return true, nil } -func LoadPlugin(tool string) (mtvmplugin.Plugin, error) { - // var plugin mtvmplugin.Plugin - // if strings.ToLower(tool) == "go" { - // plugin = &goplugin.Plugin{} - // } else { +func LoadPlugin(_ string) (mtvmplugin.Plugin, error) { return nil, errors.New("plugin support is not yet implemented") - // } - // return plugin, nil } func IsNotFound(err error) bool { From 87092561eeaacbf6c1a68eb7dc94801b425658ea Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Thu, 26 Jun 2025 16:14:38 -0400 Subject: [PATCH 21/22] Stricter linter and modify code to meet new requirements --- .golangci.yaml | 22 ++++------------------ plugin/utils_test.go | 5 ++++- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 903596e..00f407e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,21 +2,7 @@ version: "2" linters: enable: - gocritic - exclusions: - generated: lax - presets: - - comments - - common-false-positives - - legacy - - std-error-handling - paths: - - third_party$ - - builtin$ - - examples$ -formatters: - exclusions: - generated: lax - paths: - - third_party$ - - builtin$ - - examples$ + - dupl + settings: + dupl: + threshold: 100 diff --git a/plugin/utils_test.go b/plugin/utils_test.go index 2f580e7..777ebb9 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -297,7 +297,10 @@ func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { require.NoError(t, err, "when creating config directory") file, err := fs.Create(filepath.Join(configDir, "plugins.json")) require.NoError(t, err, "when creating plugins.json") - defer file.Close() + defer func() { + err := file.Close() + assert.NoError(t, err, "when closing plugins.json") + }() _, err = file.Write(content) require.NoError(t, err, "when writing to plugins.json") } From 608acbf40dad476bc0971aa5da85b048ffab8119 Mon Sep 17 00:00:00 2001 From: leomick <83542885+leomick@users.noreply.github.com> Date: Wed, 23 Jul 2025 12:58:52 -0400 Subject: [PATCH 22/22] Fix defer parameter --- plugin/utils_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/utils_test.go b/plugin/utils_test.go index 777ebb9..9183a9d 100644 --- a/plugin/utils_test.go +++ b/plugin/utils_test.go @@ -297,10 +297,10 @@ func createAndWritePluginsJson(t *testing.T, content []byte, fs afero.Fs) { require.NoError(t, err, "when creating config directory") file, err := fs.Create(filepath.Join(configDir, "plugins.json")) require.NoError(t, err, "when creating plugins.json") - defer func() { + defer func(file afero.File) { err := file.Close() assert.NoError(t, err, "when closing plugins.json") - }() + }(file) _, err = file.Write(content) require.NoError(t, err, "when writing to plugins.json") }