Skip to content

[test] Fix flaky gateway e2e tests - #2500

Merged
varungup90 merged 1 commit into
vllm-project:mainfrom
Jeffwan:jiaxin/fix-upstream-e2e-flaky-test
Jul 26, 2026
Merged

[test] Fix flaky gateway e2e tests#2500
varungup90 merged 1 commit into
vllm-project:mainfrom
Jeffwan:jiaxin/fix-upstream-e2e-flaky-test

Conversation

@Jeffwan

@Jeffwan Jeffwan commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Pull Request Description

flaky test fix

  • TestModelRPSLimit: The Redis limiter uses fixed one-second windows. The test sent requests sequentially, and the first mock response could take long enough for the second request to enter the next window and succeed. The fix sends two short requests concurrently and verifies that one succeeds while the other receives HTTP 429.
  • TestPrefixCacheRouting: A novel prefix has no cache affinity and falls back to least-request routing, which randomly breaks ties between equally loaded pods. Therefore, expecting it to select a different pod was probabilistic and flaky. The fix keeps only the deterministic assertion that an identical prompt reuses the warmed pod.

Related Issues

Resolves: #2498

Important: Before submitting, please complete the description above and review the checklist below.


Contribution Guidelines (Expand for Details)

We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:

Pull Request Title Format

Your PR title should start with one of these prefixes to indicate the nature of the change:

  • [Bug]: Corrections to existing functionality
  • [CI]: Changes to build process or CI pipeline
  • [Docs]: Updates or additions to documentation
  • [API]: Modifications to aibrix's API or interface
  • [CLI]: Changes or additions to the Command Line Interface
  • [Misc]: For changes not covered above (use sparingly)

Note: For changes spanning multiple categories, use multiple prefixes in order of importance.

Submission Checklist

  • PR title includes appropriate prefix(es)
  • Changes are clearly explained in the PR description
  • New and existing tests pass successfully
  • Code adheres to project style and best practices
  • Documentation updated to reflect changes (if applicable)
  • Thorough testing completed, no regressions introduced

By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.

Signed-off-by: Jiaxin Shan <seedjeffwan@gmail.com>
Copilot AI review requested due to automatic review settings July 26, 2026 06:32

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors end-to-end tests to improve reliability. In model_rps_limit_test.go, sequential requests are replaced with concurrent requests to reliably test the 1 RPS limit within the same window, and a helper function is introduced to assert that exactly one request is allowed and one is rejected. In routing_strategy_test.go, the flaky probabilistic test for novel prefix routing is removed. The reviewer suggests reusing a single OpenAI client instance across concurrent requests in sendConcurrentRequests to minimize TCP/TLS handshake overhead and timing jitter, ensuring requests are dispatched as simultaneously as possible.

Comment thread test/e2e/model_rps_limit_test.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR targets flakiness in the gateway E2E test suite by making routing and rate-limit assertions deterministic under timing variance and load-balancing tie-breaking.

Changes:

  • Simplifies TestPrefixCacheRouting to only assert deterministic warm-pod reuse (removing probabilistic “novel prefix routes elsewhere” logic).
  • Updates TestModelRPSLimit to issue two short requests concurrently to reliably land in the same fixed Redis 1s window and assert one 429.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/e2e/routing_strategy_test.go Removes probabilistic assertions from prefix-cache routing E2E test to avoid tie-break flakiness.
test/e2e/model_rps_limit_test.go Sends concurrent requests and checks exactly one allowed/one throttled to avoid timing-dependent RPS window rollover.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/e2e/model_rps_limit_test.go
Copilot AI review requested due to automatic review settings July 26, 2026 06:46
@Jeffwan
Jeffwan force-pushed the jiaxin/fix-upstream-e2e-flaky-test branch from 586bc89 to d018cfb Compare July 26, 2026 06:46
@Jeffwan
Jeffwan force-pushed the jiaxin/fix-upstream-e2e-flaky-test branch from d018cfb to 0852864 Compare July 26, 2026 06:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines 141 to 143
targetPod2 := getTargetPodFromChatCompletion(t, req, "prefix-cache")
t.Logf("req: %s, target pod: %v\n", req, targetPod2)
assert.Equal(t, targetPod, targetPod2)
Copilot AI review requested due to automatic review settings July 26, 2026 06:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@varungup90
varungup90 merged commit c10bd72 into vllm-project:main Jul 26, 2026
27 of 28 checks passed
@Jeffwan
Jeffwan deleted the jiaxin/fix-upstream-e2e-flaky-test branch July 27, 2026 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Flaky E2E tests: TestModelRPSLimit/TestPrefixCacheRouting

3 participants