From 7440108bdbea831854712307eba9650888c8645c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 30 May 2026 19:43:10 +0200 Subject: [PATCH 1/2] 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). --- providers/azure/recommendations.go | 28 +++- providers/azure/recommendations_test.go | 162 ++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 4 deletions(-) diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 4bc68b03b..5098256ce 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -22,6 +22,26 @@ import ( "github.com/LeanerCloud/CUDly/providers/azure/services/savingsplans" ) +// 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 @@ -109,7 +129,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // Compute (VM) recommendations — subscription-wide. if includeCompute { goService(&computeErr, func() { - computeClient := compute.NewClient(r.cred, r.subscriptionID, "") + computeClient := newComputeClientFn(r.cred, r.subscriptionID, "") computeRecs, computeErr = computeClient.GetRecommendations(gctx, params) }) } @@ -117,7 +137,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // Database (SQL) recommendations — subscription-wide. if includeDB { goService(&dbErr, func() { - dbClient := database.NewClient(r.cred, r.subscriptionID, "") + dbClient := newDatabaseClientFn(r.cred, r.subscriptionID, "") dbRecs, dbErr = dbClient.GetRecommendations(gctx, params) }) } @@ -125,7 +145,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // Cache (Redis) recommendations — subscription-wide. if includeCache { goService(&cacheErr, func() { - cacheClient := cache.NewClient(r.cred, r.subscriptionID, "") + cacheClient := newCacheClientFn(r.cred, r.subscriptionID, "") cacheRecs, cacheErr = cacheClient.GetRecommendations(gctx, params) }) } @@ -133,7 +153,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // CosmosDB (NoSQL) recommendations — subscription-wide. if includeCosmos { goService(&cosmosErr, func() { - cosmosClient := cosmosdb.NewClient(r.cred, r.subscriptionID, "") + cosmosClient := newCosmosDBClientFn(r.cred, r.subscriptionID, "") cosmosRecs, cosmosErr = cosmosClient.GetRecommendations(gctx, params) }) } diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index 557e31f13..435b33ffa 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" @@ -557,3 +558,164 @@ func TestMergeServiceResults_StubsDoNotMaskTotalFailure(t *testing.T) { assert.Contains(t, err.Error(), "all 4 Azure recommendation services failed") assert.Nil(t, recs) } + +// 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 2x per-service +// sleep (i.e. less than 200ms) rather than near 4x (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 2x 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 >= 2x 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") +} From 01c4583e2da4109a247b1654fe4d3dc0f3e1ddc9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 19 Jun 2026 23:50:24 +0200 Subject: [PATCH 2/2] fix(test/azure): inject savingsplans+advisor to make timing test hermetic TestGetRecommendations_Parallelism was failing (~300-700ms wall clock vs 200ms threshold) because the savingsplans and advisor goroutines used real constructors that made ARM network calls, adding unbounded latency outside the four fakeServiceClient mocks. Add newSavingsPlansClientFn package-level var (parallel to the other four) and a getAdvisorRecsFn field on RecommendationsClientAdapter (defaulting to r.getAdvisorRecommendations, with a nil guard for struct-literal adapters in existing tests). Wire newSavingsPlansClientFn in GetRecommendations to replace the direct savingsplans.NewClient call. In the three concurrency tests (Parallelism, OrderPreservation, ErrorIsolation) inject noopAdvisorFn and a zero-sleep fakeServiceClient for savingsplans so all latency comes exclusively from the four injectable mocks. Tests are now fully hermetic: no real ARM calls, deterministic rec count, pass consistently under -race. 699 azure provider tests pass; 0 lint issues in touched files. --- providers/azure/recommendations.go | 60 ++++++++++++++++++------- providers/azure/recommendations_test.go | 48 +++++++++++++++----- 2 files changed, 83 insertions(+), 25 deletions(-) diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 5098256ce..0a010c853 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -30,16 +30,33 @@ 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. +// newComputeClientFn, newDatabaseClientFn, newCacheClientFn, +// newCosmosDBClientFn, and newSavingsPlansClientFn 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. +// +// getAdvisorRecsFn wraps r.getAdvisorRecommendations; it is a field rather +// than a package-level var so that injection is scoped to the adapter +// instance and avoids shared-state issues when tests run the advisor path +// concurrently. Tests that do not need real ARM calls set it to noopAdvisorFn. 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) } + 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) + } + newSavingsPlansClientFn func(azcore.TokenCredential, string, string) serviceRecsGetter = func(cred azcore.TokenCredential, sub, region string) serviceRecsGetter { + return savingsplans.NewClient(cred, sub, region) + } ) // RecommendationsClientAdapter aggregates Azure reservation recommendations across all services. @@ -51,9 +68,13 @@ var ( // path is NewRecommendationsClientAdapter; direct struct literals bypass the // invariant check and should be confined to tests that deliberately exercise // the unvalidated shape. +// +// getAdvisorRecsFn defaults to r.getAdvisorRecommendations and may be +// overridden per-instance in tests to avoid real ARM network calls. type RecommendationsClientAdapter struct { - cred azcore.TokenCredential - subscriptionID string + cred azcore.TokenCredential + subscriptionID string + getAdvisorRecsFn func(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) } // NewRecommendationsClientAdapter builds a RecommendationsClientAdapter with @@ -64,10 +85,12 @@ func NewRecommendationsClientAdapter(cred azcore.TokenCredential, subscriptionID if subscriptionID == "" { return nil, fmt.Errorf("azure recommendations: subscriptionID is required") } - return &RecommendationsClientAdapter{ + r := &RecommendationsClientAdapter{ cred: cred, subscriptionID: subscriptionID, - }, nil + } + r.getAdvisorRecsFn = r.getAdvisorRecommendations + return r, nil } // GetRecommendations retrieves all Azure reservation recommendations across services. @@ -165,7 +188,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // a scheduler change. if includeSP { goService(&spErr, func() { - spClient := savingsplans.NewClient(r.cred, r.subscriptionID, "") + spClient := newSavingsPlansClientFn(r.cred, r.subscriptionID, "") spRecs, spErr = spClient.GetRecommendations(gctx, params) }) } @@ -173,8 +196,15 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // Azure Advisor adds cross-cutting cost recommendations independent of the // per-service Reservation API. Failures here are non-fatal — the per-service // results above are still useful on their own. + // + // getAdvisorRecsFn defaults to r.getAdvisorRecommendations and may be + // replaced per-adapter in tests to avoid real ARM network calls. + advisorFn := r.getAdvisorRecsFn + if advisorFn == nil { + advisorFn = r.getAdvisorRecommendations + } goService(&advisorErr, func() { - advisorRecs, advisorErr = r.getAdvisorRecommendations(gctx, params) + advisorRecs, advisorErr = advisorFn(gctx, params) }) // Wait for all goroutines. g.Wait() always returns nil because every diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index 435b33ffa..8a13718d8 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -585,37 +585,51 @@ func newFakeFn(fake serviceRecsGetter) func(azcore.TokenCredential, string, stri return func(_ azcore.TokenCredential, _, _ string) serviceRecsGetter { return fake } } +// noopAdvisorFn is a getAdvisorRecsFn replacement that returns immediately +// with zero results, used in timing/isolation tests to keep all latency inside +// the injectable fakeServiceClient mocks. +func noopAdvisorFn(_ context.Context, _ common.RecommendationParams) ([]common.Recommendation, error) { + return nil, nil +} + const fakeServiceSleep = 100 * time.Millisecond // TestGetRecommendations_Parallelism proves that all four service goroutines // run concurrently: total wall-clock time must be well under 2x per-service // sleep (i.e. less than 200ms) rather than near 4x (400ms sequential). -// Advisor always calls getAdvisorRecommendations which skips the DI seam, so -// we only count the four injectable services. +// +// savingsplans and advisor are injected as instant no-ops so that all latency +// comes from the four fakeServiceClient mocks; without this, both paths make +// real ARM network calls whose RTT dwarfs the 100ms threshold. func TestGetRecommendations_Parallelism(t *testing.T) { origCompute := newComputeClientFn origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute newDatabaseClientFn = origDatabase newCacheClientFn = origCache newCosmosDBClientFn = origCosmos + newSavingsPlansClientFn = origSP }) rec := func(svc common.ServiceType) common.Recommendation { return common.Recommendation{Provider: common.ProviderAzure, Service: svc} } + noopFake := newFakeFn(&fakeServiceClient{}) 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)}}) + newSavingsPlansClientFn = noopFake adapter := &RecommendationsClientAdapter{ - cred: &mockAzureTokenCredential{}, - subscriptionID: "sub-parallelism", + cred: &mockAzureTokenCredential{}, + subscriptionID: "sub-parallelism", + getAdvisorRecsFn: noopAdvisorFn, } start := time.Now() @@ -635,16 +649,21 @@ func TestGetRecommendations_Parallelism(t *testing.T) { // follows the canonical order compute -> database -> cache -> cosmosdb regardless // of which fake goroutine returns first (staggered sleeps force an // out-of-start-order completion). +// +// savingsplans and advisor are injected as instant no-ops so the test is +// fully hermetic: no real ARM calls, deterministic rec count. func TestGetRecommendations_OrderPreservation(t *testing.T) { origCompute := newComputeClientFn origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute newDatabaseClientFn = origDatabase newCacheClientFn = origCache newCosmosDBClientFn = origCosmos + newSavingsPlansClientFn = origSP }) makeRec := func(svc common.ServiceType) common.Recommendation { @@ -658,17 +677,19 @@ func TestGetRecommendations_OrderPreservation(t *testing.T) { 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)}}) + newSavingsPlansClientFn = newFakeFn(&fakeServiceClient{}) adapter := &RecommendationsClientAdapter{ - cred: &mockAzureTokenCredential{}, - subscriptionID: "sub-order", + cred: &mockAzureTokenCredential{}, + subscriptionID: "sub-order", + getAdvisorRecsFn: noopAdvisorFn, } 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. + // savingsplans and advisor are no-ops; we expect exactly 4 recs (one per + // injectable service) in canonical order. 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") @@ -679,16 +700,21 @@ func TestGetRecommendations_OrderPreservation(t *testing.T) { // TestGetRecommendations_ErrorIsolation asserts that a single service error // does not prevent the other services' results from appearing in the merged // slice (no sibling cancellation). +// +// savingsplans and advisor are injected as instant no-ops so the rec count is +// fully deterministic regardless of network availability. func TestGetRecommendations_ErrorIsolation(t *testing.T) { origCompute := newComputeClientFn origDatabase := newDatabaseClientFn origCache := newCacheClientFn origCosmos := newCosmosDBClientFn + origSP := newSavingsPlansClientFn t.Cleanup(func() { newComputeClientFn = origCompute newDatabaseClientFn = origDatabase newCacheClientFn = origCache newCosmosDBClientFn = origCosmos + newSavingsPlansClientFn = origSP }) makeRec := func(svc common.ServiceType) common.Recommendation { @@ -700,10 +726,12 @@ func TestGetRecommendations_ErrorIsolation(t *testing.T) { 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)}}) + newSavingsPlansClientFn = newFakeFn(&fakeServiceClient{}) adapter := &RecommendationsClientAdapter{ - cred: &mockAzureTokenCredential{}, - subscriptionID: "sub-isolation", + cred: &mockAzureTokenCredential{}, + subscriptionID: "sub-isolation", + getAdvisorRecsFn: noopAdvisorFn, } recs, err := adapter.GetRecommendations(context.Background(), common.RecommendationParams{})