Skip to content

test: TestCtlGetCampaignAggregatesPredictedAndRealPerTenant is load-sensitive — real_saved_usd reads 0 on a starved CI runner #163

Description

@amiddavid

TestCtlGetCampaignAggregatesPredictedAndRealPerTenant (proxy/campaign_test.go:627, from #150) fails intermittently on CI runners under package-parallel load:

--- FAIL: TestCtlGetCampaignAggregatesPredictedAndRealPerTenant (0.26s)
    campaign_test.go:627: real_saved_usd = 0, want ~0.10 — this campaign's own strategy only,
    not the $5.10 of credit this tenant has in total

Evidence

Where CGO Result
CI purego job, PR #141 0 FAIL
CI purego job, PR #160 0 FAIL
CI purego job, same commit, re-run 0 pass
CI purego job, PR #161 0 pass
CI build-test, all of the above 1 pass
That test alone, clean main, 16-core box 0 and 1 6/6 pass
Whole proxy package, clean main, 16-core box 0 3/3 pass
Whole proxy package, PR #141 branch, 16-core box 0 3/3 pass

Neither #141 nor #160 touches the campaign or control-plane code, and the same commit passed on
re-run, so this is not caused by those changes. It reproduces only on a 2-core CI runner.

Why it started showing up now

#161 adds a purego job that runs go test over five package trees. go test starts up to
GOMAXPROCS package binaries in parallel, so on a 2-core runner the proxy package now competes
with four other trees — a contention level build-test does not produce. That job has been changed
to -p 1 so it stops provoking this, which removes the symptom from CI but not the cause.

What the failure is not

The obvious guess is an unflushed asynchronous write, and it does not fit: the fixture's record
helper (proxy/manager_test.go:81) polls DB().Requests(...) until the row is persisted, up to 5s,
and fails loudly with "the writer did not persist" if it is not. The observed failure is different —
the row persisted, and the aggregate returned 0 for it. So something between the persisted
request row and real_saved_usd in GET /api/keepalive/campaigns/<id> is timing-dependent: the
campaign→strategy association, or a second write this test makes whose visibility is not waited for
the way the request row's is.

I stopped there rather than guess further — this is #150's machinery and its author will see the
answer faster than I will. What I can say with confidence is the shape: the assertion reads an
aggregate whose inputs are not all waited for, and it only loses the race when starved of CPU.

Suggested fix

Wait for the aggregate's inputs, not just the request row — e.g. poll
GET /api/keepalive/campaigns/<id> until real_saved_usd is non-zero (with the same 5s deadline
and loud failure), or have record also confirm whatever secondary row the credit aggregation
joins against. Either keeps the assertion meaningful while removing the load sensitivity.

Found while adding the purego CI job in #161. Related: #162, a different intermittently-failing
test with the same underlying cause (a precondition the test hopes for rather than manufactures).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions