Skip to content

Commit f9b2402

Browse files
mchmarnymchmarny
committed
test: add missing test coverage for k8s/pod, replace custom contains helpers
Replace placeholder tests with real unit tests: - job_test.go: test WaitForJobCompletion with fake watcher (success, failure, timeout, cancel) - logs_test.go: test StreamLogs and GetPodLogs with fake client (error paths, cancelled context) - wait_test.go: test WaitForPodReady with fake client (ready, failed, timeout, cancel) Replace hand-rolled substring helpers with stdlib: - Replace contains(s, substr) with strings.Contains across 12 test files - Replace contains(slice, substr) with slices.Contains in checks_test.go - Replace containsString/containsHelper/findSubstr variants
1 parent 79c454e commit f9b2402

23 files changed

+307
-184
lines changed

‎pkg/bundler/validations/checks_test.go‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ package validations
1616

1717
import (
1818
"context"
19-
"strings"
19+
"slices"
2020
"testing"
2121

2222
"github.com/NVIDIA/aicr/pkg/bundler/config"
@@ -129,7 +129,7 @@ func TestCheckWorkloadSelectorMissing(t *testing.T) {
129129
}
130130

131131
if tt.wantWarningMsg != "" && len(warnings) > 0 {
132-
if !contains(warnings, tt.wantWarningMsg) {
132+
if !slices.Contains(warnings, tt.wantWarningMsg) {
133133
t.Errorf("CheckWorkloadSelectorMissing() warning message = %v, want to contain %q", warnings, tt.wantWarningMsg)
134134
}
135135
}
@@ -260,7 +260,7 @@ func TestCheckAcceleratedSelectorMissing(t *testing.T) {
260260
}
261261

262262
if tt.wantWarningMsg != "" && len(warnings) > 0 {
263-
if !contains(warnings, tt.wantWarningMsg) {
263+
if !slices.Contains(warnings, tt.wantWarningMsg) {
264264
t.Errorf("CheckAcceleratedSelectorMissing() warning message = %v, want to contain %q", warnings, tt.wantWarningMsg)
265265
}
266266
}
@@ -386,13 +386,3 @@ func TestCheckConditions(t *testing.T) {
386386
})
387387
}
388388
}
389-
390-
// contains checks if a slice of strings contains a substring.
391-
func contains(slice []string, substr string) bool {
392-
for _, s := range slice {
393-
if strings.Contains(s, substr) {
394-
return true
395-
}
396-
}
397-
return false
398-
}

‎pkg/bundler/validations/registry.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import (
2121
"strings"
2222
"sync"
2323

24-
aicrerrors "github.com/NVIDIA/aicr/pkg/errors"
2524
"github.com/NVIDIA/aicr/pkg/bundler/config"
25+
aicrerrors "github.com/NVIDIA/aicr/pkg/errors"
2626
"github.com/NVIDIA/aicr/pkg/recipe"
2727
)
2828

‎pkg/cli/root.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,14 @@ func Execute() {
138138
}
139139

140140
ctx, cancel := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
141-
defer cancel()
142141

143142
if err := cmd.Run(ctx, os.Args); err != nil {
143+
cancel()
144144
exitCode := errors.ExitCodeFromError(err)
145145
slog.Error("command failed", "error", err, "exitCode", exitCode)
146146
os.Exit(exitCode)
147147
}
148+
cancel()
148149
}
149150

