test: deflake TestBaseMetricObject with testing/synctest (#2686)

# Description

`TestBaseMetricObject` races against real timers and fails
intermittently in the `Test Retina Image` workflow. It failed two runs
in the last week: [Aug 23, merge
queue](https://github.com/microsoft/retina/actions/runs/32652429087),
[Aug 24, PR
run](https://github.com/microsoft/retina/actions/runs/32736068992/attempts/2).
The second failure sits in an earlier attempt of a re-run, so the run
list hides it.

```
basemetricsobject_test.go:100: expected number of goroutines to be the same as before, expected 4, got 3
```

The test has three timing defects:

- It compares two process-wide `runtime.NumGoroutine()` snapshots.
Unrelated goroutine churn breaks the comparison. This caused both CI
failures.
- It sleeps `101ms`, then reads `expireCalled`. The expiration goroutine
writes that variable. The race detector reports the pair.
- The expiration ticker starts before `b.updated`. A late first
assertion can observe the label already expired.

This PR moves the test body into a `testing/synctest` bubble (standard
library since Go 1.25 — `go.mod` requires `go 1.26.0`):

- The fake clock makes the ticker fire deterministically and removes all
real sleeps. This closes the early-expiration gap.
- `synctest.Wait()` orders the callback write before the test reads it.
This removes the data race.
- The bubble fails the test if the expiration goroutine leaks. This
replaces the process-wide count. `t.Cleanup(b.clean)` stops the
goroutine on every exit path.

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/Contributing/overview).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

```bash
go test -tags=unit,dashboard -run 'TestBaseMetricObject' -count=100 ./pkg/module/metrics/
go test -race -tags=unit,dashboard -run 'TestBaseMetricObject' -count=100 ./pkg/module/metrics/
```

Both pass. The 100 plain iterations complete in `0.04s` because the
bubble removes the real sleeps. Before this change, the race detector
failed the test on the first run. `go vet` is clean on the package.

Signed-off-by: Quang Nguyen <28567936+nddq@users.noreply.github.com>

Author: nddq

pkg/module/metrics/basemetricsobject_test.go

@@ -3,9 +3,9 @@
 package metrics
 
 import (
-	"runtime"
 	"slices"
 	"testing"
+	"testing/synctest"
 	"time"
 
 	api "github.com/microsoft/retina/crd/api/v1alpha1"
@@ -42,63 +42,56 @@ func TestBaseMetricObject(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			before := runtime.NumGoroutine()
-			expireCalled := new([]string)
-			b := newBaseMetricsObject(
-				&api.MetricsContextOptions{
-					MetricName: "test_metric",
-				},
-				l,
-				localContext,
-				func(lbs []string) bool {
-					*expireCalled = lbs
-					return true
-				},
-				tt.ttl,
-			)
+			// The bubble gives the test a fake clock, so the expiration
+			// ticker fires deterministically. It also fails the test if the
+			// expiration goroutine is still alive when the bubble ends.
+			synctest.Test(t, func(t *testing.T) {
+				var expireCalled []string
+				b := newBaseMetricsObject(
+					&api.MetricsContextOptions{
+						MetricName: "test_metric",
+					},
+					l,
+					localContext,
+					func(lbs []string) bool {
+						expireCalled = lbs
+						return true
+					},
+					tt.ttl,
+				)
+				t.Cleanup(b.clean)
 
-			testLabels := []string{"test"}
-			b.updated(testLabels)
+				testLabels := []string{"test"}
+				b.updated(testLabels)
 
-			metrics := len(b.trackedMetricLabels())
-			if tt.trackMetrics {
-				if metrics != 1 {
-					t.Errorf("expected 1 tracked metric label, got %d", metrics)
+				metrics := len(b.trackedMetricLabels())
+				if tt.trackMetrics {
+					if metrics != 1 {
+						t.Errorf("expected 1 tracked metric label, got %d", metrics)
+					}
+				} else {
+					if metrics != 0 {
+						t.Errorf("expected 0 tracked metric labels, got %d", metrics)
+					}
 				}
-			} else {
-				if metrics != 0 {
-					t.Errorf("expected 0 tracked metric labels, got %d", metrics)
-				}
-			}
 
-			// If we have a positive TTL, we should see the expire function get called after the TTL has passed
-			if tt.ttl > 0 {
-				time.Sleep(tt.ttl + time.Millisecond*100)
-				if !slices.Equal(*expireCalled, testLabels) {
-					t.Errorf("expected expire to be called with %v, got %v", testLabels, *expireCalled)
-				}
-				metrics = len(b.trackedMetricLabels())
-				if metrics != 0 {
-					t.Errorf("expected 0 tracked metric labels after expiration, got %d", metrics)
+				// If we have a positive TTL, we should see the expire function get called after the TTL has passed
+				if tt.ttl > 0 {
+					// Advance the fake clock to the first tick, then wait for
+					// the expiration goroutine to block again.
+					time.Sleep(tt.ttl)
+					synctest.Wait()
+					if !slices.Equal(expireCalled, testLabels) {
+						t.Errorf("expected expire to be called with %v, got %v", testLabels, expireCalled)
+					}
+					metrics = len(b.trackedMetricLabels())
+					if metrics != 0 {
+						t.Errorf("expected 0 tracked metric labels after expiration, got %d", metrics)
+					}
+				} else if len(expireCalled) != 0 {
+					t.Errorf("expected expire to not be called, but got %v", expireCalled)
 				}
-			} else if len(*expireCalled) != 0 {
-				t.Errorf("expected expire to not be called, but got %v", *expireCalled)
-			}
-
-			b.clean()
-			if b.expireFn != nil {
-				<-b.ctx.Done()
-			}
-
-			// Wait for any goroutines to exit after clean is called
-			if tt.trackMetrics {
-				time.Sleep(tt.ttl + time.Millisecond*100)
-			}
-
-			after := runtime.NumGoroutine()
-			if after != before {
-				t.Errorf("expected number of goroutines to be the same as before, expected %d, got %d", before, after)
-			}
+			})
 		})
 	}
 }