Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
Signed-off-by: Suleiman Dibirov <[email protected]>
  • Loading branch information
idsulik committed Jan 28, 2025
1 parent 0117802 commit ac07fca
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 14 deletions.
3 changes: 3 additions & 0 deletions pkg/skaffold/deploy/helm/dependencygraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ func NewDependencyGraph(releases []latest.HelmRelease) (*DependencyGraph, error)
releaseNames := make(map[string]bool)

for _, r := range releases {
if _, exists := releaseNames[r.Name]; exists {
return nil, fmt.Errorf("duplicate release name %s", r.Name)
}
releaseNames[r.Name] = true
}

Expand Down
33 changes: 19 additions & 14 deletions pkg/skaffold/deploy/helm/dependencygraph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ import (

func TestNewDependencyGraph(t *testing.T) {
tests := []struct {
description string
releases []latest.HelmRelease
expected map[string][]string
shouldErr bool
description string
releases []latest.HelmRelease
expected map[string][]string
shouldErr bool
errorMessage string
}{
{
description: "simple dependency graph",
Expand Down Expand Up @@ -73,6 +74,16 @@ func TestNewDependencyGraph(t *testing.T) {
{Name: "release2"},
},
shouldErr: true,
errorMessage: "release release1 depends on non-existent release release3",
},
{
description: "duplicate release names",
releases: []latest.HelmRelease{
{Name: "release1"},
{Name: "release1"},
},
shouldErr: true,
errorMessage: "duplicate release name release1",
},
}

Expand All @@ -81,7 +92,7 @@ func TestNewDependencyGraph(t *testing.T) {
graph, err := NewDependencyGraph(test.releases)

if test.shouldErr {
t.CheckError(test.shouldErr, err)
t.CheckErrorContains(test.errorMessage, err)
return
}
t.CheckDeepEqual(len(test.expected), len(graph.graph))
Expand Down Expand Up @@ -142,13 +153,6 @@ func TestHasCycles(t *testing.T) {
},
shouldErr: true,
},
{
description: "self dependency",
releases: []latest.HelmRelease{
{Name: "a", DependsOn: []string{"a"}},
},
shouldErr: true,
},
{
description: "empty graph",
releases: []latest.HelmRelease{},
Expand All @@ -168,8 +172,9 @@ func TestHasCycles(t *testing.T) {

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
graph, _ := NewDependencyGraph(test.releases)
err := graph.HasCycles()
graph, err := NewDependencyGraph(test.releases)
t.CheckError(false, err)
err = graph.HasCycles()

if test.shouldErr {
t.CheckErrorContains("cycle detected", err)
Expand Down

0 comments on commit ac07fca

Please sign in to comment.