-
Notifications
You must be signed in to change notification settings - Fork 744
test: make frontend hermetic tests xdist-safe with dynamic ports #4992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add shared per-test port allocation for frontend/serve tests\n- Add gRPC metrics port flag and plumb http_metrics_port to avoid 8788 collisions\n- Update hermetic HTTP/gRPC frontend tests for dynamic ports + readiness\n- Write per-test logs under /tmp to avoid polluting the repo root\n- CI wall time improvement (measured): pre_merge gRPC pair 87.48s -> 25.27s (3.46x); post_merge completion mocker 97.29s -> 30.29s (3.21x) Signed-off-by: Keiven Chang <keivenchang@users.noreply.github.com>
Signed-off-by: Keiven Chang <keivenchang@users.noreply.github.com>
Signed-off-by: Keiven Chang <keivenchang@users.noreply.github.com>
Signed-off-by: Keiven Chang <keivenchang@users.noreply.github.com>
| default=False, | ||
| help="Start KServe gRPC server.", | ||
| ) | ||
| parser.add_argument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a configurable parameter so that parallel running gRPC tests can use different ports without colliding.
| TEST_MODEL = QWEN | ||
|
|
||
|
|
||
| class DynamoFrontendProcess(ManagedProcess): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed; using a common class now.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class DynamoFrontendProcess(ManagedProcess): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed; using a common class.
|
|
||
| @pytest.mark.e2e | ||
| @pytest.mark.pre_merge | ||
| @pytest.mark.gpu_1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't use any GPU, so marking it as gpu_0.
| # - tests/frontend/test_vllm.py | ||
| # - tests/frontend/test_completion_mocker_engine.py | ||
| # - tests/frontend/grpc/test_tensor_parameters.py | ||
| # - tests/frontend/grpc/test_tensor_mocker_engine.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 down, many more tests to go!
Overview:
Make hermetic frontend tests safe to run under pytest-xdist by allocating per-test ports (frontend + worker system ports) and removing the hardcoded gRPC metrics port collision. This also redirects per-test logs to /tmp to avoid polluting the repo root during parallel runs.
Details:
ServicePorts) and use it across frontend + serve tests--grpc-metrics-portflag to the frontend and plumbhttp_metrics_portthrough Python/Rust so gRPC mode can run in parallel without 8788 conflicts-n auto)/tmpto avoid creating lots of untrackedtest_*directories in the repo rootWhere should the reviewer start?
Related Issues: (use Closes / Fixes / Resolves / Relates to)
Relates to OPS-2568
/coderabbit profile chill