From 19dba433cc6db5b63978b2b14dfe5b2c81fcc96d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 30 Sep 2026 03:52:48 +0200 Subject: [PATCH] test(aws): preserve requested-window SP coverage rates Verify sparse and explicit-zero days produce identical hourly rates, including paginated and nil-coverage responses. Clarify that Days does not certify reporting completeness or alter the requested-window divisor. Refs #51 --- providers/aws/recommendations/sp_coverage.go | 21 +++--- .../sp_coverage_window_test.go | 69 +++++++++++++++++++ 2 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 providers/aws/recommendations/sp_coverage_window_test.go diff --git a/providers/aws/recommendations/sp_coverage.go b/providers/aws/recommendations/sp_coverage.go index 33dfcff..d416299 100644 --- a/providers/aws/recommendations/sp_coverage.go +++ b/providers/aws/recommendations/sp_coverage.go @@ -38,22 +38,17 @@ type SPCoverageSummary struct { // (no SP-eligible activity in the window) or Days==0; a fully covered // window (OnDemandCost==0, covered>0) yields &100.0, not nil. CoveragePct *float64 - // CoveredUSDPerHour is the average SP-covered spend per hour over the - // window (total SpendCoveredBySavingsPlans / windowHours). Nil when - // Days==0. + // CoveredUSDPerHour normalizes reported SP-covered spend over the requested + // window, without adjusting for reporting completeness. Nil when Days==0. CoveredUSDPerHour *float64 - // OnDemandUSDPerHour is the average spend billed at on-demand rates per - // hour, i.e. the SP-eligible spend NOT covered by any Savings Plan. - // This is the uncovered portion (CE's OnDemandCost), not the eligible - // total - see EligibleUSDPerHour for the total. Nil when Days==0. + // OnDemandUSDPerHour normalizes reported uncovered SP-eligible spend over + // the same requested window. Nil when Days==0. OnDemandUSDPerHour *float64 - // EligibleUSDPerHour is the average total SP-eligible spend per hour: - // CoveredUSDPerHour + OnDemandUSDPerHour. Provided so consumers (the - // ladder sizing math) do not have to re-derive the coverage - // denominator. Nil when Days==0. + // EligibleUSDPerHour is CoveredUSDPerHour + OnDemandUSDPerHour, normalized + // over the requested window including zero-activity days. Nil when Days==0. EligibleUSDPerHour *float64 - // Days is the count of daily CE data points that had a non-nil Coverage - // block in the response. Zero means CE returned no data for the window. + // Days counts daily CE data points with a non-nil Coverage block. + // It does not indicate reporting completeness or change the rate divisor. Days int } diff --git a/providers/aws/recommendations/sp_coverage_window_test.go b/providers/aws/recommendations/sp_coverage_window_test.go new file mode 100644 index 0000000..34e9957 --- /dev/null +++ b/providers/aws/recommendations/sp_coverage_window_test.go @@ -0,0 +1,69 @@ +package recommendations + +import ( + "context" + "strconv" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/costexplorer" + "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetSPCoverageSummary_SparseWindowRates(t *testing.T) { + for _, mode := range []string{"omitted", "zero", "nil"} { + for _, paged := range []bool{false, true} { + t.Run(mode+"/paged="+strconv.FormatBool(paged), func(t *testing.T) { + periods := make([]types.SavingsPlansCoverage, 0, 30) + start := time.Now().UTC().AddDate(0, 0, -30) + for day := 0; day < 30; day++ { + if day >= 10 && mode == "omitted" { + continue + } + cost := "12" + if day >= 10 { + cost = "0" + } + period := types.SavingsPlansCoverage{ + TimePeriod: &types.DateInterval{Start: aws.String(start.AddDate(0, 0, day).Format(time.DateOnly)), End: aws.String(start.AddDate(0, 0, day+1).Format(time.DateOnly))}, + Coverage: &types.SavingsPlansCoverageData{SpendCoveredBySavingsPlans: aws.String(cost), OnDemandCost: aws.String(cost)}, + } + if day >= 10 && mode == "nil" { + period.Coverage = nil + } + periods = append(periods, period) + } + pages := []*costexplorer.GetSavingsPlansCoverageOutput{{SavingsPlansCoverages: periods}} + if paged { + pages = []*costexplorer.GetSavingsPlansCoverageOutput{ + {SavingsPlansCoverages: periods[:5], NextToken: aws.String("second")}, + {SavingsPlansCoverages: periods[5:]}, + } + } + mock := &mockSPCE{coveragePages: pages} + got, err := NewClientWithAPI(mock, "us-east-1").GetSPCoverageSummary(context.Background(), "us-east-1", 30) + require.NoError(t, err) + require.NotNil(t, got.CoveredUSDPerHour) + require.NotNil(t, got.OnDemandUSDPerHour) + require.NotNil(t, got.EligibleUSDPerHour) + require.NotNil(t, got.CoveragePct) + assert.InDelta(t, 1.0/6, *got.CoveredUSDPerHour, 1e-9) + assert.InDelta(t, 1.0/6, *got.OnDemandUSDPerHour, 1e-9) + assert.InDelta(t, 1.0/3, *got.EligibleUSDPerHour, 1e-9) + assert.InDelta(t, 50, *got.CoveragePct, 1e-9) + wantDays := 10 + if mode == "zero" { + wantDays = 30 + } + assert.Equal(t, wantDays, got.Days) + assert.Len(t, mock.coverageInputs, len(pages)) + if paged { + assert.Equal(t, "second", aws.ToString(mock.coverageInputs[1].NextToken)) + } + }) + } + } +}