Skip to content

Commit f14866b

Browse files
refactor: extract stopWithRetry to shared test utility
Address code review feedback by extracting the duplicated stopWithRetry function to a shared test utility package, following the DRY principle. Changes: - Add StopWithRetry to test/utils.go with comprehensive documentation - Update api/v1/suite_test.go to use test.StopWithRetry - Update internal/operator-controller/controllers/suite_test.go to use test.StopWithRetry - Remove duplicate implementations from both files This makes the retry logic easier to maintain - any updates only need to be made in one place rather than in multiple test files. Assisted-by: Cursor/Claude Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 8e504ee commit f14866b

3 files changed

Lines changed: 25 additions & 40 deletions

File tree

api/v1/suite_test.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -59,26 +59,7 @@ func TestMain(m *testing.M) {
5959
code := m.Run()
6060
// Use Eventually wrapper for graceful test environment teardown
6161
// controller-runtime v0.23.0+ requires this to prevent timing-related errors
62-
stopErr := stopWithRetry(testEnv, time.Minute, time.Second)
62+
stopErr := test.StopWithRetry(testEnv, time.Minute, time.Second)
6363
utilruntime.Must(stopErr)
6464
os.Exit(code)
6565
}
66-
67-
// stopWithRetry wraps testEnv.Stop() with retry logic for graceful shutdown
68-
func stopWithRetry(env interface{ Stop() error }, timeout, interval time.Duration) error {
69-
deadline := time.Now().Add(timeout)
70-
var lastErr error
71-
for time.Now().Before(deadline) {
72-
if err := env.Stop(); err == nil {
73-
return nil
74-
} else {
75-
lastErr = err
76-
log.Printf("stopWithRetry: env.Stop() failed during teardown, retrying in %s: %v", interval, err)
77-
}
78-
time.Sleep(interval)
79-
}
80-
if lastErr != nil {
81-
log.Printf("stopWithRetry: timeout reached before successful teardown; last error: %v", lastErr)
82-
}
83-
return env.Stop() // Final attempt
84-
}

internal/operator-controller/controllers/suite_test.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -134,26 +134,7 @@ func TestMain(m *testing.M) {
134134
code := m.Run()
135135
// Use Eventually wrapper for graceful test environment teardown
136136
// controller-runtime v0.23.0+ requires this to prevent timing-related errors
137-
stopErr := stopWithRetry(testEnv, time.Minute, time.Second)
137+
stopErr := test.StopWithRetry(testEnv, time.Minute, time.Second)
138138
utilruntime.Must(stopErr)
139139
os.Exit(code)
140140
}
141-
142-
// stopWithRetry wraps testEnv.Stop() with retry logic for graceful shutdown
143-
func stopWithRetry(env interface{ Stop() error }, timeout, interval time.Duration) error {
144-
deadline := time.Now().Add(timeout)
145-
var lastErr error
146-
for time.Now().Before(deadline) {
147-
if err := env.Stop(); err == nil {
148-
return nil
149-
} else {
150-
lastErr = err
151-
log.Printf("stopWithRetry: env.Stop() failed during teardown, retrying in %s: %v", interval, err)
152-
}
153-
time.Sleep(interval)
154-
}
155-
if lastErr != nil {
156-
log.Printf("stopWithRetry: timeout reached before successful teardown; last error: %v", lastErr)
157-
}
158-
return env.Stop() // Final attempt
159-
}

test/utils.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package test
22

33
import (
4+
"log"
45
"os"
56
"path"
67
"path/filepath"
78
"runtime"
9+
"time"
810

911
"sigs.k8s.io/controller-runtime/pkg/envtest"
1012
)
@@ -53,3 +55,24 @@ func getFirstFoundEnvTestBinaryDir() string {
5355
}
5456
return ""
5557
}
58+
59+
// StopWithRetry wraps testEnv.Stop() with retry logic for graceful shutdown.
60+
// This is useful for controller-runtime v0.23.0+ where direct calls to testEnv.Stop()
61+
// can fail intermittently due to graceful shutdown timing.
62+
func StopWithRetry(env interface{ Stop() error }, timeout, interval time.Duration) error {
63+
deadline := time.Now().Add(timeout)
64+
var lastErr error
65+
for time.Now().Before(deadline) {
66+
if err := env.Stop(); err == nil {
67+
return nil
68+
} else {
69+
lastErr = err
70+
log.Printf("StopWithRetry: env.Stop() failed during teardown, retrying in %s: %v", interval, err)
71+
}
72+
time.Sleep(interval)
73+
}
74+
if lastErr != nil {
75+
log.Printf("StopWithRetry: timeout reached before successful teardown; last error: %v", lastErr)
76+
}
77+
return env.Stop() // Final attempt
78+
}

0 commit comments

Comments
 (0)