CRE-4040: stabilize TestCompute_SpendValueRelativeToComputeTime#22341
Open
CRE-4040: stabilize TestCompute_SpendValueRelativeToComputeTime#22341
Conversation
Replace per-row expectedUpperLimit with an overhead-bounded assertion that caps measured-minus-mocked-delay at a single budget (1500ms). The previous 400ms upper buffer was too tight for wall-clock measurement in compute.go under CI load — observed 438ms overhead broke the 2s case and the 2.5s/3s rows had identical 400ms buffers waiting to flake. Also drop the inner t.Parallel() on the subtests to reduce timer-heap and CPU contention across the 5 wasm-module subtests; outer t.Parallel() is preserved. Verified: 10/10 passes with -race -shuffle=on.
Contributor
|
✅ No conflicts with other open PRs targeting |
Restructure the upper-bound assertion to stay in uint64 space: keep meteringOverheadBudgetMs as uint64 and compare measured value against a precomputed upperBound (mocked delay ms + budget) instead of subtracting across signed/unsigned types. The remaining int64→uint64 cast on test.time.Milliseconds() is annotated with //nolint:gosec since the duration is a positive constant. Re-verified: 10/10 passes with -race -shuffle=on.
|
product-security-plaid-production
Bot
requested review from
DylanTinianov and
fernandezlautaro
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes flake in
TestCompute_SpendValueRelativeToComputeTime(subtest/2sreported by Trunk).expectedUpperLimitwith an overhead-bounded assertion:(measured - mocked_delay_ms) < meteringOverheadBudgetMs (1500ms). Decouples tolerance from case duration and tests a single invariant — metering overhead is bounded.t.Parallel()on the 5 subtests to reduce timer-heap and CPU contention. Outert.Parallel()is preserved.compute.go:executeWithModule; flagged for follow-up but out of scope here).Why the previous bound was flaky
Each table row had a fixed 400ms upper buffer over its mocked delay. Production
executeWithModulemeasures elapsed time with rawtime.Now()/time.Since()(no mockable clock), so any CI scheduling jitter or wasm-module setup overhead falls into the assertion window. Trunk recorded a failing run with 438ms of overhead on the 2s case (measured 2438 > expected 2400). The 2.5s and 3s rows have identical 400ms buffers and were waiting to flake the same way.Flaky test fixes
TestCompute_SpendValueRelativeToComputeTime/2sTest plan
go test -race -shuffle=on -run '^TestCompute_SpendValueRelativeToComputeTime$' ./core/capabilities/compute/...— 10/10 consecutive runs PASS locally🤖 Generated with Claude Code