From 7e024c7a90b3f6fbc0b885d0413ee421f2a540fc Mon Sep 17 00:00:00 2001 From: "Calvin A. Allen" Date: Mon, 9 Feb 2026 10:22:06 -0500 Subject: [PATCH] fix(config): return ("", nil) from GlobalVersion when unconfigured On a clean system with no ~/.dtvem/config/runtimes.json, installing the first version of a runtime should auto-set it as the global default. This failed because GlobalVersion() returned an error when the config file was missing, and autoSetGlobalIfNeeded() bailed out on any error. Change GlobalVersion() to return ("", nil) for two "unconfigured" states (missing config file, runtime not in config) instead of an error. Reserve errors for actual failures (I/O errors, corrupt JSON). Closes #220 --- src/cmd/install_test.go | 19 ++++++++- src/internal/config/version.go | 17 ++++++-- src/internal/config/version_test.go | 40 +++++++++++++++++++ src/internal/runtime/provider_test_harness.go | 12 ++++-- 4 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/cmd/install_test.go b/src/cmd/install_test.go index 9c105ba..b9fb367 100644 --- a/src/cmd/install_test.go +++ b/src/cmd/install_test.go @@ -1,6 +1,7 @@ package cmd import ( + "fmt" "testing" "github.com/CodingWithCalvin/dtvem.cli/src/internal/runtime" @@ -11,6 +12,7 @@ type mockProvider struct { name string displayName string globalVersion string + globalVersionErr error globalSetError error setGlobalCalls []string availableVersions []runtime.AvailableVersion @@ -53,7 +55,7 @@ func (m *mockProvider) GetEnvironment(_ string) (map[string]string, error) { } func (m *mockProvider) GlobalVersion() (string, error) { - return m.globalVersion, nil + return m.globalVersion, m.globalVersionErr } func (m *mockProvider) SetGlobalVersion(version string) error { @@ -225,6 +227,21 @@ func TestResolveVersionForProvider_NoMatch(t *testing.T) { } } +func TestAutoSetGlobalIfNeeded_GlobalVersionError(t *testing.T) { + provider := &mockProvider{ + name: "test", + displayName: "Test", + globalVersion: "", + globalVersionErr: fmt.Errorf("permission denied"), + } + + autoSetGlobalIfNeeded(provider, "1.0.0") + + if len(provider.setGlobalCalls) != 0 { + t.Errorf("Expected SetGlobalVersion to not be called when GlobalVersion returns error, got %d calls", len(provider.setGlobalCalls)) + } +} + func TestResolveVersionForProvider_PythonVersions(t *testing.T) { provider := &mockProvider{ name: "python", diff --git a/src/internal/config/version.go b/src/internal/config/version.go index f82eea9..5050ad5 100644 --- a/src/internal/config/version.go +++ b/src/internal/config/version.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" ) // RuntimesConfig represents the flat structure of runtimes.json @@ -142,16 +143,24 @@ func LocalVersion(runtimeName string) (string, error) { return findLocalVersion(runtimeName) } -// GlobalVersion reads the global version for a runtime +// GlobalVersion reads the global version for a runtime. +// Returns ("", nil) when no version is configured (file missing or runtime not in config). +// Returns an error only for actual failures (I/O errors, corrupt JSON). func GlobalVersion(runtimeName string) (string, error) { configPath := GlobalConfigPath() - // Check if config file exists + // No config file yet — valid "unconfigured" state if _, err := os.Stat(configPath); os.IsNotExist(err) { - return "", fmt.Errorf("no global version configured") + return "", nil } - return readVersionFile(configPath, runtimeName) + version, err := readVersionFile(configPath, runtimeName) + if err != nil && strings.Contains(err.Error(), "not found in config file") { + // Runtime not in config — valid "unconfigured" state + return "", nil + } + + return version, err } // SetGlobalVersion sets the global version for a runtime diff --git a/src/internal/config/version_test.go b/src/internal/config/version_test.go index 57314f6..c133be1 100644 --- a/src/internal/config/version_test.go +++ b/src/internal/config/version_test.go @@ -442,6 +442,46 @@ func TestSetGlobalVersion_MultipleRuntimes(t *testing.T) { } } +func TestGlobalVersion_NoConfigFile(t *testing.T) { + // On a clean system with no config file, GlobalVersion should return ("", nil) + tmpRoot := t.TempDir() + t.Setenv("HOME", tmpRoot) + t.Setenv("USERPROFILE", tmpRoot) + ResetPathsCache() + defer ResetPathsCache() + + version, err := GlobalVersion("python") + if err != nil { + t.Errorf("GlobalVersion() with no config file should return nil error, got: %v", err) + } + if version != "" { + t.Errorf("GlobalVersion() with no config file should return empty string, got: %q", version) + } +} + +func TestGlobalVersion_RuntimeNotInConfig(t *testing.T) { + // When config exists but runtime is not in it, GlobalVersion should return ("", nil) + tmpRoot := t.TempDir() + t.Setenv("HOME", tmpRoot) + t.Setenv("USERPROFILE", tmpRoot) + ResetPathsCache() + defer ResetPathsCache() + + // Set a version for python so the config file exists + if err := SetGlobalVersion("python", "3.11.0"); err != nil { + t.Fatalf("SetGlobalVersion() setup error: %v", err) + } + + // Ask for node which is not in the config + version, err := GlobalVersion("node") + if err != nil { + t.Errorf("GlobalVersion() for missing runtime should return nil error, got: %v", err) + } + if version != "" { + t.Errorf("GlobalVersion() for missing runtime should return empty string, got: %q", version) + } +} + func TestSetLocalVersion_CreatesDirectoryAndFile(t *testing.T) { // Create temp directory and change to it tmpRoot := t.TempDir() diff --git a/src/internal/runtime/provider_test_harness.go b/src/internal/runtime/provider_test_harness.go index 6990a41..dff9da4 100644 --- a/src/internal/runtime/provider_test_harness.go +++ b/src/internal/runtime/provider_test_harness.go @@ -250,10 +250,14 @@ func (h *ProviderTestHarness) TestIsInstalled(t *testing.T) { func (h *ProviderTestHarness) TestGetGlobalVersion(t *testing.T) { version, err := h.Provider.GlobalVersion() - // It's OK to have error if no global version is set - // But if version is returned, it should be non-empty - if err == nil && version == "" { - t.Error("GetGlobalVersion() returned empty string without error") + // ("", nil) is valid — means no global version configured + // If a version is returned, it should be non-empty + if err == nil && version != "" { + // Valid: a global version is configured + } + if err != nil { + // Valid: an actual error occurred (I/O, corrupt config) + t.Logf("GetGlobalVersion() returned error (may be expected): %v", err) } }