Skip to content

Commit 8e691ff

Browse files
committed
test: harden the finalizer-wait regression guard's timing check
start was captured after the goroutine that fires the second delete was already launched, so elapsed could in principle dip marginally under holdFinalizer if the goroutine's 200ms sleep began before start was assigned. Move the capture before the go statement. Also note in a comment that deleteCalls reaching 2 alone doesn't prove cleanup blocked. The goroutine fires that second delete unconditionally regardless of what cleanupNCCLResources does. The elapsed-time assertion right after it is the real guard. Signed-off-by: Mike Cook <micook@nvidia.com>
1 parent 3241a05 commit 8e691ff

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

validators/performance/nccl_roce_apply_test.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,17 +211,26 @@ func TestCleanupNCCLResources_WaitsForFinalizerHeldNamespace(t *testing.T) {
211211
return false, nil, nil
212212
})
213213

214+
// Captured before the goroutine launches, so elapsed below can't dip
215+
// under holdFinalizer merely because the goroutine's sleep started
216+
// before start was assigned.
217+
start := time.Now()
214218
go func() {
215219
time.Sleep(holdFinalizer)
216220
_ = fakeClient.CoreV1().Namespaces().Delete(context.Background(), ns, metav1.DeleteOptions{})
217221
}()
218222

219-
start := time.Now()
220223
if err := cleanupNCCLResources(fakeClient, ns, ""); err != nil {
221224
t.Fatalf("cleanup should succeed once the finalizer clears, got: %v", err)
222225
}
223226
elapsed := time.Since(start)
224227

228+
// deleteCalls reaching 2 alone doesn't prove cleanup blocked. The
229+
// goroutine above fires the second delete unconditionally at t=200ms
230+
// regardless of what cleanupNCCLResources does. The elapsed-time check
231+
// below is the real guard. A pre-fix early return (reporting success as
232+
// soon as the first Delete call was accepted) would hit it at ~0ms,
233+
// with deleteCalls still at 1.
225234
if got := atomic.LoadInt32(&deleteCalls); got < 2 {
226235
t.Fatalf("expected cleanup to observe the namespace still present and wait for a second delete, got %d delete call(s)", got)
227236
}

0 commit comments

Comments
 (0)