Skip to content

chore(detector): wrap fnm GlobalNpmPrefix error; add NVM.Current tests#98

Merged
dipto0321 merged 1 commit into
mainfrom
chore/cleanup/feature-debris
Jul 3, 2026
Merged

chore(detector): wrap fnm GlobalNpmPrefix error; add NVM.Current tests#98
dipto0321 merged 1 commit into
mainfrom
chore/cleanup/feature-debris

Conversation

@dipto0321

Copy link
Copy Markdown
Owner

Summary

Bundles the two remaining cleanup-feature debris items from issue
#61. Two of the four items called out in the issue body were
already removed by PR #95 / issue #76cleanup.go's
confirm := true / _ = confirm dead lines and upgrade.go's
_ = yes line — so this PR focuses on the items still standing:

  1. internal/detector/fnm.go's GlobalNpmPrefix final error path
    now wraps the underlying os.Stat error with %w.
    The
    pre-fix line discarded err2 entirely:

    return "", fmt.Errorf("fnm global npm prefix for %s not found at %s or %s", v, prefix, filepath.Join(dir, "installation", "lib", "node_modules"))

    Every sibling implementation (asdf, mise, n, nodenv, nvm, volta)
    correctly uses %w in the equivalent function. Low-severity
    consistency fix.

  2. NVM.Current() now has full method-level test coverage.
    internal/detector/nvm_test.go previously only exercised the
    pure output parser parseNVMCurrent, but not NVM.Current()
    itself — every other manager's Current() has a
    Test<X>Current_InvokesShell-style test (fnm, asdf, mise,
    nodenv, volta). Three new tests close the gap:

    • TestNVMCurrent_InvokesShell — pins the production script
      shape (sources nvm.sh, runs nvm current, parses output).
    • TestNVMCurrent_PropagatesShellError — pins the wrapped-
      error contract (runScript failure surfaces with %w).
    • TestNVMCurrent_SystemIsError — pins the end-to-end "system"
      handling (nvm current printing "system" surfaces an error,
      not just at the parse layer).

Linked issues

Closes #61

Type of change

  • chore — maintenance / dependency bump
  • feat — new feature (MINOR bump)
  • fix — bug fix (PATCH bump)
  • refactor — no behavior change
  • perf — performance improvement
  • test — tests only
  • docs — documentation only
  • ci — CI/CD only
  • build — build system only
  • Breaking change (MAJOR bump) — explain below

Checklist

  • Title follows Conventional Commits (chore(scope): subject)
  • I ran make ci locally and it passes (golangci-lint run,
    go vet, go test -race ./... all green; detector
    coverage rose from 76.8% → 77.5%)
  • I added or updated tests for the change (3 new tests for
    NVM.Current(); GlobalNpmPrefix is exercised by the
    existing TestFNM_* matrix)
  • I updated relevant docs (no user-facing change; CHANGELOG.md
    gets a ### Fixed entry under [Unreleased])
  • No new linter warnings (make lint green)
  • If breaking: I documented the migration path in the PR body
    and updated CHANGELOG.md

Notes

Items #1 (confirm := true / _ = confirm) and #2 (_ = yes)
from issue #61's body were already fixed by PR #95 / issue #76
when that PR restructured the per-version prompt loop. Those
removals are noted in the PR body / CHANGELOG entry but no
new code touches them in this branch.

Files touched

Two cleanup-feature debris items from issue #61 that weren't already
removed by #76:

- 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).
  Pre-fix the err2 was discarded.
- internal/detector/nvm_test.go gains three tests for
  NVM.Current() to close the only Current() method in the package
  that had no method-level test coverage:
  - TestNVMCurrent_InvokesShell: pins the production script shape
    (sources nvm.sh, runs `nvm current`, parses output).
  - TestNVMCurrent_PropagatesShellError: pins the wrapped-error
    contract (%w wrap of runScript failures).
  - TestNVMCurrent_SystemIsError: pins the end-to-end "system"
    handling (nvm current printing "system" surfaces an error).

Items #1 (cleanup.go confirm := true / _ = confirm) and #2
(upgrade.go _ = yes) from the issue body were already cleaned up
by PR #95 / #76.

Closes #61
@cocogitto-bot

cocogitto-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown

✔️ 6db4199 - Conventional commits check succeeded.

@dipto0321 dipto0321 merged commit a950dad into main Jul 3, 2026
10 checks passed
@dipto0321 dipto0321 deleted the chore/cleanup/feature-debris branch July 3, 2026 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(cleanup): dead code + fnm error-wrapping + nvm Current() test gap from cleanup-feature PR

1 participant