Skip to content

Commit caa3728

Browse files
committed
fix unit_test
Signed-off-by: Suleiman Dibirov <[email protected]>
1 parent b05399e commit caa3728

File tree

2 files changed

+43
-21
lines changed

2 files changed

+43
-21
lines changed

pkg/skaffold/deploy/helm/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func VerifyNoCycles(graph map[string][]string) error {
6060
// ensuring that releases are deployed after their dependencies.
6161
func calculateDeploymentOrder(graph map[string][]string) ([]string, error) {
6262
visited := make(map[string]bool)
63-
var order []string
63+
order := make([]string, 0)
6464

6565
var visit func(node string) error
6666
visit = func(node string) error {

pkg/skaffold/deploy/helm/util_test.go

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66

77
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
88
"github.com/GoogleContainerTools/skaffold/v2/testutil"
9-
"github.com/stretchr/testify/require"
109
)
1110

1211
func TestBuildDependencyGraph(t *testing.T) {
@@ -65,12 +64,32 @@ func TestBuildDependencyGraph(t *testing.T) {
6564
graph, err := BuildDependencyGraph(test.releases)
6665

6766
if test.shouldErr {
68-
require.Error(t, err)
67+
t.CheckError(true, err)
6968
return
7069
}
7170

72-
require.NoError(t, err)
73-
require.Equal(t, test.expected, graph)
71+
t.CheckError(false, err)
72+
t.CheckDeepEqual(len(test.expected), len(graph))
73+
74+
for release, deps := range test.expected {
75+
actualDeps, exists := graph[release]
76+
if !exists {
77+
t.Errorf("missing release %s in graph", release)
78+
continue
79+
}
80+
81+
if len(deps) != len(actualDeps) {
82+
t.Errorf("expected %d dependencies for %s, got %d", len(deps), release, len(actualDeps))
83+
continue
84+
}
85+
86+
// Check all expected dependencies exist
87+
for _, dep := range deps {
88+
if !slices.Contains(actualDeps, dep) {
89+
t.Errorf("missing dependency %s for release %s", dep, release)
90+
}
91+
}
92+
}
7493
})
7594
}
7695
}
@@ -137,10 +156,9 @@ func TestVerifyNoCycles(t *testing.T) {
137156
err := VerifyNoCycles(test.graph)
138157

139158
if test.shouldErr {
140-
require.Error(t, err)
141-
require.Contains(t, err.Error(), "cycle detected")
159+
t.CheckErrorContains("cycle detected", err)
142160
} else {
143-
require.NoError(t, err)
161+
t.RequireNoError(err)
144162
}
145163
})
146164
}
@@ -199,27 +217,32 @@ func TestCalculateDeploymentOrder(t *testing.T) {
199217
}
200218

201219
for _, test := range tests {
202-
t.Run(test.description, func(t *testing.T) {
220+
testutil.Run(t, test.description, func(t *testutil.T) {
203221
order, err := calculateDeploymentOrder(test.graph)
204222

205223
if test.shouldErr {
206-
require.Error(t, err)
224+
t.CheckError(true, err)
207225
return
208226
}
209227

210-
require.NoError(t, err)
211-
require.Equal(t, len(test.expected), len(order), "deployment order length mismatch")
228+
t.CheckError(false, err)
212229

213230
// Verify order satisfies dependencies
214231
installed := make(map[string]bool)
215232
for _, release := range order {
216233
// Check all dependencies are installed
217234
for _, dep := range test.graph[release] {
218-
require.True(t, installed[dep],
219-
"release %s deployed before dependency %s", release, dep)
235+
if !installed[dep] {
236+
t.Errorf("dependency %s not installed before %s", dep, release)
237+
}
220238
}
221239
installed[release] = true
222240
}
241+
242+
// Verify all nodes are present
243+
if len(order) != len(test.graph) {
244+
t.Errorf("expected %d nodes, got %d", len(test.graph), len(order))
245+
}
223246
})
224247
}
225248
}
@@ -297,14 +320,13 @@ func TestGroupReleasesByLevel(t *testing.T) {
297320
}
298321

299322
for _, test := range tests {
300-
t.Run(test.description, func(t *testing.T) {
323+
testutil.Run(t, test.description, func(t *testutil.T) {
301324
levels := groupReleasesByLevel(test.order, test.graph)
302325

303-
require.Equal(t, len(test.expected), len(levels), "number of levels mismatch")
326+
t.CheckDeepEqual(len(test.expected), len(levels))
304327

305328
for level, releases := range test.expected {
306-
require.ElementsMatch(t, releases, levels[level],
307-
"releases at level %d don't match", level)
329+
t.CheckDeepEqual(releases, levels[level])
308330
}
309331

310332
// Verify level assignments are correct
@@ -314,9 +336,9 @@ func TestGroupReleasesByLevel(t *testing.T) {
314336
for _, dep := range test.graph[release] {
315337
for depLevel, depReleases := range levels {
316338
if slices.Contains(depReleases, dep) {
317-
require.Less(t, depLevel, level,
318-
"dependency %s at level %d >= release %s at level %d",
319-
dep, depLevel, release, level)
339+
if depLevel >= level {
340+
t.Errorf("dependency %s at level %d >= release %s at level %d", dep, depLevel, release, level)
341+
}
320342
}
321343
}
322344
}

0 commit comments

Comments
 (0)