Fix flaky TestGenerateUserAgent test using dependency injection (#2561)

The test was flaky due to race conditions from parallel subtests manipulating
the shared global environment variable KUBERNETES_SERVICE_HOST. This caused
intermittent failures when tests executed concurrently.

This fix refactors generateUserAgent() to use dependency injection with the
existing env.Reader interface, allowing tests to use isolated mock environment
readers instead of modifying global state. The tests now run reliably in
parallel without interference.

Changes:
- Add generateUserAgentWithEnv() that accepts an env.Reader parameter
- Keep generateUserAgent() as a wrapper using env.OSReader for production
- Update tests to use gomock-based environment readers
- Remove all os.Setenv/Unsetenv calls from tests
- Tests remain parallel and pass consistently with race detector

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>

Author: JAORMX

pkg/usagemetrics/client.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	rt "github.com/stacklok/toolhive/pkg/container/runtime"
+	"github.com/stacklok/toolhive/pkg/env"
 	"github.com/stacklok/toolhive/pkg/logger"
 	"github.com/stacklok/toolhive/pkg/updates"
 	"github.com/stacklok/toolhive/pkg/versions"
@@ -87,12 +88,18 @@ func (c *Client) SendMetrics(instanceID string, record MetricRecord) error {
 	return nil
 }
 
-// generateUserAgent creates the user agent string
+// generateUserAgent creates the user agent string using the OS environment
 // Format: toolhive/[local|operator] [version] [build_type] (os/arch)
 func generateUserAgent() string {
+	return generateUserAgentWithEnv(&env.OSReader{})
+}
+
+// generateUserAgentWithEnv creates the user agent string using the provided environment reader
+// Format: toolhive/[local|operator] [version] [build_type] (os/arch)
+func generateUserAgentWithEnv(envReader env.Reader) string {
 	// Determine component type
 	envType := "local"
-	if rt.IsKubernetesRuntime() {
+	if rt.IsKubernetesRuntimeWithEnv(envReader) {
 		envType = "operator"
 	}
 

pkg/usagemetrics/client_test.go

@@ -3,12 +3,14 @@ package usagemetrics
 import (
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
+
+	envmocks "github.com/stacklok/toolhive/pkg/env/mocks"
 )
 
 // newTestClient creates a client for testing with a pre-set anonymous ID
@@ -50,15 +52,24 @@ func TestGenerateUserAgent(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			// Set environment variable
-			if tt.k8sEnvValue != "" {
-				os.Setenv("KUBERNETES_SERVICE_HOST", tt.k8sEnvValue)
-				defer os.Unsetenv("KUBERNETES_SERVICE_HOST")
-			} else {
-				os.Unsetenv("KUBERNETES_SERVICE_HOST")
-			}
+			// Create mock environment reader
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockEnv := envmocks.NewMockReader(ctrl)
+
+			// Set up mock expectations
+			mockEnv.EXPECT().
+				Getenv("TOOLHIVE_RUNTIME").
+				Return("").
+				AnyTimes()
+
+			mockEnv.EXPECT().
+				Getenv("KUBERNETES_SERVICE_HOST").
+				Return(tt.k8sEnvValue).
+				AnyTimes()
 
-			userAgent := generateUserAgent()
+			userAgent := generateUserAgentWithEnv(mockEnv)
 
 			// Verify it starts with expected prefix
 			assert.True(t, strings.HasPrefix(userAgent, tt.expectedPrefix),