Skip to content

[serve] Improve test_root_path in test_fastapi.py#60270

Merged
abrarsheikh merged 5 commits intoray-project:masterfrom
axreldable:root_path_test
Jan 26, 2026
Merged

[serve] Improve test_root_path in test_fastapi.py#60270
abrarsheikh merged 5 commits intoray-project:masterfrom
axreldable:root_path_test

Conversation

@axreldable
Copy link
Contributor

Description

Update the test_root_path unit test in the test_fastapi.py.

The new implementation is more descriptive in the expected values of the request urls, ASGI scope["root_path"], and scope["path"] for different root_path configurations provided via the app and serve layers.

Related issues

Relates to the PR #57555

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
Copy link
Contributor

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

Choose a reason for hiding this comment

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

Code Review

The pull request significantly improves the test_root_path unit test by introducing more descriptive expected values and a comprehensive set of test URLs. The updated test logic now correctly verifies both root_path and path from the ASGI scope for various app_root_path and serve_root_path configurations, including handling different uvicorn versions. The detailed docstring with the explanation table is a great addition for understanding the test's intent and expected behaviors.

cursor[bot]

This comment was marked as outdated.

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
@ray-gardener ray-gardener bot added serve Ray Serve Related Issue community-contribution Contributed by the community labels Jan 18, 2026
@abrarsheikh abrarsheikh added the go add ONLY when ready to merge, run all tests label Jan 19, 2026
Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

@abrarsheikh abrarsheikh merged commit a829bda into ray-project:master Jan 26, 2026
6 checks passed
jinbum-kim pushed a commit to jinbum-kim/ray that referenced this pull request Jan 29, 2026
## Description

Update the `test_root_path` unit test in the `test_fastapi.py`.

The new implementation is more descriptive in the expected values of the
request urls, ASGI scope["root_path"], and scope["path"] for different
root_path configurations provided via the app and serve layers.

 ## Related issues

Relates to the PR ray-project#57555

---------

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
Signed-off-by: jinbum-kim <jinbum9958@gmail.com>
limarkdcunha pushed a commit to limarkdcunha/ray that referenced this pull request Jan 29, 2026
## Description

Update the `test_root_path` unit test in the `test_fastapi.py`. 

The new implementation is more descriptive in the expected values of the
request urls, ASGI scope["root_path"], and scope["path"] for different
root_path configurations provided via the app and serve layers.

 ## Related issues

Relates to the PR ray-project#57555

---------

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
400Ping pushed a commit to 400Ping/ray that referenced this pull request Feb 1, 2026
## Description

Update the `test_root_path` unit test in the `test_fastapi.py`.

The new implementation is more descriptive in the expected values of the
request urls, ASGI scope["root_path"], and scope["path"] for different
root_path configurations provided via the app and serve layers.

 ## Related issues

Relates to the PR ray-project#57555

---------

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>
Signed-off-by: 400Ping <jiekaichang@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community go add ONLY when ready to merge, run all tests serve Ray Serve Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants