From 1fa7de6d6695b0cb7129886392b5d2e41874d209 Mon Sep 17 00:00:00 2001 From: Tai Groot Date: Fri, 6 Mar 2026 00:13:41 +0000 Subject: [PATCH] feat: add PackageUpgrade to Capabilities, exhaustive detect + CLI tests - Add PackageUpgrade field to Capabilities struct and GetCapabilities - Add PackageUpgrade to all 11 provider capability tests - Add pkg-upgrade to CLI detect command output - Expand detect tests: ByName for all managers, concurrent Reset, HasBinary, candidates/allManagers coverage - Add cmd/snack unit tests: targets, opts, getManager, version - 838 tests passing, 0 failures --- apk/apk_test.go | 3 + apt/apt_test.go | 1 + aur/aur_test.go | 1 + capabilities.go | 39 ++++++----- capabilities_test.go | 2 + cmd/snack/main.go | 3 + cmd/snack/main_test.go | 149 ++++++++++++++++++++++++++++++++++++++++ detect/detect_test.go | 114 ++++++++++++++++++++++++++++-- dnf/dnf_test.go | 1 + dpkg/dpkg_test.go | 1 + flatpak/flatpak_test.go | 3 + pacman/pacman_test.go | 1 + pkg/pkg_test.go | 3 + ports/ports_test.go | 3 + rpm/rpm_test.go | 1 + snap/snap_test.go | 3 + 16 files changed, 306 insertions(+), 22 deletions(-) create mode 100644 cmd/snack/main_test.go diff --git a/apk/apk_test.go b/apk/apk_test.go index baf8c4b..577fd30 100644 --- a/apk/apk_test.go +++ b/apk/apk_test.go @@ -405,6 +405,9 @@ func TestCapabilities(t *testing.T) { if caps.NameNormalize { t.Error("expected NameNormalize=false") } + if !caps.PackageUpgrade { + t.Error("expected PackageUpgrade=true") + } } func TestSupportsDryRun(t *testing.T) { diff --git a/apt/apt_test.go b/apt/apt_test.go index 0a84f7b..eccb997 100644 --- a/apt/apt_test.go +++ b/apt/apt_test.go @@ -90,6 +90,7 @@ func TestCapabilities(t *testing.T) { {"Groups", caps.Groups, false}, {"NameNormalize", caps.NameNormalize, true}, {"DryRun", caps.DryRun, true}, + {"PackageUpgrade", caps.PackageUpgrade, true}, } for _, c := range checks { t.Run(c.name, func(t *testing.T) { diff --git a/aur/aur_test.go b/aur/aur_test.go index f576a2a..803c056 100644 --- a/aur/aur_test.go +++ b/aur/aur_test.go @@ -117,6 +117,7 @@ func TestCapabilities(t *testing.T) { {"Groups", caps.Groups, false}, {"NameNormalize", caps.NameNormalize, false}, {"DryRun", caps.DryRun, false}, + {"PackageUpgrade", caps.PackageUpgrade, true}, } for _, tt := range tests { diff --git a/capabilities.go b/capabilities.go index 048a9bd..76027f5 100644 --- a/capabilities.go +++ b/capabilities.go @@ -4,15 +4,16 @@ package snack // Useful for grlx to determine what operations are available before // attempting them. type Capabilities struct { - VersionQuery bool - Hold bool - Clean bool - FileOwnership bool - RepoManagement bool - KeyManagement bool - Groups bool - NameNormalize bool - DryRun bool + VersionQuery bool + Hold bool + Clean bool + FileOwnership bool + RepoManagement bool + KeyManagement bool + Groups bool + NameNormalize bool + DryRun bool + PackageUpgrade bool } // GetCapabilities probes a Manager for all optional interface support. @@ -26,15 +27,17 @@ func GetCapabilities(m Manager) Capabilities { _, g := m.(Grouper) _, nn := m.(NameNormalizer) _, dr := m.(DryRunner) + _, pu := m.(PackageUpgrader) return Capabilities{ - VersionQuery: vq, - Hold: h, - Clean: c, - FileOwnership: fo, - RepoManagement: rm, - KeyManagement: km, - Groups: g, - NameNormalize: nn, - DryRun: dr, + VersionQuery: vq, + Hold: h, + Clean: c, + FileOwnership: fo, + RepoManagement: rm, + KeyManagement: km, + Groups: g, + NameNormalize: nn, + DryRun: dr, + PackageUpgrade: pu, } } diff --git a/capabilities_test.go b/capabilities_test.go index 1efb8a0..bbed6b8 100644 --- a/capabilities_test.go +++ b/capabilities_test.go @@ -77,6 +77,7 @@ func TestGetCapabilities_BaseManager(t *testing.T) { assert.False(t, caps.Groups) assert.False(t, caps.NameNormalize) assert.False(t, caps.DryRun) + assert.False(t, caps.PackageUpgrade) } func TestGetCapabilities_FullManager(t *testing.T) { @@ -90,4 +91,5 @@ func TestGetCapabilities_FullManager(t *testing.T) { assert.True(t, caps.Groups) assert.True(t, caps.NameNormalize) assert.True(t, caps.DryRun) + assert.True(t, caps.PackageUpgrade) } diff --git a/cmd/snack/main.go b/cmd/snack/main.go index cad838d..1abafef 100644 --- a/cmd/snack/main.go +++ b/cmd/snack/main.go @@ -387,6 +387,9 @@ func detectCmd() *cobra.Command { if caps.NameNormalize { capList = append(capList, "normalize") } + if caps.PackageUpgrade { + capList = append(capList, "pkg-upgrade") + } capStr := "" if len(capList) > 0 { capStr = " [" + strings.Join(capList, ", ") + "]" diff --git a/cmd/snack/main_test.go b/cmd/snack/main_test.go new file mode 100644 index 0000000..8739de6 --- /dev/null +++ b/cmd/snack/main_test.go @@ -0,0 +1,149 @@ +package main + +import ( + "testing" + + "github.com/gogrlx/snack" +) + +func TestTargets(t *testing.T) { + tests := []struct { + name string + args []string + ver string + want []snack.Target + }{ + { + name: "no_args", + args: nil, + want: nil, + }, + { + name: "single_no_version", + args: []string{"curl"}, + want: []snack.Target{{Name: "curl"}}, + }, + { + name: "multiple_no_version", + args: []string{"curl", "wget"}, + want: []snack.Target{{Name: "curl"}, {Name: "wget"}}, + }, + { + name: "single_with_version", + args: []string{"curl"}, + ver: "7.88", + want: []snack.Target{{Name: "curl", Version: "7.88"}}, + }, + { + name: "multiple_with_version", + args: []string{"curl", "wget"}, + ver: "1.0", + want: []snack.Target{{Name: "curl", Version: "1.0"}, {Name: "wget", Version: "1.0"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := targets(tt.args, tt.ver) + if len(got) != len(tt.want) { + t.Fatalf("targets() returned %d, want %d", len(got), len(tt.want)) + } + for i, g := range got { + if g.Name != tt.want[i].Name { + t.Errorf("[%d] Name = %q, want %q", i, g.Name, tt.want[i].Name) + } + if g.Version != tt.want[i].Version { + t.Errorf("[%d] Version = %q, want %q", i, g.Version, tt.want[i].Version) + } + } + }) + } +} + +func TestOpts(t *testing.T) { + // Reset flags + flagSudo = false + flagYes = false + flagDry = false + + o := opts() + if len(o) != 0 { + t.Errorf("expected 0 options with no flags, got %d", len(o)) + } + + flagSudo = true + o = opts() + if len(o) != 1 { + t.Errorf("expected 1 option with sudo, got %d", len(o)) + } + + flagYes = true + flagDry = true + o = opts() + if len(o) != 3 { + t.Errorf("expected 3 options with all flags, got %d", len(o)) + } + + // Clean up + flagSudo = false + flagYes = false + flagDry = false +} + +func TestOptsApply(t *testing.T) { + flagSudo = true + flagYes = true + flagDry = true + defer func() { + flagSudo = false + flagYes = false + flagDry = false + }() + + applied := snack.ApplyOptions(opts()...) + if !applied.Sudo { + t.Error("expected Sudo=true") + } + if !applied.AssumeYes { + t.Error("expected AssumeYes=true") + } + if !applied.DryRun { + t.Error("expected DryRun=true") + } +} + +func TestGetManager(t *testing.T) { + // Default detection + flagMgr = "" + m, err := getManager() + if err != nil { + t.Skipf("no manager available: %v", err) + } + if m.Name() == "" { + t.Error("expected non-empty manager name") + } + + // Explicit override + flagMgr = "apt" + m, err = getManager() + if err != nil { + t.Fatalf("getManager() with --manager=apt failed: %v", err) + } + if m.Name() != "apt" { + t.Errorf("expected Name()=apt, got %q", m.Name()) + } + + // Unknown manager + flagMgr = "nonexistent-manager-xyz" + _, err = getManager() + if err == nil { + t.Error("expected error for unknown manager") + } + + flagMgr = "" +} + +func TestVersionString(t *testing.T) { + if version == "" { + t.Error("version should not be empty") + } +} diff --git a/detect/detect_test.go b/detect/detect_test.go index fd3ba5f..7b548f1 100644 --- a/detect/detect_test.go +++ b/detect/detect_test.go @@ -1,7 +1,10 @@ package detect import ( + "errors" "testing" + + "github.com/gogrlx/snack" ) func TestByNameUnknown(t *testing.T) { @@ -9,15 +12,62 @@ func TestByNameUnknown(t *testing.T) { if err == nil { t.Fatal("expected error for unknown manager") } + if !errors.Is(err, snack.ErrManagerNotFound) { + t.Errorf("expected ErrManagerNotFound, got %v", err) + } +} + +func TestByNameKnown(t *testing.T) { + // All known manager names should be resolvable by ByName, even if + // unavailable on this system. + knownNames := []string{"apt", "dnf", "pacman", "apk", "flatpak", "snap", "aur"} + for _, name := range knownNames { + t.Run(name, func(t *testing.T) { + m, err := ByName(name) + if err != nil { + t.Fatalf("ByName(%q) returned error: %v", name, err) + } + if m.Name() != name { + t.Errorf("ByName(%q).Name() = %q", name, m.Name()) + } + }) + } +} + +func TestByNameReturnsCorrectType(t *testing.T) { + m, err := ByName("apt") + if err != nil { + t.Skip("apt not in allManagers on this platform") + } + if m.Name() != "apt" { + t.Errorf("expected Name()=apt, got %q", m.Name()) + } } func TestAllReturnsSlice(t *testing.T) { - // Just verify it doesn't panic; actual availability depends on system. - _ = All() + managers := All() + // On Linux with apt installed, we should get at least 1 + // But don't fail if none — could be a weird CI environment + seen := make(map[string]bool) + for _, m := range managers { + name := m.Name() + if seen[name] { + t.Errorf("duplicate manager in All(): %s", name) + } + seen[name] = true + } +} + +func TestAllManagersAreAvailable(t *testing.T) { + for _, m := range All() { + if !m.Available() { + t.Errorf("All() returned unavailable manager: %s", m.Name()) + } + } } func TestDefaultDoesNotPanic(t *testing.T) { - // May return error if no managers available; that's fine. + Reset() _, _ = Default() } @@ -37,9 +87,65 @@ func TestDefaultCachesResult(t *testing.T) { } } +func TestDefaultReturnsAvailableManager(t *testing.T) { + Reset() + m, err := Default() + if err != nil { + t.Skipf("no manager available on this system: %v", err) + } + if !m.Available() { + t.Error("Default() returned unavailable manager") + } + if m.Name() == "" { + t.Error("Default() returned manager with empty name") + } +} + func TestResetAllowsRedetection(t *testing.T) { _, _ = Default() Reset() - // After reset, defaultOnce should be fresh; calling Default() again should work. + // After reset, calling Default() again should work. _, _ = Default() } + +func TestResetConcurrent(t *testing.T) { + done := make(chan struct{}) + for i := 0; i < 10; i++ { + go func() { + defer func() { done <- struct{}{} }() + Reset() + _, _ = Default() + }() + } + for i := 0; i < 10; i++ { + <-done + } +} + +func TestHasBinary(t *testing.T) { + // sh should exist on any Unix system + if !HasBinary("sh") { + t.Error("expected HasBinary(sh) = true") + } + if HasBinary("this-binary-does-not-exist-anywhere-12345") { + t.Error("expected HasBinary(nonexistent) = false") + } +} + +func TestCandidatesNotEmpty(t *testing.T) { + c := candidates() + if len(c) == 0 { + t.Error("candidates() returned empty slice") + } +} + +func TestAllManagersNotEmpty(t *testing.T) { + a := allManagers() + if len(a) == 0 { + t.Error("allManagers() returned empty slice") + } + // allManagers should be a superset of candidates + if len(a) < len(candidates()) { + t.Error("allManagers() should include at least all candidates") + } +} diff --git a/dnf/dnf_test.go b/dnf/dnf_test.go index aca96d9..6b1b364 100644 --- a/dnf/dnf_test.go +++ b/dnf/dnf_test.go @@ -52,6 +52,7 @@ func TestGetCapabilities(t *testing.T) { {"Groups", caps.Groups}, {"NameNormalize", caps.NameNormalize}, {"DryRun", caps.DryRun}, + {"PackageUpgrade", caps.PackageUpgrade}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/dpkg/dpkg_test.go b/dpkg/dpkg_test.go index 882b39a..19a17a2 100644 --- a/dpkg/dpkg_test.go +++ b/dpkg/dpkg_test.go @@ -117,6 +117,7 @@ func TestCapabilities(t *testing.T) { {"RepoManagement", caps.RepoManagement, false}, {"KeyManagement", caps.KeyManagement, false}, {"Groups", caps.Groups, false}, + {"PackageUpgrade", caps.PackageUpgrade, false}, } for _, c := range checks { t.Run(c.name, func(t *testing.T) { diff --git a/flatpak/flatpak_test.go b/flatpak/flatpak_test.go index de73f82..f03aeff 100644 --- a/flatpak/flatpak_test.go +++ b/flatpak/flatpak_test.go @@ -400,6 +400,9 @@ func TestCapabilities(t *testing.T) { if caps.NameNormalize { t.Error("expected NameNormalize=false") } + if !caps.PackageUpgrade { + t.Error("expected PackageUpgrade=true") + } } func TestName(t *testing.T) { diff --git a/pacman/pacman_test.go b/pacman/pacman_test.go index f892c21..bf0051f 100644 --- a/pacman/pacman_test.go +++ b/pacman/pacman_test.go @@ -173,6 +173,7 @@ func TestCapabilities(t *testing.T) { {"RepoManagement", caps.RepoManagement, false}, {"KeyManagement", caps.KeyManagement, false}, {"NameNormalize", caps.NameNormalize, false}, + {"PackageUpgrade", caps.PackageUpgrade, true}, } for _, tt := range tests { diff --git a/pkg/pkg_test.go b/pkg/pkg_test.go index e677dfb..3dc14ec 100644 --- a/pkg/pkg_test.go +++ b/pkg/pkg_test.go @@ -665,4 +665,7 @@ func TestCapabilities(t *testing.T) { if caps.DryRun { t.Error("expected DryRun=false") } + if !caps.PackageUpgrade { + t.Error("expected PackageUpgrade=true") + } } diff --git a/ports/ports_test.go b/ports/ports_test.go index 221f89a..a7eb3c9 100644 --- a/ports/ports_test.go +++ b/ports/ports_test.go @@ -802,4 +802,7 @@ func TestCapabilities(t *testing.T) { if caps.DryRun { t.Error("expected DryRun=false") } + if !caps.PackageUpgrade { + t.Error("expected PackageUpgrade=true") + } } diff --git a/rpm/rpm_test.go b/rpm/rpm_test.go index f11f104..5eae3e9 100644 --- a/rpm/rpm_test.go +++ b/rpm/rpm_test.go @@ -44,6 +44,7 @@ func TestGetCapabilities(t *testing.T) { "KeyManagement": caps.KeyManagement, "Groups": caps.Groups, "DryRun": caps.DryRun, + "PackageUpgrade": caps.PackageUpgrade, } for name, got := range wantFalse { t.Run(name+"_false", func(t *testing.T) { diff --git a/snap/snap_test.go b/snap/snap_test.go index 2144a4d..c0566d9 100644 --- a/snap/snap_test.go +++ b/snap/snap_test.go @@ -444,6 +444,9 @@ func TestCapabilities(t *testing.T) { if caps.NameNormalize { t.Error("expected NameNormalize=false") } + if !caps.PackageUpgrade { + t.Error("expected PackageUpgrade=true") + } } func TestName(t *testing.T) {