amadan
console: distinguish bounce vs complaint breach in mail-limit tests
Review found the bounce and complaint thresholds could be silently swapped without any test failing: no test exercised a complaint-only breach, and the existing bounce test's 10% rate exceeded both thresholds, so it could not tell which one fired. Adds a complaint-only scenario (0.5% complaint, 0% bounce) and tightens the bounce scenario to be bounce-only (6% bounce, 0% complaint), each asserting the reason names the right rate and not the other. Also pins BounceThreshold/ComplaintThreshold directly as SES's own published review thresholds, not arbitrary values.
1 file changed,
+53
−3
internal/console/mailcounts_test.go+53 −3
diff --git a/internal/console/mailcounts_test.go b/internal/console/mailcounts_test.go| index c675be91c..d43f8a38a 100644 |
| --- a/internal/console/mailcounts_test.go |
| +++ b/internal/console/mailcounts_test.go |
| @@ -125,22 +125,72 @@ func TestEnforceIgnoresRatesBelowTheSendFloor(t *testing.T) { |
| } |
| } |
| +// Bounces only — Complaints stays at 0, so this cannot be mistaken for a |
| +// complaint breach (its reason must name "bounce rate", never "complaint |
| +// rate"). 6% clears BounceThreshold (5%); Sends=1000 comfortably clears |
| +// MinSendsForRates. |
| func TestEnforcePausesOnBounceRate(t *testing.T) { |
| fx, now := mailFixture(t) |
| // Cap is 50, so raise it first: this test is about the RATE, and a |
| // cap breach would mask it. |
| mail.ApplyGrant(context.Background(), fx.rel, release.OpsAccount, "seapointish", |
| []string{"x.com"}, 100000, "operator", "paul@rslw.com", now) |
| - seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 100, Bounces: 10}) |
| + seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Bounces: 60}) |
| paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| if !paused { |
| - t.Fatal("a 10% bounce rate did not pause") |
| + t.Fatal("a 6% bounce rate did not pause") |
| } |
| if !strings.Contains(reason, "bounce rate") { |
| - t.Fatalf("reason = %q", reason) |
| + t.Fatalf("reason = %q, want it to name the bounce rate", reason) |
| + } |
| + if strings.Contains(reason, "complaint rate") { |
| + t.Fatalf("reason = %q names the complaint rate on a bounce-only breach", reason) |
| + } |
| +} |
| + |
| +// Complaints only — Bounces stays at 0, so this cannot be mistaken for a |
| +// bounce breach (its reason must name "complaint rate", never "bounce |
| +// rate"). 0.5% clears ComplaintThreshold (0.1%) while staying far under |
| +// BounceThreshold (5%), so a threshold swap (BounceThreshold and |
| +// ComplaintThreshold exchanged) makes this scenario fail to pause at all: |
| +// the swapped ComplaintThreshold becomes 5%, which 0.5% no longer clears, |
| +// and BounceRate is 0 so the bounce case never fires either. That is |
| +// exactly the silent-swap failure mode this test exists to catch. |
| +func TestEnforcePausesOnComplaintRate(t *testing.T) { |
| + fx, now := mailFixture(t) |
| + // Cap is 50, so raise it first: this test is about the RATE, and a |
| + // cap breach would mask it. |
| + mail.ApplyGrant(context.Background(), fx.rel, release.OpsAccount, "seapointish", |
| + []string{"x.com"}, 100000, "operator", "paul@rslw.com", now) |
| + seedCounts(t, fx, now.UTC().Format("2006-01-02"), MailCounts{Sends: 1000, Complaints: 5}) |
| + paused, reason, err := fx.srv.enforceMailLimits(context.Background(), release.OpsAccount, "seapointish", now) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !paused { |
| + t.Fatal("a 0.5% complaint rate did not pause") |
| + } |
| + if !strings.Contains(reason, "complaint rate") { |
| + t.Fatalf("reason = %q, want it to name the complaint rate", reason) |
| + } |
| + if strings.Contains(reason, "bounce rate") { |
| + t.Fatalf("reason = %q names the bounce rate on a complaint-only breach", reason) |
| + } |
| +} |
| + |
| +// BounceThreshold and ComplaintThreshold are SES's own published |
| +// account-review thresholds, not values this package chose — pinned here so |
| +// a later edit that swaps or "tidies" them is caught even if the scenario |
| +// numbers above are changed too. |
| +func TestSESThresholdConstants(t *testing.T) { |
| + if BounceThreshold != 0.05 { |
| + t.Fatalf("BounceThreshold = %v, want 0.05 (SES's published bounce-rate review threshold)", BounceThreshold) |
| + } |
| + if ComplaintThreshold != 0.001 { |
| + t.Fatalf("ComplaintThreshold = %v, want 0.001 (SES's published complaint-rate review threshold)", ComplaintThreshold) |
| } |
| } |