150151
func commandLister(_ context.Context, cmd *cli.Command) {

‎pkg/cli/snapshot.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import (
1919

2020
"github.com/urfave/cli/v3"
2121

22-
"github.com/NVIDIA/aicr/pkg/defaults"
2322
"github.com/NVIDIA/aicr/pkg/collector"
23+
"github.com/NVIDIA/aicr/pkg/defaults"
2424
"github.com/NVIDIA/aicr/pkg/errors"
2525
"github.com/NVIDIA/aicr/pkg/serializer"
2626
"github.com/NVIDIA/aicr/pkg/snapshotter"

‎pkg/component/base_test.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"os"
2020
"path/filepath"
21+
"strings"
2122
"testing"
2223
"time"
2324

@@ -281,7 +282,7 @@ func TestBaseBundler_GenerateChecksums(t *testing.T) {
281282

282283
contentStr := string(content)
283284
for _, tf := range testFiles {
284-
if !filepath.IsAbs(tf.name) && !contains(contentStr, tf.name) {
285+
if !filepath.IsAbs(tf.name) && !strings.Contains(contentStr, tf.name) {
285286
t.Errorf("Checksums file does not contain %s", tf.name)
286287
}
287288
}
@@ -540,10 +541,6 @@ func TestBaseBundler_GenerateFileFromTemplate(t *testing.T) {
540541
})
541542
}
542543
}
543-
func contains(s, substr string) bool {
544-
return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && (s[:len(substr)] == substr || contains(s[1:], substr)))
545-
}
546-
547544
func TestGetBundlerVersion(t *testing.T) {
548545
tests := []struct {
549546
name string

‎pkg/component/helpers_test.go‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package component
1616

1717
import (
18+
"strings"
1819
"testing"
1920
)
2021

@@ -291,16 +292,16 @@ func TestMarshalYAMLWithHeader(t *testing.T) {
291292
RecipeVersion: "2.0.0",
292293
},
293294
verify: func(t *testing.T, result string) {
294-
if !contains(result, "# GPU Operator Helm Values") {
295+
if !strings.Contains(result, "# GPU Operator Helm Values") {
295296
t.Error("missing component name in header")
296297
}
297-
if !contains(result, "# Bundler Version: 1.2.3") {
298+
if !strings.Contains(result, "# Bundler Version: 1.2.3") {
298299
t.Error("missing bundler version in header")
299300
}
300-
if !contains(result, "# Recipe Version: 2.0.0") {
301+
if !strings.Contains(result, "# Recipe Version: 2.0.0") {
301302
t.Error("missing recipe version in header")
302303
}
303-
if !contains(result, "key: value") {
304+
if !strings.Contains(result, "key: value") {
304305
t.Error("missing YAML content")
305306
}
306307
},
@@ -314,10 +315,10 @@ func TestMarshalYAMLWithHeader(t *testing.T) {
314315
RecipeVersion: "",
315316
},
316317
verify: func(t *testing.T, result string) {
317-
if !contains(result, "# Generated from Cloud Native Stack Recipe") {
318+
if !strings.Contains(result, "# Generated from Cloud Native Stack Recipe") {
318319
t.Error("missing standard header line")
319320
}
320-
if !contains(result, "test: data") {
321+
if !strings.Contains(result, "test: data") {
321322
t.Error("missing YAML content")
322323
}
323324
},
@@ -339,13 +340,13 @@ func TestMarshalYAMLWithHeader(t *testing.T) {
339340
RecipeVersion: "v2.0.0",
340341
},
341342
verify: func(t *testing.T, result string) {
342-
if !contains(result, "driver:") {
343+
if !strings.Contains(result, "driver:") {
343344
t.Error("missing driver section")
344345
}
345-
if !contains(result, "version: 550.0.0") {
346+
if !strings.Contains(result, "version: 550.0.0") {
346347
t.Error("missing driver version")
347348
}
348-
if !contains(result, "mig:") {
349+
if !strings.Contains(result, "mig:") {
349350
t.Error("missing mig section")
350351
}
351352
},
@@ -359,7 +360,7 @@ func TestMarshalYAMLWithHeader(t *testing.T) {
359360
RecipeVersion: "1.0.0",
360361
},
361362
verify: func(t *testing.T, result string) {
362-
if !contains(result, "null") {
363+
if !strings.Contains(result, "null") {
363364
t.Error("nil should serialize to null")
364365
}
365366
},
@@ -373,10 +374,10 @@ func TestMarshalYAMLWithHeader(t *testing.T) {
373374
RecipeVersion: "1.0.0",
374375
},
375376
verify: func(t *testing.T, result string) {
376-
if !contains(result, "- item1") {
377+
if !strings.Contains(result, "- item1") {
377378
t.Error("missing first item")
378379
}
379-
if !contains(result, "- item2") {
380+
if !strings.Contains(result, "- item2") {
380381
t.Error("missing second item")
381382
}
382383
},

‎pkg/k8s/agent/permissions_test.go‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package agent
1616

1717
import (
1818
"context"
19+
"strings"
1920
"testing"
2021

2122
authv1 "k8s.io/api/authorization/v1"
@@ -73,7 +74,7 @@ func TestCheckPermissions(t *testing.T) {
7374
}
7475

7576
if tt.wantErr && err != nil && tt.errContains != "" {
76-
if !contains(err.Error(), tt.errContains) {
77+
if !strings.Contains(err.Error(), tt.errContains) {
7778
t.Errorf("CheckPermissions() error = %v, should contain %q", err, tt.errContains)
7879
}
7980
}
@@ -153,18 +154,3 @@ func TestCheckPermission(t *testing.T) {
153154
})
154155
}
155156
}
156-
157-
func contains(s, substr string) bool {
158-
return len(s) >= len(substr) && (s == substr || len(s) > len(substr) &&
159-
(s[:len(substr)] == substr || s[len(s)-len(substr):] == substr ||
160-
len(s) > len(substr)+1 && findSubstring(s, substr)))
161-
}
162-
163-
func findSubstring(s, substr string) bool {
164-
for i := 0; i <= len(s)-len(substr); i++ {
165-
if s[i:i+len(substr)] == substr {
166-
return true
167-
}
168-
}
169-
return false
170-
}

‎pkg/k8s/client/client_test.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package client
1717
import (
1818
"os"
1919
"path/filepath"
20+
"strings"
2021
"sync"
2122
"testing"
2223
)
@@ -73,7 +74,7 @@ func TestBuildKubeClient_PathResolution(t *testing.T) {
7374
}
7475

7576
if err != nil && tt.errorContains != "" {
76-
if !containsString(err.Error(), tt.errorContains) {
77+
if !strings.Contains(err.Error(), tt.errorContains) {
7778
t.Errorf("BuildKubeClient() error = %v, want error containing %q", err, tt.errorContains)
7879
}
7980
}
@@ -124,7 +125,7 @@ func TestBuildKubeClient_ExplicitPath(t *testing.T) {
124125
t.Error("BuildKubeClient() with invalid config should return error")
125126
}
126127

127-
if !containsString(err.Error(), "failed to build kube config") {
128+
if !strings.Contains(err.Error(), "failed to build kube config") {
128129
t.Errorf("BuildKubeClient() error = %v, want error containing 'failed to build kube config'", err)
129130
}
130131
}
@@ -220,13 +221,3 @@ func TestGetKubeClient_CallsOnce(t *testing.T) {
220221
t.Errorf("GetKubeClient() returned inconsistent results: %d successes, %d failures", successCount, failCount)
221222
}
222223
}
223-
224-
// containsString checks if a string contains a substring.
225-
func containsString(s, substr string) bool {
226-
for i := 0; i <= len(s)-len(substr); i++ {
227-
if s[i:i+len(substr)] == substr {
228-
return true
229-
}
230-
}
231-
return false
232-
}

‎pkg/k8s/pod/job_test.go‎

Lines changed: 90 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,97 @@
1515
package pod_test
1616

1717
import (
18+
"context"
1819
"testing"
20+
"time"
21+
22+
"github.com/NVIDIA/aicr/pkg/k8s/pod"
23+
batchv1 "k8s.io/api/batch/v1"
24+
corev1 "k8s.io/api/core/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26+
"k8s.io/apimachinery/pkg/watch"
27+
"k8s.io/client-go/kubernetes/fake"
28+
k8stesting "k8s.io/client-go/testing"
1929
)
2030

21-
// Tests for Job operations will be added as needed.
22-
// The implementation is validated through integration tests in k8s/agent and validator/agent.
23-
func TestJobPackage(t *testing.T) {
24-
// Placeholder test to satisfy go test
25-
t.Log("pkg/k8s/pod/job package loaded successfully")
31+
func TestWaitForJobCompletion_Success(t *testing.T) {
32+
job := &batchv1.Job{
33+
ObjectMeta: metav1.ObjectMeta{Name: "test-job", Namespace: "default"},
34+
}
35+
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
36+
client := fake.NewSimpleClientset(job)
37+
38+
watcher := watch.NewFake()
39+
client.PrependWatchReactor("jobs", k8stesting.DefaultWatchReactor(watcher, nil))
40+
41+
go func() {
42+
time.Sleep(50 * time.Millisecond)
43+
completedJob := job.DeepCopy()
44+
completedJob.Status.Conditions = []batchv1.JobCondition{
45+
{Type: batchv1.JobComplete, Status: corev1.ConditionTrue},
46+
}
47+
watcher.Modify(completedJob)
48+
}()
49+
50+
err := pod.WaitForJobCompletion(context.Background(), client, "default", "test-job", 5*time.Second)
51+
if err != nil {
52+
t.Errorf("expected no error, got: %v", err)
53+
}
54+
}
55+
56+
func TestWaitForJobCompletion_Failure(t *testing.T) {
57+
job := &batchv1.Job{
58+
ObjectMeta: metav1.ObjectMeta{Name: "test-job", Namespace: "default"},
59+
}
60+
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
61+
client := fake.NewSimpleClientset(job)
62+
63+
watcher := watch.NewFake()
64+
client.PrependWatchReactor("jobs", k8stesting.DefaultWatchReactor(watcher, nil))
65+
66+
go func() {
67+
time.Sleep(50 * time.Millisecond)
68+
failedJob := job.DeepCopy()
69+
failedJob.Status.Conditions = []batchv1.JobCondition{
70+
{Type: batchv1.JobFailed, Status: corev1.ConditionTrue, Reason: "BackoffLimitExceeded"},
71+
}
72+
watcher.Modify(failedJob)
73+
}()
74+
75+
err := pod.WaitForJobCompletion(context.Background(), client, "default", "test-job", 5*time.Second)
76+
if err == nil {
77+
t.Error("expected error for failed job")
78+
}
79+
}
80+
81+
func TestWaitForJobCompletion_Timeout(t *testing.T) {
82+
job := &batchv1.Job{
83+
ObjectMeta: metav1.ObjectMeta{Name: "test-job", Namespace: "default"},
84+
}
85+
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
86+
client := fake.NewSimpleClientset(job)
87+
88+
watcher := watch.NewFake()
89+
client.PrependWatchReactor("jobs", k8stesting.DefaultWatchReactor(watcher, nil))
90+
91+
err := pod.WaitForJobCompletion(context.Background(), client, "default", "test-job", 100*time.Millisecond)
92+
if err == nil {
93+
t.Error("expected timeout error")
94+
}
95+
}
96+
97+
func TestWaitForJobCompletion_ContextCancelled(t *testing.T) {
98+
//nolint:staticcheck // SA1019: fake.NewSimpleClientset is sufficient for tests
99+
client := fake.NewSimpleClientset()
100+
101+
watcher := watch.NewFake()
102+
client.PrependWatchReactor("jobs", k8stesting.DefaultWatchReactor(watcher, nil))
103+
104+
ctx, cancel := context.WithCancel(context.Background())
105+
cancel()
106+
107+
err := pod.WaitForJobCompletion(ctx, client, "default", "test-job", 5*time.Second)
108+
if err == nil {
109+
t.Error("expected error for cancelled context")
110+
}
26111
}

0 commit comments

Comments
 (0)