test(api): fix flaky GetEvaluations tests caused by CreatedAt - #2792
Conversation
The GetEvaluations tests compared the whole response including CreatedAt, which is generated at request time, so the assertion failed occasionally. Add normalizeUserEvaluationsCreatedAt to assert CreatedAt is roughly the current time, then align the expected value so the equality check stays stable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes flaky GetEvaluations tests by tolerating small CreatedAt timing differences.
Changes:
- Adds a helper to validate and normalize
CreatedAt. - Applies it across affected HTTP and gRPC tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/api/api/api_test.go |
Stabilizes HTTP evaluation assertions. |
pkg/api/api/api_grpc_test.go |
Adds the helper and stabilizes gRPC assertions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // normalizeUserEvaluationsCreatedAt asserts CreatedAt is roughly now, then aligns expected so Equal stays stable. | ||
| func normalizeUserEvaluationsCreatedAt( | ||
| t *testing.T, | ||
| expected, actual *featureproto.UserEvaluations, | ||
| msgAndArgs ...interface{}, | ||
| ) { | ||
| t.Helper() | ||
| if expected == nil || actual == nil { | ||
| return | ||
| } | ||
| assert.InDelta(t, time.Now().Unix(), actual.CreatedAt, 5, msgAndArgs...) | ||
| expected.CreatedAt = actual.CreatedAt | ||
| } |
There was a problem hiding this comment.
The comment describes what the function does (readable from the signature + body). Replace with why — the flaky CreatedAt timing that motivates this helper.
There was a problem hiding this comment.
@t-kikuc
I have updated the description to address the instability (flaky behavior) regarding the timing of CreatedAt.
| func normalizeUserEvaluationsCreatedAt( | ||
| t *testing.T, | ||
| expected, actual *featureproto.UserEvaluations, | ||
| msgAndArgs ...interface{}, |
There was a problem hiding this comment.
msgAndArgs can be dropped — t.Helper() already attributes the failure to the caller line, and the call sites run inside t.Run(p.desc) which names the case in the output.
Explain why the helper exists (the service stamps CreatedAt mid-test, so a plain Equal is flaky) instead of restating what it does, and drop the msgAndArgs parameter: t.Helper() already points at the caller line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes #1023
What this PR does
Removes the flakiness in the
GetEvaluationstests by assertingUserEvaluations.CreatedAtwith a time tolerance instead of an exact equality check.Background / Why this PR is needed
UserEvaluations.CreatedAtis stamped at request time inside the handler, so the hard-coded expectation and the actual value differ whenever the second boundary is crossed between building the expectation and running the request. The tests compared the whole response withassert.Equal, so this occasionally failed.api_test.gocarried aFIXME: This is a flaky test. CreateAt may not be equal ocasionally.comment acknowledging the problem without fixing it; that comment is now removed.Points
normalizeUserEvaluationsCreatedAtasserts the actualCreatedAtis within 5 seconds oftime.Now().Unix(), then copies it into the expected value so the subsequentassert.Equalstill verifies every other field. This keeps the field covered rather than ignoring it.TestGrpcGetEvaluationsValidation,TestGrpcGetEvaluationsZeroFeature(gRPC) andTestGetEvaluationsValidation,TestGetEvaluationsZeroFeature(HTTP).pkg/gateway/api/api_grpc_test.go; that package has since moved topkg/api/api/, and the failing test is the sameTestGrpcGetEvaluationsValidation/successcase fixed here.