diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d13bfa..037f4c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -692,6 +692,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 cleanup + mutation have shipped while distribution packaging (#17 / #18) is the remaining Phase 7 work. Closes #60. +- Bundled cleanup-feature debris from PR #56 / issue #41: + `internal/detector/fnm.go`'s `GlobalNpmPrefix` final error path + now wraps the underlying `os.Stat` error with `%w` (matching + every sibling manager's behavior — asdf, mise, n, nodenv, nvm, + volta); previously the `err2` was discarded. Adds three + `internal/detector/nvm_test.go` tests pinning `NVM.Current()`: + `TestNVMCurrent_InvokesShell` (sources nvm.sh + runs `nvm + current` + parses the output), `TestNVMCurrent_PropagatesShellError` + (`runScript` failure surfaces with `%w`), and + `TestNVMCurrent_SystemIsError` (`nvm current` printing "system" + surfaces an error end-to-end, not just at the parse layer). + `Current()` was the safety-critical gate protecting the active + version from cleanup deletion, and was the only `Current()` + method in the package without a test at the method layer. The + dead `confirm := true` / `_ = confirm` lines in `cleanup.go` + and the `_ = yes` line in `upgrade.go` flagged in #61 had + already been removed by #76 / PR #95. Closes #61. ## [0.0.0] - 2024-07-01 diff --git a/internal/detector/fnm.go b/internal/detector/fnm.go index 554967f..831681b 100644 --- a/internal/detector/fnm.go +++ b/internal/detector/fnm.go @@ -244,7 +244,7 @@ func (f *FNM) GlobalNpmPrefix(v semver.Version) (string, error) { // Older fnm layout (no `installation/` subdir). prefix = filepath.Join(dir, "lib", "node_modules") if _, err2 := os.Stat(prefix); err2 != nil { - return "", fmt.Errorf("fnm global npm prefix for %s not found at %s or %s", v, prefix, filepath.Join(dir, "installation", "lib", "node_modules")) + return "", fmt.Errorf("fnm global npm prefix for %s not found at %s or %s: %w", v, prefix, filepath.Join(dir, "installation", "lib", "node_modules"), err2) } } else { return "", fmt.Errorf("stat fnm prefix %s: %w", prefix, err) diff --git a/internal/detector/nvm_test.go b/internal/detector/nvm_test.go index 91f85e6..69514c7 100644 --- a/internal/detector/nvm_test.go +++ b/internal/detector/nvm_test.go @@ -589,6 +589,68 @@ func TestParseNVMCurrent_Empty(t *testing.T) { } } +// --- NVM.Current -------------------------------------------------------- + +// TestNVMCurrent_InvokesShell pins the production behavior that +// Current() (a) sources nvm.sh, (b) runs `nvm current`, and (c) parses +// the result through parseNVMCurrent. Sister to TestFNMCurrent_ +// InvokesShell / TestASDFCurrent_InvokesShell / etc. — every other +// manager's Current() has one of these, so the absence here is a +// real coverage gap given Current() is the safety-critical gate +// protecting the active version from deletion in cleanup.go. +func TestNVMCurrent_InvokesShell(t *testing.T) { + withStubNVMScript(t) + rec := withStubScript(t, "v22.11.0\n", nil) + + got, err := NewNVM().Current() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.String() != "22.11.0" { + t.Errorf("got %q, want %q", got.String(), "22.11.0") + } + if !strings.Contains(rec.script, "source ") { + t.Errorf("script missing source: %q", rec.script) + } + if !strings.Contains(rec.script, "nvm current") { + t.Errorf("script missing `nvm current`: %q", rec.script) + } +} + +// TestNVMCurrent_PropagatesShellError pins the wrapped-error contract +// for Current(): a `runScript` failure must surface with `%w` so the +// cleanup-prompt's `errors.Is(err, ...)` checks (and any future +// `errors.As` debugging) can introspect it. Sister to the equivalent +// test on fnm / asdf / mise / nodenv / volta. +func TestNVMCurrent_PropagatesShellError(t *testing.T) { + withStubNVMScript(t) + withStubScript(t, "", errSentinelForTest) + + _, err := NewNVM().Current() + if err == nil { + t.Fatal("expected error, got nil") + } + if !errors.Is(err, errSentinelForTest) { + t.Errorf("error %v should wrap %v", err, errSentinelForTest) + } +} + +// TestNVMCurrent_SystemIsError verifies the parseNVMCurrent branch +// is exercised end-to-end: when nvm prints "system" (no nvm-managed +// version is active), Current() must surface an error so the cleanup +// prompt doesn't try to exclude "system" from the candidates set. +// This complements TestParseNVMCurrent_SystemIsError (which only +// exercises the pure parser). +func TestNVMCurrent_SystemIsError(t *testing.T) { + withStubNVMScript(t) + withStubScript(t, "system\n", nil) + + _, err := NewNVM().Current() + if err == nil { + t.Error("expected error when `nvm current` returns 'system'") + } +} + // --- helpers ------------------------------------------------------------ // errSentinelForTest is the error returned by stubs to verify error