test(metrics): wait for observation after image job completion - #285
test(metrics): wait for observation after image job completion#285excniesNIED wants to merge 1 commit into
Conversation
CompleteImageJob commits the completed status before observeGatewayCall publishes the counters, so polling only the job status raced the metrics assertion and failed intermittently under CI load (got 0). Also wait for the overhead series, the last one ObserveGatewayCall writes, so the assertions run after publication.
astaxie
left a comment
astaxie
left a comment
There was a problem hiding this comment.
Reviewed the complete diff at exact head 3678c56ffea200386b7c375474edf3e0f3f778c1. The test fix is focused and technically sound: it waits for the last collector written by ObserveGatewayCall after job completion while preserving immediate failure handling. The focused test passed 10 normal runs and 3 race-enabled runs locally; go vet ./internal/server/, git diff --check, and all GitHub CI checks are green.
Approval is blocked by the PR body not preserving .github/pull_request_template.md, as required by AGENTS.md. It currently omits Related Issue, Changes, Type of Change, Compatibility, Security, and Operations, and Checklist. Please restore every template section, mark this as a test/maintenance change, record the no-runtime-impact compatibility assessment, and complete the checklist (using explicit N/A explanations where appropriate).
|
PR body rewritten to follow |
Summary
Fixes an intermittent flake in
TestMetricsImageJobAttributesAttemptsobserved in CI.CompleteImageJobcommits thecompletedstatus to the store beforeobserveGatewayCallpublishes the request counters. The test polled only the job status and asserted the metrics immediately, so under CI load it could observecompletedwhile the counters were still zero and fail withexpected the image request to be counted once, got 0. The wait loop now also waits for theoverheadseries, the last oneObserveGatewayCallwrites, so the assertions run after publication.Related Issue
N/A (observed in CI on the implicit-TLS SMTP PR #271; split out to keep that PR scoped to notification channels)
Changes
TestMetricsImageJobAttributesAttemptsnow breaks the polling loop only when the job iscompletedand theoverheadmetrics series is publishedType of Change
Verification
go test ./internal/server/ -run TestMetricsImageJobAttributesAttempts -count=10— passesgo test ./internal/server/ -race -run TestMetricsImageJobAttributesAttempts -count=8— passesgo vet ./internal/server/— cleangit diff --check— cleanCompatibility, Security, and Operations
None.
Checklist
.envfiles, databases, backups, or runtime logs are included.start.sh, and deployment documentation where applicable.data/model-catalog.yamlremains tracked and catalog changes were reviewed where applicable.git diff --checkpasses.