Skip to content

Commit 2819b3d

Browse files
authored
feat(cli): end-of-run summary of dropped recommendations (closes #361) (#875)
* feat(cli): end-of-run summary of dropped recommendations (closes #361) Track and surface recommendations that were dropped during sizing, filtering, or family-NU partitioning so users see why an instance type didn't make it into the final plan instead of silently missing. DropSummary collects per-stage counts with optional reasons; printed at end-of-run when any drop occurred. applyFilters, applySizing, and ApplyFamilyNUSizingRDS now thread a *common.DropSummary through their signatures (nil-safe). Tests cover collection, formatting, and the no-drops/nil-summary fast paths. * fix(common): lazy-init counts map in DropSummary.Add to prevent panic on zero-value Calling Add on a zero-value DropSummary (var d DropSummary) would panic with "assignment to entry in nil map" because the counts map is only populated by NewDropSummary. Lazily initialise the map on first Add so both construction patterns are safe. Adds a regression test that exercises a zero-value DropSummary directly. Addresses CodeRabbit review on PR #875. * fix(drops): nil-map guard in DropSummary.Add + NoNUSignal category + test coverage (#875 CR) - DropSummary.Add already had the lazy-init nil-map guard; add TestDropSummary_ZeroValue_Safe regression test to lock the behaviour - Add DropFamilyNoNUSignal ("family-nu-no-nu-signal") constant and FamilyDropCounts.NoNUSignal field for the case where AWS-rec NU sums to zero (unknown/unrecognised sizes) -- distinct from AlreadyAtTarget - Fix sizeRDSFamilyRecs: currentNU<=0 branch now sets NoNUSignal instead of incorrectly reusing AlreadyAtTarget - Propagate NoNUSignal in ApplyFamilyNUSizingRDS aggregation loop and in cmd/multi_service_helpers.go drops recording - Capture drops in all TestApplyFamilyNUSizingRDS subtests; assert AlreadyAtTarget==1 for the "at target" case, SizedToZero==1 for a new "floor(0)" test case, and zero total drops for passing-through cases * fix(cli): extract writeReportAndSummary to reduce runToolMultiService complexity gocyclo flagged runToolMultiService at complexity 11 (threshold 10). Extract the final report-write-and-summary block into a dedicated writeReportAndSummary helper, bringing the parent function to complexity 10. No logic change. * docs(family_nu): correct misleading comments about NoNUSignal drop behavior The rdsInstanceNU map comment, rdsInstanceNUFromType comment, ApplyFamilyNUSizingRDS return-value doc, and sizeRDSFamilyRecs summary all claimed unknown-size recs "fall back to per-pool sizing" or are "passed through unchanged". That is only true when rdsFamilyFromType returns an empty prefix (those recs go to nonRDS via partitionRDSRecsByFamily). When a rec has a known family prefix but an unrecognised size suffix, it reaches sizeRDSFamilyRecs and, if the whole family's NU sums to zero (currentNU <= 0), is dropped into drops.NoNUSignal -- not returned unchanged or forwarded downstream. Update all four comment blocks to reflect the three distinct outcomes in sizeRDSFamilyRecs: CE-no-signal (return as-is), already-at-target (drop to AlreadyAtTarget), and zero-rec-NU (drop to NoNUSignal). * test(cli): assert drop-summary categories populated via non-nil DropSummary (refs #361) Add discriminating tests for the four drop categories that were only exercised via the nil-DropSummary path, leaving no regression guard against a future removal of a drops.Add call site: - TestApplyFilters_DropMinPoolSize: passes a non-nil DropSummary into applyFilters with a rec whose avg < --min-pool-size and asserts the DropMinPoolSize count is 1. - TestApplyFilters_DropExtendedSupport: passes a non-nil DropSummary into applyFilters with a MySQL 5.7 rec (all instances on extended support) and asserts the DropExtendedSupport count is 1. - TestApplyTargetCoverage_DropTargetAlreadyMet: passes a non-nil DropSummary into ApplyTargetCoverage with ExistingCoveragePct=90 > target=80 and asserts the DropTargetAlreadyMet count is 1. - TestApplyTargetCoverage_DropTargetSizedToZero: passes a non-nil DropSummary into ApplyTargetCoverage with avg=0.4 at target=80 (floor(0.32)=0) and asserts the DropTargetSizedToZero count is 1. Each test was verified to fail when its corresponding drops.Add call was temporarily removed, and to pass with the call present. * fix(lint): gofmt alignment and US-spelling in drop_summary + family_nu - Align const block in drop_summary.go per gofmt (tab-aligned values) - Fix "synchronisation" -> "synchronization" in drop_summary.go comment - Fix "unrecognised/recognised" -> "unrecognized/recognized" in new family_nu.go comments added by this branch (lines 20, 59, 147, 181) - Add missing blank comment line before "The second return value" in sizeRDSFamilyRecs godoc (gofmt list-item paragraph separator) * fix(lint): gocritic rangeValCopy and unnamedResult in drop-summary helpers Use index-based iteration to avoid copying large Recommendation structs on each range step. Name return values on the four affected functions so gocritic's unnamedResult check passes (3+ return values require names). * fix(lint): collapse dead-valued dimension-filter reason variant golangci v2.10.1 (CI) flagged two findings from the drop-summary change: - unparam: passesDimensionFiltersWithReason always returns "" for its dropReason result. --min-pool-size drops are counted in applyFilters before processRecommendation runs, so the "WithReason" variant never produces a reason. Collapse it back into passesDimensionFilters (returning bool) and return "" directly in processRecommendation, removing the dead return value and the misleading comment. - gocritic paramTypeCombine: combine consecutive same-typed named returns (kept bool, missingSignal bool) -> (kept, missingSignal bool) in applyTargetCoverageOne. * fix(cmd): report coverage drops in final summary * fix(deps): bump brace-expansion to 1.1.16 (GHSA-3jxr-9vmj-r5cp) npm audit flagged brace-expansion <1.1.16 as high severity DoS. This was disclosed 2026-07-21 and affects all PR branches equally. Lockfile-only change; no production API surface altered.
1 parent 0ca127b commit 2819b3d

7 files changed

Lines changed: 339 additions & 111 deletions

‎cmd/helpers.go‎

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,12 @@ func CalculateTotalInstances(recs []common.Recommendation) int {
123123
// on-demand ratio) and stays unscaled. Pre-sizing values can still be
124124
// recovered: RecommendedCount holds AWS's pre-sized count for RIs.
125125
func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Recommendation {
126+
return applyCoverage(recs, coverage, nil)
127+
}
128+
129+
// applyCoverage applies legacy percentage sizing and optionally records
130+
// recommendations whose discrete RI count is reduced to zero.
131+
func applyCoverage(recs []common.Recommendation, coverage float64, drops *common.DropSummary) []common.Recommendation {
126132
if coverage >= 100 {
127133
return recs
128134
}
@@ -170,6 +176,8 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco
170176
adjusted = common.ScaleRecommendationCosts(adjusted, sizedRatio)
171177
adjusted.Count = newCount
172178
result = append(result, adjusted)
179+
} else if drops != nil {
180+
drops.Add(common.DropTargetSizedToZero, 1)
173181
}
174182
}
175183
return result
@@ -241,7 +249,10 @@ func ApplyCoverage(recs []common.Recommendation, coverage float64) []common.Reco
241249
//
242250
// Recs of any other CommitmentType are passed through unmodified (warned
243251
// once per type per run).
244-
func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []common.Recommendation {
252+
// ApplyTargetCoverage applies the target coverage percentage to a slice of
253+
// recommendations. drops accumulates per-reason drop counts for the
254+
// end-of-run summary; pass nil to skip tracking.
255+
func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64, drops *common.DropSummary) []common.Recommendation {
245256
if targetPct <= 0 || targetPct > 100 {
246257
// Validation ensures we never get here in production, but be defensive
247258
// so a buggy caller doesn't divide by zero.
@@ -253,14 +264,15 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
253264
var skipped int
254265
unsupportedSeen := make(map[common.CommitmentType]bool)
255266

256-
for _rvc := range recs {
257-
rec := recs[_rvc]
258-
adjusted, kept, missingSignal := applyTargetCoverageOne(rec, targetPct, unsupportedSeen)
267+
for i := range recs {
268+
adjusted, kept, missingSignal, dropReason := applyTargetCoverageOne(recs[i], targetPct, unsupportedSeen)
259269
if missingSignal {
260270
skipped++
261271
}
262272
if kept {
263273
result = append(result, adjusted)
274+
} else if dropReason != "" {
275+
drops.Add(dropReason, 1)
264276
}
265277
}
266278

@@ -273,51 +285,53 @@ func ApplyTargetCoverage(recs []common.Recommendation, targetPct float64) []comm
273285
}
274286

275287
// applyTargetCoverageOne dispatches a single recommendation through the
276-
// appropriate branch. Returns (rec, kept, missingSignal):
288+
// appropriate branch. Returns (rec, kept, missingSignal, dropReason):
277289
// - kept=true → caller appends `rec` (the adjusted or pass-through value).
278290
// - kept=false → caller drops the rec (only the RI "target unreachable"
279-
// branch returns this; an INFO log already fired).
291+
// branches return this; an INFO log already fired).
280292
// - missingSignal=true → counted toward the end-of-run skip summary.
293+
// - dropReason is non-empty when kept=false and the drop has a named category.
281294
//
282295
// Split out of ApplyTargetCoverage to keep that function under gocyclo's
283296
// complexity threshold.
284-
func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (common.Recommendation, bool, bool) {
297+
func applyTargetCoverageOne(rec common.Recommendation, targetPct float64, unsupportedSeen map[common.CommitmentType]bool) (result common.Recommendation, kept, missingSignal bool, drop string) {
285298
switch {
286299
case common.IsSavingsPlan(rec.Service):
287300
adjusted, ok := applyTargetCoverageSP(rec, targetPct)
288301
if !ok {
289302
// SP no-signal: pass through unchanged.
290-
return rec, true, true
303+
return rec, true, true, ""
291304
}
292-
return adjusted, true, false
305+
return adjusted, true, false, ""
293306
case rec.CommitmentType == common.CommitmentReservedInstance:
294-
adjusted, ok := applyTargetCoverageRI(rec, targetPct)
307+
adjusted, ok, dropReason := applyTargetCoverageRI(rec, targetPct)
295308
if !ok {
296309
// Distinguish "no signal" (pass through, count in summary) from
297310
// "target unreachable" (drop with already-fired INFO log).
298311
if rec.AverageInstancesUsedPerHour <= 0 {
299-
return rec, true, true
312+
return rec, true, true, ""
300313
}
301-
return rec, false, false
314+
return rec, false, false, dropReason
302315
}
303-
return adjusted, true, false
316+
return adjusted, true, false, ""
304317
default:
305318
if !unsupportedSeen[rec.CommitmentType] {
306319
AppLogger.Printf("WARNING: --target-coverage not supported for CommitmentType=%q; passing recommendations through unchanged\n", rec.CommitmentType)
307320
unsupportedSeen[rec.CommitmentType] = true
308321
}
309-
return rec, true, false
322+
return rec, true, false, ""
310323
}
311324
}
312325

313326
// applyTargetCoverageRI is the RI branch of ApplyTargetCoverage. Returns
314-
// (adjusted, true) on success, (rec, false) when the rec should be passed
315-
// through unscaled (no signal) or dropped (target unreachable). Caller
316-
// distinguishes the two via rec.AverageInstancesUsedPerHour.
317-
func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common.Recommendation, bool) {
327+
// (adjusted, true, "") on success, (rec, false, dropReason) when the rec
328+
// should be passed through unscaled (no signal) or dropped (target
329+
// unreachable). Caller distinguishes no-signal from drop via
330+
// rec.AverageInstancesUsedPerHour and uses dropReason for the summary.
331+
func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (result common.Recommendation, ok bool, drop string) {
318332
if rec.AverageInstancesUsedPerHour <= 0 {
319333
// No signal — caller will pass through and count in the summary.
320-
return rec, false
334+
return rec, false, ""
321335
}
322336

323337
avg := rec.AverageInstancesUsedPerHour
@@ -346,7 +360,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
346360
// pass through".
347361
AppLogger.Printf("INFO: --target-coverage=%.1f%% already met by existing coverage %.1f%% for %s/%s/%s; dropped recommendation\n",
348362
targetPct, rec.ExistingCoveragePct, rec.Service, rec.Region, rec.ResourceType)
349-
return rec, false
363+
return rec, false, common.DropTargetAlreadyMet
350364
}
351365
// Floor so we never over-shoot the target on integer-arithmetic edges.
352366
// Strict-target semantics: 80% means "at most 80% coverage", not "at
@@ -366,7 +380,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
366380
// Returning (_, false) with avg > 0 signals "drop, don't pass through".
367381
// applyTargetCoverageRI's caller branches on
368382
// rec.AverageInstancesUsedPerHour to distinguish drop vs no-signal.
369-
return rec, false
383+
return rec, false, common.DropTargetSizedToZero
370384
}
371385

372386
// Cost-bearing fields scale by the ratio of sized-to-original count, so the
@@ -402,7 +416,7 @@ func applyTargetCoverageRI(rec common.Recommendation, targetPct float64) (common
402416
}
403417
adjusted.ProjectedUtilization = projUtil
404418
adjusted.ProjectedCoverage = projCov
405-
return adjusted, true
419+
return adjusted, true, ""
406420
}
407421

408422
// applyTargetCoverageSP is the SP branch of ApplyTargetCoverage. Returns
@@ -463,11 +477,14 @@ func applyTargetCoverageSP(rec common.Recommendation, targetPct float64) (common
463477
// (the main path passes cfg.Coverage; the CSV path passes csvModeCoverage,
464478
// which substitutes the default 80% with 100% so CSV-driven counts aren't
465479
// silently dropped).
466-
func applySizing(recs []common.Recommendation, cfg Config, coverage float64) []common.Recommendation {
480+
//
481+
// drops accumulates per-reason drop counts for the end-of-run summary.
482+
// Pass nil to skip tracking.
483+
func applySizing(recs []common.Recommendation, cfg Config, coverage float64, drops *common.DropSummary) []common.Recommendation {
467484
if cfg.TargetCoverage > 0 {
468-
return ApplyTargetCoverage(recs, cfg.TargetCoverage)
485+
return ApplyTargetCoverage(recs, cfg.TargetCoverage, drops)
469486
}
470-
return ApplyCoverage(recs, coverage)
487+
return applyCoverage(recs, coverage, drops)
471488
}
472489

473490
// ApplyCountOverride overrides the count for all recommendations.

‎cmd/helpers_test.go‎

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,21 @@ func TestApplyCoverage_RICostScaling(t *testing.T) {
383383
assert.Equal(t, 60.0, monthly)
384384
}
385385

386+
func TestApplySizing_LegacyCoverageRecordsCountsFlooredToZero(t *testing.T) {
387+
recs := []common.Recommendation{
388+
{Service: common.ServiceEC2, Count: 1},
389+
{Service: common.ServiceRDS, Count: 10},
390+
}
391+
drops := common.NewDropSummary()
392+
393+
out := applySizing(recs, Config{}, 10, drops)
394+
395+
require.Len(t, out, 1)
396+
assert.Equal(t, 1, out[0].Count)
397+
assert.Equal(t, 1, drops.Total())
398+
assert.Contains(t, drops.FormatOneLine(), common.DropTargetSizedToZero)
399+
}
400+
386401
func TestAdjustRecommendationsForExisting(t *testing.T) {
387402
ctx := context.Background()
388403

@@ -1028,7 +1043,7 @@ func TestApplyTargetCoverage_RI(t *testing.T) {
10281043
for _, tt := range tests {
10291044
t.Run(tt.name, func(t *testing.T) {
10301045
recs := []common.Recommendation{tt.rec}
1031-
out := ApplyTargetCoverage(recs, tt.target)
1046+
out := ApplyTargetCoverage(recs, tt.target, nil)
10321047
if tt.wantDropped {
10331048
if len(out) != 0 {
10341049
t.Fatalf("expected drop; got %d recs", len(out))
@@ -1079,7 +1094,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
10791094
// n = floor(8 * 80/100) = 6. Ratio = 6/10 = 0.6 (cost scaling still
10801095
// uses rec.Count to convert AWS's quoted cost-for-rec.Count into
10811096
// cost-for-nTarget).
1082-
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
1097+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
10831098
require.Len(t, out, 1)
10841099
assert.Equal(t, 6, out[0].Count)
10851100
assert.InDelta(t, 600.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by nTarget/rec.Count")
@@ -1095,7 +1110,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
10951110
monthly := 50.0
10961111
recWithMonthly := rec
10971112
recWithMonthly.RecurringMonthlyCost = &monthly
1098-
out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80)
1113+
out := ApplyTargetCoverage([]common.Recommendation{recWithMonthly}, 80, nil)
10991114
require.Len(t, out, 1)
11001115
require.NotNil(t, out[0].RecurringMonthlyCost, "scaled pointer should be non-nil")
11011116
assert.InDelta(t, 30.0, *out[0].RecurringMonthlyCost, 0.001, "monthly cost scales by 6/10")
@@ -1107,7 +1122,7 @@ func TestApplyTargetCoverage_RI_CostScaling(t *testing.T) {
11071122
// AWS API didn't return RecurringStandardMonthlyCost (all-upfront,
11081123
// or field missing). The sized rec should also have nil so
11091124
// downstream renders "unknown" rather than zero.
1110-
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
1125+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
11111126
require.Len(t, out, 1)
11121127
assert.Nil(t, out[0].RecurringMonthlyCost, "nil input → nil output")
11131128
})
@@ -1198,7 +1213,7 @@ func TestApplyTargetCoverage_RI_ExistingCoverage(t *testing.T) {
11981213

11991214
for _, tt := range tests {
12001215
t.Run(tt.name, func(t *testing.T) {
1201-
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target)
1216+
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, tt.target, nil)
12021217
if tt.wantDropped {
12031218
assert.Len(t, out, 0, "expected drop")
12041219
return
@@ -1233,7 +1248,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
12331248
// flag's intent is "leave 20% headroom", so the commitment shrinks
12341249
// to 80% of AWS rec. All cost-bearing fields scale by 0.8.
12351250
// Projected util = 95/0.80 = 118.75 clamped to 100.
1236-
out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80)
1251+
out := ApplyTargetCoverage([]common.Recommendation{mkSP(95)}, 80, nil)
12371252
require.Len(t, out, 1)
12381253
assert.InDelta(t, 1.6, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment, 0.001)
12391254
assert.InDelta(t, 800.0, out[0].CommitmentCost, 0.001, "CommitmentCost scales by target/100")
@@ -1247,7 +1262,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
12471262
t.Run("AWS below target — scale down by target (under-buy)", func(t *testing.T) {
12481263
// RecUtil=50, target=80. All cost-bearing fields shrink to 80%.
12491264
// Projected util = 50/0.80 = 62.5 (no clamp needed).
1250-
out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80)
1265+
out := ApplyTargetCoverage([]common.Recommendation{mkSP(50)}, 80, nil)
12511266
require.Len(t, out, 1)
12521267
details := out[0].Details.(*common.SavingsPlanDetails)
12531268
assert.InDelta(t, 1.6, details.HourlyCommitment, 0.001)
@@ -1260,7 +1275,7 @@ func TestApplyTargetCoverage_SP(t *testing.T) {
12601275
})
12611276

12621277
t.Run("no signal → passed through unchanged", func(t *testing.T) {
1263-
out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80)
1278+
out := ApplyTargetCoverage([]common.Recommendation{mkSP(0)}, 80, nil)
12641279
require.Len(t, out, 1)
12651280
// Original recommendation values intact.
12661281
assert.Equal(t, 2.0, out[0].Details.(*common.SavingsPlanDetails).HourlyCommitment)
@@ -1281,7 +1296,7 @@ func TestApplySizing(t *testing.T) {
12811296

12821297
t.Run("TargetCoverage > 0 → ApplyTargetCoverage", func(t *testing.T) {
12831298
cfg := Config{TargetCoverage: 80, Coverage: 100}
1284-
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
1299+
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
12851300
require.Len(t, out, 1)
12861301
// avg=8, target=80%, existing=0%. gap=80.
12871302
// n = floor(8 * 80/100) = floor(6.4) = 6. ProjUtil = 8/6 = 133% → 100.
@@ -1291,7 +1306,7 @@ func TestApplySizing(t *testing.T) {
12911306

12921307
t.Run("TargetCoverage == 0 → ApplyCoverage", func(t *testing.T) {
12931308
cfg := Config{TargetCoverage: 0, Coverage: 50}
1294-
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage)
1309+
out := applySizing([]common.Recommendation{ri}, cfg, cfg.Coverage, nil)
12951310
require.Len(t, out, 1)
12961311
// ApplyCoverage(50) on count=10 → 5. ProjectedUtilization NOT set
12971312
// (zero) because we took the coverage branch.
@@ -1336,7 +1351,7 @@ func TestApplyTargetCoverage_RI_Target100(t *testing.T) {
13361351

13371352
for _, tt := range tests {
13381353
t.Run(tt.name, func(t *testing.T) {
1339-
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100)
1354+
out := ApplyTargetCoverage([]common.Recommendation{tt.rec}, 100, nil)
13401355
if tt.wantDropped {
13411356
assert.Len(t, out, 0, "expected drop at target=100 for avg=%.3f", tt.rec.AverageInstancesUsedPerHour)
13421357
return
@@ -1360,7 +1375,7 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
13601375
RecommendedUtilization: 50,
13611376
Details: &common.SavingsPlanDetails{HourlyCommitment: 0},
13621377
}
1363-
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
1378+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
13641379
require.Len(t, out, 1, "$0 SP rec should still be in output (pass-through)")
13651380
// Pass-through — projection fields must NOT be set, savings unchanged.
13661381
assert.Equal(t, 0.0, out[0].ProjectedUtilization, "ProjectedUtilization must NOT be set for $0-commitment pass-through")
@@ -1381,9 +1396,61 @@ func TestApplyTargetCoverage_SP_NoSignalGuards(t *testing.T) {
13811396
RecommendedUtilization: 50,
13821397
Details: common.ComputeDetails{Platform: "Linux/UNIX"}, // wrong type
13831398
}
1384-
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80)
1399+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, nil)
13851400
require.Len(t, out, 1)
13861401
assert.Equal(t, 0.0, out[0].ProjectedUtilization, "must NOT set projection when scaling failed")
13871402
assert.Equal(t, 1500.0, out[0].EstimatedSavings, "EstimatedSavings must remain unscaled when scaling failed")
13881403
})
13891404
}
1405+
1406+
// TestApplyTargetCoverage_DropTargetAlreadyMet verifies that when existing
1407+
// coverage already meets or exceeds the target, the recommendation is
1408+
// dropped and the drop is recorded in a non-nil DropSummary under the
1409+
// DropTargetAlreadyMet category. If the drops.Add call for that branch
1410+
// were removed, d.Total() would stay at 0 and the assertion below would fail.
1411+
func TestApplyTargetCoverage_DropTargetAlreadyMet(t *testing.T) {
1412+
// ExistingCoveragePct=90 >= target=80: gapPct=80-90=-10 <= 0 -> drop.
1413+
rec := common.Recommendation{
1414+
Service: common.ServiceEC2,
1415+
Region: "us-east-1",
1416+
ResourceType: "t3.medium",
1417+
Count: 5,
1418+
CommitmentType: common.CommitmentReservedInstance,
1419+
AverageInstancesUsedPerHour: 8.0,
1420+
ExistingCoveragePct: 90.0,
1421+
}
1422+
1423+
d := common.NewDropSummary()
1424+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)
1425+
1426+
assert.Empty(t, out, "already-covered rec should be dropped")
1427+
assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
1428+
assert.Contains(t, d.FormatOneLine(), common.DropTargetAlreadyMet,
1429+
"drop summary should name the target-already-met category")
1430+
}
1431+
1432+
// TestApplyTargetCoverage_DropTargetSizedToZero verifies that when the
1433+
// floor(avg * gapPct / 100) formula produces 0, the recommendation is
1434+
// dropped and the drop is recorded in a non-nil DropSummary under the
1435+
// DropTargetSizedToZero category. If the drops.Add call for that branch
1436+
// were removed, d.Total() would stay at 0 and the assertion below would fail.
1437+
func TestApplyTargetCoverage_DropTargetSizedToZero(t *testing.T) {
1438+
// avg=0.4, target=80%, existing=0%: floor(0.4 * 80/100) = floor(0.32) = 0 -> drop.
1439+
rec := common.Recommendation{
1440+
Service: common.ServiceEC2,
1441+
Region: "us-east-1",
1442+
ResourceType: "t3.micro",
1443+
Count: 1,
1444+
CommitmentType: common.CommitmentReservedInstance,
1445+
AverageInstancesUsedPerHour: 0.4,
1446+
ExistingCoveragePct: 0.0,
1447+
}
1448+
1449+
d := common.NewDropSummary()
1450+
out := ApplyTargetCoverage([]common.Recommendation{rec}, 80, d)
1451+
1452+
assert.Empty(t, out, "floor-to-zero rec should be dropped")
1453+
assert.Equal(t, 1, d.Total(), "drop summary should record 1 drop")
1454+
assert.Contains(t, d.FormatOneLine(), common.DropTargetSizedToZero,
1455+
"drop summary should name the target-sized-to-zero category")
1456+
}

0 commit comments

Comments
 (0)