Skip to content

[Infra] Add Server Root Test to GitHub Actions#21353

Merged
yuneng-jiang merged 7 commits intomainfrom
litellm_gh_server_root_test
Feb 17, 2026
Merged

[Infra] Add Server Root Test to GitHub Actions#21353
yuneng-jiang merged 7 commits intomainfrom
litellm_gh_server_root_test

Conversation

@yuneng-jiang
Copy link
Copy Markdown
Contributor

Relevant issues

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🚄 Infrastructure

Changes

There has been regressions around the SERVER_ROOT_PATH environment handling. Since our normal CI/CD does not run on non-team member PRs it is difficult to catch issues relating to this. Adding a github actions CI step that always run to help catch regressions

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 17, 2026 4:10am

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 17, 2026

Greptile Summary

Adds a new GitHub Actions workflow (.github/workflows/test_server_root_path.yml) that tests SERVER_ROOT_PATH handling by building the Docker image and validating that health, UI, docs, OpenAPI, and model endpoints work correctly under multiple root path prefixes (/api/v1, /litellm, /proxy) as well as the default no-prefix configuration.

  • Matrix strategy tests 3 different root paths, plus a separate job validates behavior without SERVER_ROOT_PATH set
  • Tests cover health endpoints (including liveness/readiness), UI accessibility, API docs, OpenAPI schema, authenticated model list endpoint, and verifies that endpoints are not accessible without the prefix
  • Runs existing unit tests (test_server_root_path.py) inside the Docker container
  • Builds the full Dockerfile.database image, which is heavy but necessary for end-to-end validation
  • Runs on every PR and push to main, which is appropriate given the stated goal of catching regressions from non-team-member PRs

Confidence Score: 4/5

  • This PR is safe to merge — it only adds a new CI workflow with no changes to application code.
  • The PR adds a single CI workflow file with no application code changes. The workflow is well-structured, tests meaningful scenarios, and uses established Docker/GitHub Actions patterns. The only concerns are minor style issues (unnecessary master branch references) and the CI resource cost of building Docker images on every PR.
  • No files require special attention — this is a CI-only change.

Important Files Changed

Filename Overview
.github/workflows/test_server_root_path.yml New CI workflow that builds a Docker image and tests SERVER_ROOT_PATH functionality across 3 path variants plus a no-root-path baseline. Well-structured with good endpoint coverage. Minor style issues with unnecessary master branch references.

Flowchart

flowchart TD
    A[PR / Push to main] --> B[test-server-root-path job]
    A --> C[test-without-root-path job]
    
    B --> D[Build Docker Image]
    D --> E{Matrix: 3 root paths}
    E --> F["/api/v1"]
    E --> G["/litellm"]
    E --> H["/proxy"]
    
    F --> I[Start container with SERVER_ROOT_PATH]
    G --> I
    H --> I
    
    I --> J[Wait for Uvicorn]
    J --> K[Test /health]
    K --> L[Test /health/liveliness]
    L --> M[Test /health/readiness]
    M --> N[Test /ui]
    N --> O[Test /ui/ content]
    O --> P[Test /docs]
    P --> Q[Test /openapi.json]
    Q --> R[Test /v1/models with auth]
    R --> S[Verify 404 without prefix]
    S --> T[Run unit tests in container]
    T --> U[Cleanup]
    
    C --> V[Build Docker Image]
    V --> W[Start container without SERVER_ROOT_PATH]
    W --> X[Wait for Uvicorn]
    X --> Y[Test /health and /ui at root]
    Y --> Z[Cleanup]
Loading

Last reviewed commit: 0e736a7

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +225 to +228
- name: Run unit tests for SERVER_ROOT_PATH
run: |
docker exec litellm-test pip install pytest pytest-asyncio
docker exec litellm-test pytest /app/tests/proxy_unit_tests/test_server_root_path.py -v
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.

Missing if: always() on cleanup dependency

If the "Run unit tests" step fails, the workflow proceeds to "Cleanup" because it has if: always(). However, the pip install on line 227 could fail and leave pytest uninstalled, causing a confusing error on line 228. Consider combining them in a single docker exec call or adding || true to the install step so the test step still runs (or fails clearly). This is minor since a pip install failure would be unusual, but it could mask the real issue during debugging.

More importantly, if the container failed to start or crashed between the health check steps and this step, docker exec will fail with an opaque error. Consider adding a container health check before running the exec.

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@yuneng-jiang yuneng-jiang merged commit 96d7585 into main Feb 17, 2026
14 of 25 checks passed
@yuneng-jiang yuneng-jiang deleted the litellm_gh_server_root_test branch February 19, 2026 18:57
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.

1 participant