From a3cad0dc914e58d568cd304e8a21ecbeb952b924 Mon Sep 17 00:00:00 2001
From: Jordan Liggitt <liggitt@google.com>
Date: Tue, 1 Apr 2025 13:59:32 -0400
Subject: [PATCH] Clean up leaked goroutines in cache unit tests

Kubernetes-commit: 6747bf7a9cb3009aae4cba1e1d249d56a66a981b
---
 tools/cache/delta_fifo_test.go | 22 ++++++++++++++++++++--
 tools/cache/fifo_test.go       | 14 ++++++++++++--
 tools/cache/main_test.go       | 10 +---------
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/tools/cache/delta_fifo_test.go b/tools/cache/delta_fifo_test.go
index 8f069eb1..9c67012c 100644
--- a/tools/cache/delta_fifo_test.go
+++ b/tools/cache/delta_fifo_test.go
@@ -89,6 +89,15 @@ func testPop(f *DeltaFIFO) testFifoObject {
 	return Pop(f).(Deltas).Newest().Object.(testFifoObject)
 }
 
+// testPopIfAvailable returns `{}, false` if Pop returns a nil object
+func testPopIfAvailable(f *DeltaFIFO) (testFifoObject, bool) {
+	obj := Pop(f)
+	if obj == nil {
+		return testFifoObject{}, false
+	}
+	return obj.(Deltas).Newest().Object.(testFifoObject), true
+}
+
 // literalListerGetter is a KeyListerGetter that is based on a
 // function that returns a slice of objects to list and get.
 // The function must list the same objects every time.
@@ -306,6 +315,7 @@ func TestDeltaFIFOW_ReplaceMakesDeletionsForObjectsOnlyInQueue(t *testing.T) {
 
 func TestDeltaFIFO_addUpdate(t *testing.T) {
 	f := NewDeltaFIFOWithOptions(DeltaFIFOOptions{KeyFunction: testFifoObjectKeyFunc})
+	defer f.Close()
 	f.Add(mkFifoObj("foo", 10))
 	f.Update(mkFifoObj("foo", 12))
 	f.Delete(mkFifoObj("foo", 15))
@@ -320,7 +330,10 @@ func TestDeltaFIFO_addUpdate(t *testing.T) {
 	got := make(chan testFifoObject, 2)
 	go func() {
 		for {
-			obj := testPop(f)
+			obj, ok := testPopIfAvailable(f)
+			if !ok {
+				return
+			}
 			t.Logf("got a thing %#v", obj)
 			t.Logf("D len: %v", len(f.queue))
 			got <- obj
@@ -471,12 +484,17 @@ func TestDeltaFIFO_enqueueingWithLister(t *testing.T) {
 
 func TestDeltaFIFO_addReplace(t *testing.T) {
 	f := NewDeltaFIFOWithOptions(DeltaFIFOOptions{KeyFunction: testFifoObjectKeyFunc})
+	defer f.Close()
 	f.Add(mkFifoObj("foo", 10))
 	f.Replace([]interface{}{mkFifoObj("foo", 15)}, "0")
 	got := make(chan testFifoObject, 2)
 	go func() {
 		for {
-			got <- testPop(f)
+			obj, ok := testPopIfAvailable(f)
+			if !ok {
+				return
+			}
+			got <- obj
 		}
 	}()
 
diff --git a/tools/cache/fifo_test.go b/tools/cache/fifo_test.go
index 1831889b..39c27c6d 100644
--- a/tools/cache/fifo_test.go
+++ b/tools/cache/fifo_test.go
@@ -117,6 +117,7 @@ func TestFIFO_basic(t *testing.T) {
 
 func TestFIFO_addUpdate(t *testing.T) {
 	f := NewFIFO(testFifoObjectKeyFunc)
+	defer f.Close()
 	f.Add(mkFifoObj("foo", 10))
 	f.Update(mkFifoObj("foo", 15))
 
@@ -130,7 +131,11 @@ func TestFIFO_addUpdate(t *testing.T) {
 	got := make(chan testFifoObject, 2)
 	go func() {
 		for {
-			got <- Pop(f).(testFifoObject)
+			obj := Pop(f)
+			if obj == nil {
+				return
+			}
+			got <- obj.(testFifoObject)
 		}
 	}()
 
@@ -151,12 +156,17 @@ func TestFIFO_addUpdate(t *testing.T) {
 
 func TestFIFO_addReplace(t *testing.T) {
 	f := NewFIFO(testFifoObjectKeyFunc)
+	defer f.Close()
 	f.Add(mkFifoObj("foo", 10))
 	f.Replace([]interface{}{mkFifoObj("foo", 15)}, "15")
 	got := make(chan testFifoObject, 2)
 	go func() {
 		for {
-			got <- Pop(f).(testFifoObject)
+			obj := Pop(f)
+			if obj == nil {
+				return
+			}
+			got <- obj.(testFifoObject)
 		}
 	}()
 
diff --git a/tools/cache/main_test.go b/tools/cache/main_test.go
index b0ed00f3..46893ee7 100644
--- a/tools/cache/main_test.go
+++ b/tools/cache/main_test.go
@@ -23,13 +23,5 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	options := []goleak.Option{
-		// These tests run goroutines which get stuck in Pop.
-		// This cannot be fixed without modifying the API.
-		goleak.IgnoreAnyFunction("k8s.io/client-go/tools/cache.TestFIFO_addReplace.func1"),
-		goleak.IgnoreAnyFunction("k8s.io/client-go/tools/cache.TestFIFO_addUpdate.func1"),
-		goleak.IgnoreAnyFunction("k8s.io/client-go/tools/cache.TestDeltaFIFO_addReplace.func1"),
-		goleak.IgnoreAnyFunction("k8s.io/client-go/tools/cache.TestDeltaFIFO_addUpdate.func1"),
-	}
-	goleak.VerifyTestMain(m, options...)
+	goleak.VerifyTestMain(m)
 }