test(azure): DI seam + concurrency timing tests for parallel dispatcher (closes #262)

Introduce four package-level newXxxClientFn vars (defaulting to the real
compute/database/cache/cosmosdb constructors) so tests can inject fake
clients without changing production behaviour.

Add three tests in recommendations_test.go:
- TestGetRecommendations_Parallelism: 4 fakes sleep 100ms each; asserts
  wall-clock < 200ms, proving concurrent dispatch (serial would be ~400ms).
- TestGetRecommendations_OrderPreservation: staggered sleeps force
  out-of-order completion; asserts merged slice is compute->db->cache->cosmos.
- TestGetRecommendations_ErrorIsolation: one fake errors; asserts the other
  three services still contribute recs (no sibling cancellation).

Author: cristim

providers/azure/recommendations.go

@@ -21,6 +21,26 @@ import (
 	"github.com/LeanerCloud/CUDly/providers/azure/services/database"
 )
 
+// serviceRecsGetter is the narrow interface satisfied by each per-service
+// client (compute, database, cache, cosmosdb). The interface exists solely to
+// allow tests to substitute fake implementations; production code uses the
+// concrete types via the newXxxClientFn variables below.
+type serviceRecsGetter interface {
+	GetRecommendations(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error)
+}
+
+// newComputeClientFn, newDatabaseClientFn, newCacheClientFn, and
+// newCosmosDBClientFn default to the real constructors and are overridden in
+// tests to inject fakes. The variables are package-level (not fields on the
+// adapter) so that the constructor signature stays unchanged and the injection
+// is limited to the test package that owns the test binary's address space.
+var (
+	newComputeClientFn  func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { return compute.NewClient(cred, sub, region) }
+	newDatabaseClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { return database.NewClient(cred, sub, region) }
+	newCacheClientFn    func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { return cache.NewClient(cred, sub, region) }
+	newCosmosDBClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { return cosmosdb.NewClient(cred, sub, region) }
+)
+
 // RecommendationsClientAdapter aggregates Azure reservation recommendations across all services.
 //
 // Invariant: subscriptionID must be non-empty. Downstream converters use it as
@@ -99,31 +119,31 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p
 	// Compute (VM) recommendations — subscription-wide.
 	if shouldIncludeService(params, common.ServiceCompute) {
 		goService(&computeErr, func() {
-			computeClient := compute.NewClient(r.cred, r.subscriptionID, "")
+			computeClient := newComputeClientFn(r.cred, r.subscriptionID, "")
 			computeRecs, computeErr = computeClient.GetRecommendations(gctx, params)
 		})
 	}
 
 	// Database (SQL) recommendations — subscription-wide.
 	if shouldIncludeService(params, common.ServiceRelationalDB) {
 		goService(&dbErr, func() {
-			dbClient := database.NewClient(r.cred, r.subscriptionID, "")
+			dbClient := newDatabaseClientFn(r.cred, r.subscriptionID, "")
 			dbRecs, dbErr = dbClient.GetRecommendations(gctx, params)
 		})
 	}
 
 	// Cache (Redis) recommendations — subscription-wide.
 	if shouldIncludeService(params, common.ServiceCache) {
 		goService(&cacheErr, func() {
-			cacheClient := cache.NewClient(r.cred, r.subscriptionID, "")
+			cacheClient := newCacheClientFn(r.cred, r.subscriptionID, "")
 			cacheRecs, cacheErr = cacheClient.GetRecommendations(gctx, params)
 		})
 	}
 
 	// CosmosDB (NoSQL) recommendations — subscription-wide.
 	if shouldIncludeService(params, common.ServiceNoSQL) {
 		goService(&cosmosErr, func() {
-			cosmosClient := cosmosdb.NewClient(r.cred, r.subscriptionID, "")
+			cosmosClient := newCosmosDBClientFn(r.cred, r.subscriptionID, "")
 			cosmosRecs, cosmosErr = cosmosClient.GetRecommendations(gctx, params)
 		})
 	}

providers/azure/recommendations_test.go

@@ -2,7 +2,9 @@ package azure
 
 import (
 	"context"
+	"errors"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
@@ -379,3 +381,164 @@ func TestConvertAdvisorRecommendation_UnknownService(t *testing.T) {
 func strPtr(s string) *string {
 	return &s
 }
+
+// fakeServiceClient is a test double for serviceRecsGetter. It sleeps for
+// sleepDur to simulate network latency and then returns the fixed recs slice
+// (or err when non-nil). sleep is done inside GetRecommendations so that
+// mock latency is isolated to the mock body — the test's assert path never
+// sleeps (see memory feedback_no_sleep_in_tests).
+type fakeServiceClient struct {
+	sleepDur time.Duration
+	recs     []common.Recommendation
+	err      error
+}
+
+func (f *fakeServiceClient) GetRecommendations(ctx context.Context, _ common.RecommendationParams) ([]common.Recommendation, error) {
+	select {
+	case <-time.After(f.sleepDur):
+	case <-ctx.Done():
+		return nil, ctx.Err()
+	}
+	return f.recs, f.err
+}
+
+// newFakeFn returns a constructor compatible with the newXxxClientFn signature
+// that ignores the credential/subscription/region and always returns fake.
+func newFakeFn(fake serviceRecsGetter) func(azcore.TokenCredential, string, string) serviceRecsGetter {
+	return func(_ azcore.TokenCredential, _, _ string) serviceRecsGetter { return fake }
+}
+
+const fakeServiceSleep = 100 * time.Millisecond
+
+// TestGetRecommendations_Parallelism proves that all four service goroutines
+// run concurrently: total wall-clock time must be well under 2× per-service
+// sleep (i.e. less than 200ms) rather than near 4× (400ms sequential).
+// Advisor always calls getAdvisorRecommendations which skips the DI seam, so
+// we only count the four injectable services.
+func TestGetRecommendations_Parallelism(t *testing.T) {
+	origCompute := newComputeClientFn
+	origDatabase := newDatabaseClientFn
+	origCache := newCacheClientFn
+	origCosmos := newCosmosDBClientFn
+	t.Cleanup(func() {
+		newComputeClientFn = origCompute
+		newDatabaseClientFn = origDatabase
+		newCacheClientFn = origCache
+		newCosmosDBClientFn = origCosmos
+	})
+
+	rec := func(svc common.ServiceType) common.Recommendation {
+		return common.Recommendation{Provider: common.ProviderAzure, Service: svc}
+	}
+
+	newComputeClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceCompute)}})
+	newDatabaseClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceRelationalDB)}})
+	newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceCache)}})
+	newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{rec(common.ServiceNoSQL)}})
+
+	adapter := &RecommendationsClientAdapter{
+		cred:           &mockAzureTokenCredential{},
+		subscriptionID: "sub-parallelism",
+	}
+
+	start := time.Now()
+	_, err := adapter.GetRecommendations(context.Background(), common.RecommendationParams{})
+	elapsed := time.Since(start)
+
+	require.NoError(t, err)
+	// With 4 services sleeping 100ms in parallel the wall-clock must be
+	// substantially less than 2× per-service sleep. We allow 190ms headroom
+	// for scheduler jitter; if the calls were serial this would take ~400ms.
+	assert.Less(t, elapsed, 2*fakeServiceSleep,
+		"expected parallel dispatch: elapsed %v >= 2× per-service sleep %v — services may be running serially",
+		elapsed, fakeServiceSleep)
+}
+
+// TestGetRecommendations_OrderPreservation verifies that the merged slice
+// follows the canonical order compute → database → cache → cosmosdb regardless
+// of which fake goroutine returns first (staggered sleeps force an
+// out-of-start-order completion).
+func TestGetRecommendations_OrderPreservation(t *testing.T) {
+	origCompute := newComputeClientFn
+	origDatabase := newDatabaseClientFn
+	origCache := newCacheClientFn
+	origCosmos := newCosmosDBClientFn
+	t.Cleanup(func() {
+		newComputeClientFn = origCompute
+		newDatabaseClientFn = origDatabase
+		newCacheClientFn = origCache
+		newCosmosDBClientFn = origCosmos
+	})
+
+	makeRec := func(svc common.ServiceType) common.Recommendation {
+		return common.Recommendation{Provider: common.ProviderAzure, Service: svc, ResourceType: string(svc)}
+	}
+
+	// Stagger sleeps so goroutines complete in reverse order: cosmosdb
+	// finishes first (~10ms), compute last (~40ms). The merged slice must
+	// still reflect the canonical order.
+	newComputeClientFn = newFakeFn(&fakeServiceClient{sleepDur: 40 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceCompute)}})
+	newDatabaseClientFn = newFakeFn(&fakeServiceClient{sleepDur: 30 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceRelationalDB)}})
+	newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: 20 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceCache)}})
+	newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: 10 * time.Millisecond, recs: []common.Recommendation{makeRec(common.ServiceNoSQL)}})
+
+	adapter := &RecommendationsClientAdapter{
+		cred:           &mockAzureTokenCredential{},
+		subscriptionID: "sub-order",
+	}
+
+	recs, err := adapter.GetRecommendations(context.Background(), common.RecommendationParams{})
+	require.NoError(t, err)
+
+	// Advisor runs via getAdvisorRecommendations (no DI seam) and returns
+	// nothing with a mock credential, so we expect exactly 4 recs.
+	require.Len(t, recs, 4, "expected one rec per injectable service")
+	assert.Equal(t, common.ServiceCompute, recs[0].Service, "slot 0 must be compute")
+	assert.Equal(t, common.ServiceRelationalDB, recs[1].Service, "slot 1 must be database")
+	assert.Equal(t, common.ServiceCache, recs[2].Service, "slot 2 must be cache")
+	assert.Equal(t, common.ServiceNoSQL, recs[3].Service, "slot 3 must be cosmosdb")
+}
+
+// TestGetRecommendations_ErrorIsolation asserts that a single service error
+// does not prevent the other services' results from appearing in the merged
+// slice (no sibling cancellation).
+func TestGetRecommendations_ErrorIsolation(t *testing.T) {
+	origCompute := newComputeClientFn
+	origDatabase := newDatabaseClientFn
+	origCache := newCacheClientFn
+	origCosmos := newCosmosDBClientFn
+	t.Cleanup(func() {
+		newComputeClientFn = origCompute
+		newDatabaseClientFn = origDatabase
+		newCacheClientFn = origCache
+		newCosmosDBClientFn = origCosmos
+	})
+
+	makeRec := func(svc common.ServiceType) common.Recommendation {
+		return common.Recommendation{Provider: common.ProviderAzure, Service: svc}
+	}
+
+	// database returns an error; the other three must still contribute recs.
+	newComputeClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{makeRec(common.ServiceCompute)}})
+	newDatabaseClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, err: errors.New("db unavailable")})
+	newCacheClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{makeRec(common.ServiceCache)}})
+	newCosmosDBClientFn = newFakeFn(&fakeServiceClient{sleepDur: fakeServiceSleep, recs: []common.Recommendation{makeRec(common.ServiceNoSQL)}})
+
+	adapter := &RecommendationsClientAdapter{
+		cred:           &mockAzureTokenCredential{},
+		subscriptionID: "sub-isolation",
+	}
+
+	recs, err := adapter.GetRecommendations(context.Background(), common.RecommendationParams{})
+	require.NoError(t, err, "a per-service error must not surface as a GetRecommendations error")
+	require.Len(t, recs, 3, "expected recs from the 3 healthy injectable services")
+
+	services := make([]common.ServiceType, len(recs))
+	for i, r := range recs {
+		services[i] = r.Service
+	}
+	assert.Contains(t, services, common.ServiceCompute, "compute recs must be present despite db error")
+	assert.Contains(t, services, common.ServiceCache, "cache recs must be present despite db error")
+	assert.Contains(t, services, common.ServiceNoSQL, "cosmosdb recs must be present despite db error")
+	assert.NotContains(t, services, common.ServiceRelationalDB, "db recs must be absent when db errors")
+}