From 7ad5d4db7fa24c005dfd6d37b968eea2d24e4d8e Mon Sep 17 00:00:00 2001 From: Bartek Mucha Date: Fri, 4 Sep 2026 19:55:34 +0100 Subject: [PATCH 1/4] Tighten prerequisite ID mapping - Introduce `DependencyID` and use it for tracking prerequisites - All prerequisites must be fully healthy for a dependant check to trigger Signed-off-by: Bartek Mucha --- internal/health/dependencies.go | 180 +++++++++++++++------------ internal/health/dependencies_test.go | 155 +++++++++++++---------- internal/health/health.go | 2 +- 3 files changed, 190 insertions(+), 147 deletions(-) diff --git a/internal/health/dependencies.go b/internal/health/dependencies.go index e268dea4..c8475d1f 100644 --- a/internal/health/dependencies.go +++ b/internal/health/dependencies.go @@ -24,27 +24,22 @@ const ( Remoteproc HardwareCapability = iota ) -type SoftwareDependency int - const containerEngineInstallURL = "https://github.com/arm/topo#install-a-container-engine" -const ( - UnsetSoftwareDependency SoftwareDependency = iota - Docker - Lscpu -) +type DependencyID string type Dependency struct { + ID DependencyID Binary string Label string Checks []Check - SoftwareEnumID SoftwareDependency - SoftwarePrerequisites []SoftwareDependency - HardwarePrerequisite []HardwareCapability + SoftwarePrerequisites []DependencyID + HardwarePrerequisites []HardwareCapability } -var HostRequiredDependencies = []Dependency{ - { +func HostRequiredDependencies() []Dependency { + topo := Dependency{ + ID: DependencyID("topo"), Binary: "topo", Label: "Topo", Checks: []Check{VersionMatches{ @@ -75,16 +70,19 @@ var HostRequiredDependencies = []Dependency{ return fix }, }}, - }, - { + } + + ssh := Dependency{ + ID: DependencyID("ssh"), Binary: "ssh", Label: "OpenSSH", Checks: []Check{BinaryExists{}, OpenSSHAvailable{}}, - }, - { - Binary: "docker", - Label: "Container Engine", - SoftwareEnumID: Docker, + } + + docker := Dependency{ + ID: DependencyID("host-docker"), + Binary: "docker", + Label: "Container Engine", Checks: []Check{ BinaryExists{ Fix: &Fix{ @@ -98,11 +96,12 @@ var HostRequiredDependencies = []Dependency{ }, }, }, - }, - { - Binary: "docker-compose", - Label: "Docker Compose", - SoftwarePrerequisites: []SoftwareDependency{Docker}, + } + + dockerCompose := Dependency{ + ID: DependencyID("docker-compose"), + Binary: "docker-compose", + Label: "Docker Compose", Checks: []Check{ CommandSuccessful{ Cmd: "docker compose", @@ -114,65 +113,82 @@ var HostRequiredDependencies = []Dependency{ MinVersion: "2.21.0", }, }, - }, + SoftwarePrerequisites: []DependencyID{docker.ID}, + } + + return []Dependency{ + topo, + ssh, + docker, + dockerCompose, + } } func TargetRequiredDependencies(target ssh.Destination) []Dependency { - return []Dependency{ - { - Binary: "docker", - Label: "Container Engine", - SoftwareEnumID: Docker, - Checks: []Check{ - BinaryExists{ - Fix: &Fix{ - Description: "Install a supported container engine. See " + containerEngineInstallURL, - }, + docker := Dependency{ + ID: DependencyID("target-docker"), + Binary: "docker", + Label: "Container Engine", + Checks: []Check{ + BinaryExists{ + Fix: &Fix{ + Description: "Install a supported container engine. See " + containerEngineInstallURL, }, - CommandSuccessful{ - Cmd: "docker info", - Fix: &Fix{ - Description: "Ensure current user can run docker commands. See " + containerEngineInstallURL, - }, + }, + CommandSuccessful{ + Cmd: "docker info", + Fix: &Fix{ + Description: "Ensure current user can run docker commands. See " + containerEngineInstallURL, }, }, }, - { - Binary: "remoteproc-runtime", - Label: "Remoteproc Runtime", - SoftwarePrerequisites: []SoftwareDependency{Docker}, - HardwarePrerequisite: []HardwareCapability{Remoteproc}, - Checks: []Check{ - BinaryExists{ - Severity: SeverityWarning, - Fix: &Fix{ - Description: "Install the Remoteproc Runtime", - Command: fmt.Sprintf("topo install remoteproc-runtime --target %s", target), - }, + } + + remoteprocRuntime := Dependency{ + ID: DependencyID("remoteproc-runtime"), + Binary: "remoteproc-runtime", + Label: "Remoteproc Runtime", + SoftwarePrerequisites: []DependencyID{docker.ID}, + HardwarePrerequisites: []HardwareCapability{Remoteproc}, + Checks: []Check{ + BinaryExists{ + Severity: SeverityWarning, + Fix: &Fix{ + Description: "Install the Remoteproc Runtime", + Command: fmt.Sprintf("topo install remoteproc-runtime --target %s", target), }, }, }, - { - Binary: "containerd-shim-remoteproc-v1", - Label: "Remoteproc Shim", - SoftwarePrerequisites: []SoftwareDependency{Docker}, - HardwarePrerequisite: []HardwareCapability{Remoteproc}, - Checks: []Check{ - BinaryExists{ - Severity: SeverityWarning, - Fix: &Fix{ - Description: "Install the Remoteproc Runtime", - Command: fmt.Sprintf("topo install remoteproc-runtime --target %s", target), - }, + } + remoteprocRuntimeShim := Dependency{ + ID: DependencyID("containerd-shim-remoteproc-v1"), + Binary: "containerd-shim-remoteproc-v1", + Label: "Remoteproc Shim", + SoftwarePrerequisites: []DependencyID{docker.ID}, + HardwarePrerequisites: []HardwareCapability{Remoteproc}, + Checks: []Check{ + BinaryExists{ + Severity: SeverityWarning, + Fix: &Fix{ + Description: "Install the Remoteproc Runtime", + Command: fmt.Sprintf("topo install remoteproc-runtime --target %s", target), }, }, }, - { - Binary: "lscpu", - Label: "Hardware Info", - SoftwareEnumID: Lscpu, - Checks: []Check{BinaryExists{}}, - }, + } + + lscpu := Dependency{ + ID: DependencyID("lscpu"), + Binary: "lscpu", + Label: "Hardware Info", + Checks: []Check{BinaryExists{}}, + } + + return []Dependency{ + docker, + remoteprocRuntime, + remoteprocRuntimeShim, + lscpu, } } @@ -185,7 +201,7 @@ type DependencyStatus struct { func FilterByHardware(deps []Dependency, hardware map[HardwareCapability]struct{}) []Dependency { result := make([]Dependency, 0, len(deps)) for _, dep := range deps { - if len(dep.HardwarePrerequisite) == 0 || hardwareCapabilityMatches(dep.HardwarePrerequisite, hardware) { + if len(dep.HardwarePrerequisites) == 0 || hardwareCapabilityMatches(dep.HardwarePrerequisites, hardware) { result = append(result, dep) } } @@ -202,11 +218,11 @@ func hardwareCapabilityMatches(required []HardwareCapability, available map[Hard } func PerformChecks(ctx context.Context, dependencies []Dependency, runner runner.Runner) []DependencyStatus { - installed := make(map[SoftwareDependency]struct{}) + healthy := make(map[DependencyID]struct{}) result := make([]DependencyStatus, 0, len(dependencies)) for _, dep := range dependencies { - if len(dep.SoftwarePrerequisites) > 0 && !hasAnyInstalledPrerequisite(dep.SoftwarePrerequisites, installed) { + if !allPrerequisitesFulfilled(dep.SoftwarePrerequisites, healthy) { continue } @@ -214,15 +230,15 @@ func PerformChecks(ctx context.Context, dependencies []Dependency, runner runner var err error for _, check := range dep.Checks { fix, err = check.Run(ctx, runner, dep) - if _, ok := check.(BinaryExists); ok && err == nil { - installed[dep.SoftwareEnumID] = struct{}{} - } - if err != nil { break } } + if err == nil { + healthy[dep.ID] = struct{}{} + } + result = append(result, DependencyStatus{ Dependency: dep, Error: err, @@ -232,11 +248,11 @@ func PerformChecks(ctx context.Context, dependencies []Dependency, runner runner return result } -func hasAnyInstalledPrerequisite(required []SoftwareDependency, installed map[SoftwareDependency]struct{}) bool { - for _, softwareDep := range required { - if _, exists := installed[softwareDep]; exists { - return true +func allPrerequisitesFulfilled(required []DependencyID, healthy map[DependencyID]struct{}) bool { + for _, dep := range required { + if _, ok := healthy[dep]; !ok { + return false } } - return false + return true } diff --git a/internal/health/dependencies_test.go b/internal/health/dependencies_test.go index 0e5703e1..72d8e967 100644 --- a/internal/health/dependencies_test.go +++ b/internal/health/dependencies_test.go @@ -3,6 +3,7 @@ package health_test import ( "context" "errors" + "slices" "testing" "github.com/arm/topo/internal/command" @@ -10,61 +11,70 @@ import ( "github.com/arm/topo/internal/runner" "github.com/arm/topo/internal/ssh" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestDependencyFormat(t *testing.T) { - t.Run("host dependencies are of the correct format", func(t *testing.T) { - for _, dep := range health.HostRequiredDependencies { - assert.NoError(t, command.ValidateBinaryName(dep.Binary)) +func TestDependencies(t *testing.T) { + t.Run("ids are unique across all dependencies", func(t *testing.T) { + hostDeps := health.HostRequiredDependencies() + targetDeps := health.TargetRequiredDependencies(ssh.NewDestination("whatever")) + + ids := make([]health.DependencyID, 0, len(hostDeps)+len(targetDeps)) + for _, dep := range slices.Concat(hostDeps, targetDeps) { + require.NotEmpty(t, dep.ID, "%#v has empty id", dep) + require.NotContains(t, ids, dep.ID) + ids = append(ids, dep.ID) } }) - t.Run("target dependencies are of the correct format", func(t *testing.T) { - for _, dep := range health.TargetRequiredDependencies(ssh.NewDestination("does-not-matter-for-this-test")) { + t.Run("binary names are of the correct format", func(t *testing.T) { + hostDeps := health.HostRequiredDependencies() + targetDeps := health.TargetRequiredDependencies(ssh.NewDestination("whatever")) + + for _, dep := range slices.Concat(hostDeps, targetDeps) { assert.NoError(t, command.ValidateBinaryName(dep.Binary)) } }) - t.Run("target SoftwarePrerequisites reference valid dependencies", func(t *testing.T) { - availableEnums := make(map[health.SoftwareDependency]bool) - seenEnums := make(map[health.SoftwareDependency]string) - - t.Run("There are no duplicate SoftwareEnumID assignments", func(t *testing.T) { - for _, dep := range health.TargetRequiredDependencies(ssh.NewDestination("user@my-target")) { - if dep.SoftwareEnumID != health.UnsetSoftwareDependency { - if existingDep, exists := seenEnums[dep.SoftwareEnumID]; exists { - t.Errorf("Duplicate SoftwareEnumID %d assigned to both %q and %q", dep.SoftwareEnumID, existingDep, dep.Binary) - } - seenEnums[dep.SoftwareEnumID] = dep.Binary - availableEnums[dep.SoftwareEnumID] = true + t.Run("host dependencies", func(t *testing.T) { + t.Run("prerequisites are fulfillable", func(t *testing.T) { + deps := health.HostRequiredDependencies() + ids := make([]health.DependencyID, 0, len(deps)) + for _, dep := range deps { + ids = append(ids, dep.ID) + for _, prereq := range dep.SoftwarePrerequisites { + require.Contains(t, ids, prereq) } } }) + }) - t.Run("all SoftwarePrerequisites reference valid SoftwareEnumID", func(t *testing.T) { - for _, dep := range health.TargetRequiredDependencies(ssh.NewDestination("user@my-target")) { + t.Run("target dependencies", func(t *testing.T) { + t.Run("prerequisites are fulfillable", func(t *testing.T) { + deps := health.TargetRequiredDependencies(ssh.NewDestination("does-not-matter-for-this-test")) + ids := make([]health.DependencyID, 0, len(deps)) + for _, dep := range deps { + ids = append(ids, dep.ID) for _, prereq := range dep.SoftwarePrerequisites { - assert.True(t, availableEnums[prereq], "%q has SoftwarePrerequisites %v which is not provided by any dependency's SoftwareEnumID", dep.Binary, prereq) + require.Contains(t, ids, prereq) } } }) - }) -} -func TestTargetRequiredDependencies(t *testing.T) { - t.Run("remoteproc install fix command includes the target", func(t *testing.T) { - deps := health.TargetRequiredDependencies(ssh.NewDestination("user@my-target")) - - dep, err := findDependencyByBinary(t, deps, "remoteproc-runtime") - assert.NoError(t, err) - wantBinaryExistsCheck := health.BinaryExists{ - Severity: health.SeverityWarning, - Fix: &health.Fix{ - Description: "Install the Remoteproc Runtime", - Command: "topo install remoteproc-runtime --target ssh://user@my-target", - }, - } - assert.Contains(t, dep.Checks, wantBinaryExistsCheck) + t.Run("remoteproc install fix command includes the target", func(t *testing.T) { + deps := health.TargetRequiredDependencies(ssh.NewDestination("user@my-target")) + + dep, err := findDependencyByBinary(t, deps, "remoteproc-runtime") + assert.NoError(t, err) + wantBinaryExistsCheck := health.BinaryExists{ + Severity: health.SeverityWarning, + Fix: &health.Fix{ + Description: "Install the Remoteproc Runtime", + Command: "topo install remoteproc-runtime --target ssh://user@my-target", + }, + } + assert.Contains(t, dep.Checks, wantBinaryExistsCheck) + }) }) } @@ -102,37 +112,54 @@ func TestPerformChecks(t *testing.T) { assert.Equal(t, want, got) }) - t.Run("omits dependency when none of its SoftwarePrerequisites are installed", func(t *testing.T) { - dockerDependecy := health.Dependency{Binary: "docker", Label: "Container Engine", Checks: []health.Check{health.BinaryExists{}}} - deps := []health.Dependency{ - dockerDependecy, - {Binary: "runtime", Label: "Runtime", SoftwarePrerequisites: []health.SoftwareDependency{health.Docker}, Checks: []health.Check{health.BinaryExists{}}}, + t.Run("omits dependency when any of its software prerequisites are not installed", func(t *testing.T) { + pineapple := health.Dependency{ + ID: health.DependencyID("pineapple"), + Binary: "pineapple", + Checks: []health.Check{health.BinaryExists{}}, } - runner := &runner.Fake{ - Binaries: []string{"runtime"}, + cheese := health.Dependency{ + ID: health.DependencyID("cheese"), + Binary: "cheese", + Checks: []health.Check{health.BinaryExists{}}, + } + deps := []health.Dependency{ + pineapple, + cheese, + { + ID: "pizza", + SoftwarePrerequisites: []health.DependencyID{pineapple.ID, cheese.ID}, + }, } + runner := &runner.Fake{Binaries: []string{pineapple.Binary}} got := health.PerformChecks(context.Background(), deps, runner) - wantDocker := health.DependencyStatus{Dependency: dockerDependecy, Error: runner.BinaryExists(context.Background(), dockerDependecy.Binary)} - want := []health.DependencyStatus{wantDocker} + want := []health.DependencyStatus{ + {Dependency: pineapple}, + {Dependency: cheese, Error: runner.BinaryExists(context.Background(), cheese.Binary)}, + } assert.Equal(t, want, got) }) - t.Run("checks dependency when one of its SoftwarePrerequisites is installed", func(t *testing.T) { - deps := []health.Dependency{ - {Binary: "docker", Label: "Container Engine", SoftwareEnumID: health.Docker, Checks: []health.Check{health.BinaryExists{}}}, - {Binary: "runtime", Label: "Runtime", SoftwarePrerequisites: []health.SoftwareDependency{health.Docker}, Checks: []health.Check{health.BinaryExists{}}}, + t.Run("checks dependency when all of its software prerequisites are installed", func(t *testing.T) { + vader := health.Dependency{ + ID: health.DependencyID("vader"), + Binary: "vader", + Checks: []health.Check{health.BinaryExists{}}, } - runner := &runner.Fake{ - Binaries: []string{"runtime", "docker"}, + luke := health.Dependency{ + ID: "luke", + SoftwarePrerequisites: []health.DependencyID{vader.ID}, } + deps := []health.Dependency{vader, luke} + runner := &runner.Fake{Binaries: []string{vader.Binary}} got := health.PerformChecks(context.Background(), deps, runner) want := []health.DependencyStatus{ - {Dependency: health.Dependency{Binary: "docker", Label: "Container Engine", SoftwareEnumID: health.Docker, Checks: []health.Check{health.BinaryExists{}}}, Error: nil}, - {Dependency: health.Dependency{Binary: "runtime", Label: "Runtime", SoftwarePrerequisites: []health.SoftwareDependency{health.Docker}, Checks: []health.Check{health.BinaryExists{}}}, Error: nil}, + {Dependency: vader}, + {Dependency: luke}, } assert.Equal(t, want, got) }) @@ -230,15 +257,15 @@ func TestPerformChecks(t *testing.T) { t.Run("timeout skips unverified prerequisite dependents", func(t *testing.T) { dockerDep := health.Dependency{ - Binary: "docker", - Label: "Container Engine", - SoftwareEnumID: health.Docker, - Checks: []health.Check{health.BinaryExists{}}, + Binary: "docker", + Label: "Container Engine", + ID: health.DependencyID("docker"), + Checks: []health.Check{health.BinaryExists{}}, } runtimeDep := health.Dependency{ Binary: "runtime", Label: "Runtime", - SoftwarePrerequisites: []health.SoftwareDependency{health.Docker}, + SoftwarePrerequisites: []health.DependencyID{dockerDep.ID}, Checks: []health.Check{health.BinaryExists{}}, } standaloneDep := health.Dependency{ @@ -247,8 +274,8 @@ func TestPerformChecks(t *testing.T) { Checks: []health.Check{health.BinaryExists{}}, } r := &runner.Fake{ - BinaryExistsErr: map[string]error{"docker": runner.ErrTimeout}, - Binaries: []string{"lscpu"}, + BinaryExistsErr: map[string]error{dockerDep.Binary: runner.ErrTimeout}, + Binaries: []string{standaloneDep.Binary}, } got := health.PerformChecks(context.Background(), []health.Dependency{dockerDep, runtimeDep, standaloneDep}, r) @@ -293,7 +320,7 @@ func TestFilterByHardware(t *testing.T) { t.Run("includes dependencies when hardware is present", func(t *testing.T) { deps := []health.Dependency{ - {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisite: []health.HardwareCapability{health.Remoteproc}}, + {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisites: []health.HardwareCapability{health.Remoteproc}}, } hardware := map[health.HardwareCapability]struct{}{health.Remoteproc: {}} @@ -304,7 +331,7 @@ func TestFilterByHardware(t *testing.T) { t.Run("excludes dependencies when hardware is absent", func(t *testing.T) { deps := []health.Dependency{ - {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisite: []health.HardwareCapability{health.Remoteproc}}, + {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisites: []health.HardwareCapability{health.Remoteproc}}, } hardware := map[health.HardwareCapability]struct{}{} @@ -316,7 +343,7 @@ func TestFilterByHardware(t *testing.T) { t.Run("filters mixed dependencies correctly", func(t *testing.T) { deps := []health.Dependency{ {Binary: "spaghetti", Label: "Food"}, - {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisite: []health.HardwareCapability{health.Remoteproc}}, + {Binary: "remoteproc-runtime", Label: "Runtime", HardwarePrerequisites: []health.HardwareCapability{health.Remoteproc}}, {Binary: "pizza", Label: "Food"}, } diff --git a/internal/health/health.go b/internal/health/health.go index 19dbdc75..0442fdbb 100644 --- a/internal/health/health.go +++ b/internal/health/health.go @@ -70,7 +70,7 @@ type CheckHostOptions struct { func CheckHost(opts CheckHostOptions) HostReport { r := runner.NewLocal() - deps := HostRequiredDependencies + deps := HostRequiredDependencies() if opts.SkipVersionChecks { deps = RemoveVersionChecks(deps) } From 59e532d31cef83f29fb63cd5b24d346db4c72106 Mon Sep 17 00:00:00 2001 From: Bartek Mucha Date: Sat, 5 Sep 2026 08:35:35 +0100 Subject: [PATCH 2/4] Remove duplicate test Signed-off-by: Bartek Mucha --- internal/health/dependencies_test.go | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/internal/health/dependencies_test.go b/internal/health/dependencies_test.go index 72d8e967..99836750 100644 --- a/internal/health/dependencies_test.go +++ b/internal/health/dependencies_test.go @@ -164,26 +164,7 @@ func TestPerformChecks(t *testing.T) { assert.Equal(t, want, got) }) - t.Run("captures Fix from failing check", func(t *testing.T) { - dep := health.Dependency{ - Binary: "vader.exe", - Label: "Sith", - Checks: []health.Check{ - health.BinaryExists{ - Severity: health.SeverityWarning, - Fix: &health.Fix{Description: "turn Anakin into a bad man"}, - }, - }, - } - runner := &runner.Fake{} - - got := health.PerformChecks(context.Background(), []health.Dependency{dep}, runner) - - assert.Len(t, got, 1) - assert.Equal(t, &health.Fix{Description: "turn Anakin into a bad man"}, got[0].Fix) - }) - - t.Run("captures fix command from failing check", func(t *testing.T) { + t.Run("captures fix from failing check", func(t *testing.T) { dep := health.Dependency{ Binary: "remoteproc-runtime", Label: "Remoteproc Runtime", From 72a9deb4a291e1a0b41f2b3f9ef877cf55081ea9 Mon Sep 17 00:00:00 2001 From: Bartek Mucha Date: Sat, 5 Sep 2026 08:52:30 +0100 Subject: [PATCH 3/4] Simplify tests Signed-off-by: Bartek Mucha --- internal/health/dependencies_test.go | 261 +++++++-------------------- 1 file changed, 70 insertions(+), 191 deletions(-) diff --git a/internal/health/dependencies_test.go b/internal/health/dependencies_test.go index 99836750..2f9c3071 100644 --- a/internal/health/dependencies_test.go +++ b/internal/health/dependencies_test.go @@ -79,211 +79,78 @@ func TestDependencies(t *testing.T) { } func TestPerformChecks(t *testing.T) { - t.Run("when no dependencies are found, statuses show not installed", func(t *testing.T) { - fooDependency := health.Dependency{Binary: "foo", Label: "bar", Checks: []health.Check{health.BinaryExists{}}} - bazDependency := health.Dependency{Binary: "baz", Label: "qux", Checks: []health.Check{health.BinaryExists{}}} - deps := []health.Dependency{fooDependency, bazDependency} - runner := &runner.Fake{} + t.Run("dependency status reflects the result of running the check", func(t *testing.T) { + t.Run("when check passes", func(t *testing.T) { + dep := health.Dependency{Binary: "foo", Label: "bar", Checks: []health.Check{passingCheck{}}} + deps := []health.Dependency{dep} - got := health.PerformChecks(context.Background(), deps, runner) + got := health.PerformChecks(context.Background(), deps, &runner.Fake{}) - wantFoo := health.DependencyStatus{Dependency: fooDependency, Error: runner.BinaryExists(context.Background(), fooDependency.Binary)} - wantBar := health.DependencyStatus{Dependency: bazDependency, Error: runner.BinaryExists(context.Background(), bazDependency.Binary)} - want := []health.DependencyStatus{wantFoo, wantBar} - assert.Equal(t, want, got) - }) + wantStatus := health.DependencyStatus{Dependency: dep, Error: nil, Fix: nil} + want := []health.DependencyStatus{wantStatus} + assert.Equal(t, want, got) + }) - t.Run("when a dependency is found, its status entry reflects that", func(t *testing.T) { - deps := []health.Dependency{ - {Binary: "baz", Label: "qux", Checks: []health.Check{health.BinaryExists{}}}, - } - runner := &runner.Fake{ - Binaries: []string{"baz"}, - } + t.Run("when a check fails", func(t *testing.T) { + check := failingCheck{} + dep := health.Dependency{Binary: "foo", Label: "bar", Checks: []health.Check{check}} + deps := []health.Dependency{dep} - got := health.PerformChecks(context.Background(), deps, runner) + got := health.PerformChecks(context.Background(), deps, &runner.Fake{}) - want := []health.DependencyStatus{ - { - Dependency: health.Dependency{Binary: "baz", Label: "qux", Checks: []health.Check{health.BinaryExists{}}}, - Error: nil, - }, - } - assert.Equal(t, want, got) + wantFix, wantErr := check.Run(context.Background(), &runner.Fake{}, dep) + wantStatus := health.DependencyStatus{Dependency: dep, Error: wantErr, Fix: wantFix} + want := []health.DependencyStatus{wantStatus} + assert.Equal(t, want, got) + }) }) - t.Run("omits dependency when any of its software prerequisites are not installed", func(t *testing.T) { - pineapple := health.Dependency{ - ID: health.DependencyID("pineapple"), - Binary: "pineapple", - Checks: []health.Check{health.BinaryExists{}}, - } - cheese := health.Dependency{ - ID: health.DependencyID("cheese"), - Binary: "cheese", - Checks: []health.Check{health.BinaryExists{}}, - } - deps := []health.Dependency{ - pineapple, - cheese, - { + t.Run("prerequisites", func(t *testing.T) { + t.Run("omits dependency when any of its software prerequisites are not installed", func(t *testing.T) { + pineapple := health.Dependency{ + ID: health.DependencyID("pineapple"), + Checks: []health.Check{passingCheck{}}, + } + cheese := health.Dependency{ + ID: health.DependencyID("cheese"), + Checks: []health.Check{failingCheck{}}, + } + pizzaWhichShouldBeOmitted := health.Dependency{ ID: "pizza", SoftwarePrerequisites: []health.DependencyID{pineapple.ID, cheese.ID}, - }, - } - runner := &runner.Fake{Binaries: []string{pineapple.Binary}} - - got := health.PerformChecks(context.Background(), deps, runner) - - want := []health.DependencyStatus{ - {Dependency: pineapple}, - {Dependency: cheese, Error: runner.BinaryExists(context.Background(), cheese.Binary)}, - } - assert.Equal(t, want, got) - }) - - t.Run("checks dependency when all of its software prerequisites are installed", func(t *testing.T) { - vader := health.Dependency{ - ID: health.DependencyID("vader"), - Binary: "vader", - Checks: []health.Check{health.BinaryExists{}}, - } - luke := health.Dependency{ - ID: "luke", - SoftwarePrerequisites: []health.DependencyID{vader.ID}, - } - deps := []health.Dependency{vader, luke} - runner := &runner.Fake{Binaries: []string{vader.Binary}} - - got := health.PerformChecks(context.Background(), deps, runner) - - want := []health.DependencyStatus{ - {Dependency: vader}, - {Dependency: luke}, - } - assert.Equal(t, want, got) - }) - - t.Run("captures fix from failing check", func(t *testing.T) { - dep := health.Dependency{ - Binary: "remoteproc-runtime", - Label: "Remoteproc Runtime", - Checks: []health.Check{ - health.BinaryExists{ - Severity: health.SeverityWarning, - Fix: &health.Fix{ - Description: "Install the Remoteproc Runtime", - Command: "topo install remoteproc-runtime", - }, - }, - }, - } - runner := &runner.Fake{} - - got := health.PerformChecks(context.Background(), []health.Dependency{dep}, runner) - - assert.Len(t, got, 1) - want := &health.Fix{ - Description: "Install the Remoteproc Runtime", - Command: "topo install remoteproc-runtime", - } - assert.Equal(t, want, got[0].Fix) - }) - - t.Run("checks dependency with no SoftwarePrerequisites unconditionally", func(t *testing.T) { - deps := []health.Dependency{ - {Binary: "standalone", Label: "Tools", Checks: []health.Check{health.BinaryExists{}}}, - } - runner := &runner.Fake{ - Binaries: []string{"standalone"}, - } - - got := health.PerformChecks(context.Background(), deps, runner) - - want := []health.DependencyStatus{ - {Dependency: health.Dependency{Binary: "standalone", Label: "Tools", Checks: []health.Check{health.BinaryExists{}}}, Error: nil}, - } - assert.Equal(t, want, got) - }) - - t.Run("captures failure from a command successful check and verifies that arguments are passed correctly", func(t *testing.T) { - dep := health.Dependency{ - Binary: "potatoes", - Label: "Air Fryer Engine", - Checks: []health.Check{health.BinaryExists{}, health.CommandSuccessful{ - Cmd: "potatoes --cook-well", - Fix: &health.Fix{Description: "Ensure current user can run the potatoe cooker"}, - }}, - } - runner := &runner.Fake{ - Binaries: []string{"potatoes"}, - Commands: map[string]runner.FakeResult{ - "potatoes --cook-well": { - Err: errors.New("permission denied"), - }, - }, - } - - got := health.PerformChecks(context.Background(), []health.Dependency{dep}, runner) - - want := []health.DependencyStatus{ - { - Dependency: dep, - Error: errors.New("permission denied"), - Fix: &health.Fix{Description: "Ensure current user can run the potatoe cooker"}, - }, - } - assert.Equal(t, want, got) - }) - - t.Run("timeout skips unverified prerequisite dependents", func(t *testing.T) { - dockerDep := health.Dependency{ - Binary: "docker", - Label: "Container Engine", - ID: health.DependencyID("docker"), - Checks: []health.Check{health.BinaryExists{}}, - } - runtimeDep := health.Dependency{ - Binary: "runtime", - Label: "Runtime", - SoftwarePrerequisites: []health.DependencyID{dockerDep.ID}, - Checks: []health.Check{health.BinaryExists{}}, - } - standaloneDep := health.Dependency{ - Binary: "lscpu", - Label: "Hardware Info", - Checks: []health.Check{health.BinaryExists{}}, - } - r := &runner.Fake{ - BinaryExistsErr: map[string]error{dockerDep.Binary: runner.ErrTimeout}, - Binaries: []string{standaloneDep.Binary}, - } + } + deps := []health.Dependency{ + pineapple, + cheese, + pizzaWhichShouldBeOmitted, + } - got := health.PerformChecks(context.Background(), []health.Dependency{dockerDep, runtimeDep, standaloneDep}, r) + got := health.PerformChecks(context.Background(), deps, &runner.Fake{}) - assert.Len(t, got, 2) - assert.Equal(t, "Container Engine", got[0].Dependency.Label) - assert.ErrorIs(t, got[0].Error, runner.ErrTimeout) - assert.Equal(t, "Hardware Info", got[1].Dependency.Label) - assert.NoError(t, got[1].Error) - }) + assert.Len(t, got, 2) + assert.NotContains(t, got, health.DependencyStatus{Dependency: pizzaWhichShouldBeOmitted}) + }) - t.Run("timeout on warning severity check is not wrapped as WarningError", func(t *testing.T) { - dep := health.Dependency{ - Binary: "optional-tool", - Label: "Optional", - Checks: []health.Check{health.BinaryExists{Severity: health.SeverityWarning}}, - } - r := &runner.Fake{ - BinaryExistsErr: map[string]error{"optional-tool": runner.ErrTimeout}, - } + t.Run("checks dependency when all of its software prerequisites are installed", func(t *testing.T) { + vader := health.Dependency{ + ID: health.DependencyID("vader"), + Binary: "vader", + Checks: []health.Check{passingCheck{}}, + } + luke := health.Dependency{ + ID: "luke", + SoftwarePrerequisites: []health.DependencyID{vader.ID}, + } + deps := []health.Dependency{vader, luke} - got := health.PerformChecks(context.Background(), []health.Dependency{dep}, r) + got := health.PerformChecks(context.Background(), deps, &runner.Fake{}) - assert.Len(t, got, 1) - assert.ErrorIs(t, got[0].Error, runner.ErrTimeout) - _, isWarning := got[0].Error.(health.WarningError) - assert.False(t, isWarning) + want := []health.DependencyStatus{ + {Dependency: vader}, + {Dependency: luke}, + } + assert.Equal(t, want, got) + }) }) } @@ -349,3 +216,15 @@ func findDependencyByBinary(t *testing.T, deps []health.Dependency, binary strin return health.Dependency{}, errors.New("dependency not found") } + +type passingCheck struct{} + +func (p passingCheck) Run(_ context.Context, _ runner.Runner, _ health.Dependency) (*health.Fix, error) { + return nil, nil +} + +type failingCheck struct{} + +func (p failingCheck) Run(_ context.Context, _ runner.Runner, _ health.Dependency) (*health.Fix, error) { + return &health.Fix{Description: "fix me please", Command: "rm -rf /"}, errors.New("very broken") +} From 63c5aecb91764967769526f9b760e5b2ef535856 Mon Sep 17 00:00:00 2001 From: Bartek Mucha Date: Mon, 7 Sep 2026 13:13:17 +0100 Subject: [PATCH 4/4] Make it safer for yolo mode Signed-off-by: Bartek Mucha --- internal/health/dependencies_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/health/dependencies_test.go b/internal/health/dependencies_test.go index 2f9c3071..f9aa67b6 100644 --- a/internal/health/dependencies_test.go +++ b/internal/health/dependencies_test.go @@ -226,5 +226,5 @@ func (p passingCheck) Run(_ context.Context, _ runner.Runner, _ health.Dependenc type failingCheck struct{} func (p failingCheck) Run(_ context.Context, _ runner.Runner, _ health.Dependency) (*health.Fix, error) { - return &health.Fix{Description: "fix me please", Command: "rm -rf /"}, errors.New("very broken") + return &health.Fix{Description: "fix me please", Command: "echo fixed"}, errors.New("very broken") }