Unit tests middleware cicd - #130
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughA continuous integration workflow was introduced for automated testing of a Python API server. The Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub
participant CI Workflow
participant Python Env
participant Test Runner
participant Redis Service
participant Codecov
Developer->>GitHub: Push or PR to main
GitHub->>CI Workflow: Trigger workflow
CI Workflow->>Python Env: Set up Python 3.12
CI Workflow->>Redis Service: Start Redis container with health checks
CI Workflow->>Python Env: Install dependencies with uv (cache enabled)
CI Workflow->>Test Runner: Run pytest with coverage
Test Runner-->>CI Workflow: Return test results and coverage reports
CI Workflow->>Codecov: Upload coverage reports (fail-on-error)
CI Workflow-->>GitHub: Update status
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (1)📚 Learning: the exospherehost project requires python versions > 3.12 for the ci workflow, meaning python 3.13 o...Applied to files:
🪛 YAMLlint (1.37.1).github/workflows/ci.yml[warning] 3-3: truthy value should be one of [false, true] (truthy) [error] 55-55: no new line character at the end of file (new-line-at-end-of-file) 🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
api-server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)api-server/pyproject.toml(1 hunks)api-server/tests/test_request_id_middleware.py(1 hunks)api-server/tests/test_unhandled_exceptions_middleware.py(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
15-15: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
18-18: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 20-20: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/ci.yml (2)
20-20: Verify Python 3.13 compatibility across dependenciesWe ran a classifier check against PyPI and found that while most packages declare support for Python 3.13, the following do not list it explicitly:
• beanie
• docker-image-py
• email-validator
• redis[hiredis]Please:
- Run the full test suite under Python 3.13 in CI (e.g. add
python-version: ['3.11', '3.13']to the matrix).- Manually install and smoke-test the four packages above, or upgrade to versions that advertise 3.13 support.
- Confirm that no runtime issues arise (import errors, deprecations, etc.).
Once verified (or packages updated), we can be confident in promoting Python 3.13 in CI.
3-7: Fix YAML formatting issue.The
onkeyword should use boolean values instead of truthy values.Apply this diff:
on: push: - branches: [main] + branches: ["main"] pull_request: - branches: [main] + branches: ["main"]Likely an incorrect or invalid review comment.
api-server/tests/test_unhandled_exceptions_middleware.py (1)
1-35: LGTM! Well-structured middleware tests.The test implementation follows good practices with proper fixture setup, clear test separation, and coverage of both success and failure scenarios.
api-server/tests/test_request_id_middleware.py (1)
1-38: Excellent test coverage for RequestIdMiddleware!The test implementation comprehensively covers the three main scenarios for request ID handling with proper UUID validation and clear assertions.
| def test_replaces_invalid_id(client): | ||
| resp = client.get("/test", headers={"x-exosphere-request-id": "bad-id"}) | ||
| assert resp.status_code == 200 | ||
| new_id = resp.headers["x-exosphere-request-id"] | ||
| assert new_id != "bad-id" | ||
| uuid.UUID(new_id) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider testing additional edge cases.
The current tests cover the main scenarios well. Consider adding tests for edge cases like empty strings, whitespace-only strings, or extremely long invalid IDs.
You could add additional test cases:
def test_replaces_empty_id(client):
resp = client.get("/test", headers={"x-exosphere-request-id": ""})
assert resp.status_code == 200
new_id = resp.headers["x-exosphere-request-id"]
assert new_id != ""
uuid.UUID(new_id)
def test_replaces_whitespace_id(client):
resp = client.get("/test", headers={"x-exosphere-request-id": " "})
assert resp.status_code == 200
new_id = resp.headers["x-exosphere-request-id"]
assert new_id.strip() != ""
uuid.UUID(new_id)🤖 Prompt for AI Agents
In api-server/tests/test_request_id_middleware.py around lines 32 to 37, the
tests currently check for invalid request IDs but do not cover edge cases like
empty strings, whitespace-only strings, or very long invalid IDs. Add new test
functions that send requests with these edge case headers, verify the response
status is 200, confirm the returned request ID is different from the invalid
input, and validate that the new ID is a proper UUID.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
18-21: Re-evaluate hard-coding Python 3.13
Python 3.13 is still in pre-release; GitHub runners may not yet provide a stable binary andactions/setup-python@v5will silently fall back to the latest stable (currently 3.12). If the codebase truly depends on 3.13 features, the workflow could fail unpredictably in the near term. Consider one of:- python-version: '3.13' + python-version: '3.12' # or the lowest supported version +# python-version-file: '.python-version' # keep in sync with repo configor introduce a matrix to cover the supported range:
strategy: matrix: python-version: ['3.11', '3.12']
|
Why create another PR? Let's keep only 1 PR and close the other one.
…On Sat, 2 Aug, 2025, 13:22 coderabbitai[bot], ***@***.***> wrote:
***@***.***[bot]* commented on this pull request.
*Actionable comments posted: 2*
📜 Review details
*Configuration used: CodeRabbit UI*
*Review profile: ASSERTIVE*
*Plan: Pro*
📥 Commits
Reviewing files that changed from the base of the PR and between 18902ab
<18902ab>
and 80ba9f9
<80ba9f9>
.
📒 Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
18-21: *Re-evaluate hard-coding Python 3.13*
Python 3.13 is still in pre-release; GitHub runners may not yet provide a
stable binary and ***@***.*** will silently fall back to the
latest stable (currently 3.12). If the codebase truly depends on 3.13
features, the workflow could fail unpredictably in the near term. Consider
one of:
- python-version: '3.13'+ python-version: '3.12' # or the lowest supported version+# python-version-file: '.python-version' # keep in sync with repo config
or introduce a matrix to cover the supported range:
strategy:
matrix:
python-version: ['3.11', '3.12']
------------------------------
In .github/workflows/ci.yml
<#130 (comment)>
:
> + - name: Install uv
+ uses: ***@***.***
+
+ - name: Install dev dependencies with uv
+ working-directory: api-server
+ run: |
+ uv sync --group dev
+
*🧹 Nitpick (assertive)*
*Add dependency caching to speed up CI runs*
Each push/PR now rebuilds the entire uv environment, which noticeably
inflates runtime. GitHub Actions offers native caching and
astral-sh/setup-uv supports it.
- - name: Install uv- uses: ***@***.***+ - name: Install uv+ uses: ***@***.***+ with:+ cache: true # enables automatic actions/cache reuse
This can cut minutes off the feedback loop without extra maintenance.
📝 Committable suggestion
|
|
Closed the other PR |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
api-server/pyproject.toml (1)
23-33: Broken table & unmatched bracket –[dependency-groups]declared twiceLines 23-28 introduce a second
[dependency-groups]table and leave a stray closing](Line 28).
This produces both a duplicate table error and invalid TOML syntax.Consolidate the dev group into the single table that already starts at Line 30:
-[dependency-groups] -dev = [ - "ruff>=0.12.2", - "pytest>=8.0.0", -] -] - -[dependency-groups] -dev = [ - "ruff>=0.12.2", -] +[dependency-groups] +dev = [ + "ruff>=0.12.2", + "pytest>=8.0.0", +]This removes the duplicate table, fixes the bracket imbalance, and keeps
pytestin the correct dev group (per the earlier review).
After this change the file should parse and the CI workflow will succeed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
api-server/pyproject.toml (1)
27-30: Duplicate[dependency-groups]block – remove the second instanceLines 27-30 re-declare the same table, causing a duplicate-key error. Drop the redundant block or merge any unique entries into the first one.
-[dependency-groups] -dev = [ - "ruff>=0.12.2", -]
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
31-34: Surface test results & coverage artefacts for better PR feedback
Pytest output goes to stdout only, so GitHub can’t annotate failures or track coverage trends.- - name: Run tests with pytest + - name: Run tests with pytest & publish report working-directory: api-server run: | - uv run pytest --maxfail=1 --disable-warnings -v + uv run pytest \ + --maxfail=1 \ + --disable-warnings \ + --junitxml=pytest-report.xml \ + -v + + - name: Upload test report + if: always() + uses: actions/upload-artifact@v4 + with: + name: pytest-report + path: api-server/pytest-report.xml + + - name: Measure coverage + working-directory: api-server + run: | + uv run coverage run -m pytest + uv run coverage xml -o coverage.xml + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@v4 + with: + name: coverage + path: api-server/coverage.xmlThis lets the UI highlight failing lines and opens the door to coverage-gating later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
22-26: Nice touch—dependency caching enabled
Enablingcache: trueonastral-sh/setup-uvmakes repeated runs significantly faster. Good improvement.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
api-server/pyproject.toml (1)
6-6:requires-pythonis incompatible with the CI matrix
requires-python = ">=3.12"blocks installation on Python 3.11, yet the workflow runs tests on 3.11. Either widen the requirement (e.g.,">=3.11") or drop 3.11 from the matrix, otherwise dependency resolution will fail in CI and for any users on 3.11.
♻️ Duplicate comments (2)
api-server/pyproject.toml (1)
18-18: Trailing comma on the same line as the closing bracket is legal but jarring
"redis[hiredis]>=6.0.0",]is valid TOML, yet it’s an uncommon style that may trip linters or future contributors. Consider moving]to its own line for consistency with the rest of the array.- "redis[hiredis]>=6.0.0",] + "redis[hiredis]>=6.0.0", +].github/workflows/ci.yml (1)
34-37: Consider emitting JUnit & coverage artefacts for richer feedbackPublishing a JUnit XML and coverage report lets GitHub annotate failures inline and enforces test coverage thresholds.
- uv run pytest --maxfail=1 --disable-warnings -v + uv run pytest \ + --maxfail=1 --disable-warnings -v \ + --junitxml=pytest-report.xml \ + --cov=api_server --cov-report=xmlYou can then upload the reports with
actions/upload-artifactoractions/upload-test-results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)api-server/pyproject.toml(1 hunks)
🔇 Additional comments (2)
api-server/pyproject.toml (1)
23-24: Good call adding test-only tooling to thedevgroupPlacing
pytestandhttpxunderdevcleanly separates runtime from development dependencies..github/workflows/ci.yml (1)
25-29: Nice! Dependency caching is enabledUsing
cache: trueforsetup-uvwill materially speed up repeated runs.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
api-server/tests/test_unhandled_exceptions_middleware.py (1)
8-9: Consider testing different exception types.The test only covers
RuntimeError. Consider adding tests for other exception types to ensure comprehensive middleware behavior validation.api-server/tests/test_request_id_middleware.py (1)
34-41: Consider testing additional edge cases.The current tests cover the main scenarios well. Consider adding tests for edge cases like empty strings, whitespace-only strings, or extremely long invalid IDs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
api-server/tests/test_request_id_middleware.py(1 hunks)api-server/tests/test_unhandled_exceptions_middleware.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
api-server/tests/test_unhandled_exceptions_middleware.py (2)
api-server/tests/test_request_id_middleware.py (1)
client(13-17)api-server/app/middlewares/unhandled_exceptions_middleware.py (1)
UnhandledExceptionsMiddleware(10-30)
🔇 Additional comments (6)
api-server/tests/test_unhandled_exceptions_middleware.py (2)
15-21: LGTM: Clean test fixture setup.The test fixture properly sets up a FastAPI app with the middleware and defines appropriate test routes for both error and success scenarios.
30-33: LGTM: Normal request test is appropriate.The test correctly verifies that normal requests pass through the middleware unaffected.
api-server/tests/test_request_id_middleware.py (4)
8-10: LGTM: Simple test endpoint.The minimal endpoint is appropriate for testing middleware functionality without unnecessary complexity.
12-17: LGTM: Well-structured test fixture.The fixture properly sets up a FastAPI app with only the RequestIdMiddleware, providing good isolation for testing.
19-26: LGTM: Explicit UUID validation implemented.The test correctly validates UUID generation and uses explicit assertion as suggested in previous reviews.
28-32: LGTM: Valid ID preservation test.The test appropriately verifies that valid UUIDs are preserved by the middleware.
NiveditJain
left a comment
NiveditJain
left a comment
There was a problem hiding this comment.
@namidanam please fix the review comments, also along with to merge this PR you will need to:
- Fix failing test in actions (you will need to add Redis to actions pipeline and connect it to main app for tests)
- Connect codecov
- Fix comments
| with: | ||
| # Prefer released versions or test a matrix of supported ones | ||
| python-version: | | ||
| 3.11 |
There was a problem hiding this comment.
valid comment by @coderabbitai, this should be only for python versions > 3.12
There was a problem hiding this comment.
@NiveditJain, understood! I'll update the CI workflow to use Python versions > 3.12. Here's the corrected configuration:
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: |
3.13Or if you want to test against multiple versions > 3.12:
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: |
3.13
3.14-devNote that Python 3.13 is currently in release candidate stage, so there might be occasional stability issues with the GitHub runners, but if your project specifically requires features from Python 3.13+, this is the right approach.
✏️ Learnings added
Learnt from: NiveditJain
PR: exospherehost/exospherehost#130
File: .github/workflows/ci.yml:22-22
Timestamp: 2025-08-02T12:43:35.052Z
Learning: The exospherehost project requires Python versions > 3.12 for the CI workflow, meaning Python 3.13 or higher should be used despite potential stability concerns with pre-release versions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| "uvicorn>=0.35.0", | ||
| "redis[hiredis]>=6.0.0" | ||
| ] | ||
| "redis[hiredis]>=6.0.0",] |
There was a problem hiding this comment.
nit:
"redis[hiredis]>=6.0.0"
]
…se test, Redis CI service and artefacts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
api-server/pyproject.toml (1)
18-18: Bracket-on-same-line hurts readabilityClosing the
dependenciesarray on the same line as the last element is valid TOML, but it’s inconsistent with the multi-line style used above and makes future diffs noisy.- "redis[hiredis]>=6.0.0"] + "redis[hiredis]>=6.0.0", +]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)api-server/pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: the exospherehost project requires python versions > 3.12 for the ci workflow, meaning python 3.13 o...
Learnt from: NiveditJain
PR: exospherehost/exospherehost#130
File: .github/workflows/ci.yml:22-22
Timestamp: 2025-08-02T12:43:35.052Z
Learning: The exospherehost project requires Python versions > 3.12 for the CI workflow, meaning Python 3.13 or higher should be used despite potential stability concerns with pre-release versions.
Applied to files:
.github/workflows/ci.yml
🪛 actionlint (1.7.7)
.github/workflows/ci.yml
48-48: "steps" section is missing in job "uses"
(syntax-check)
48-48: "runs-on" section is missing in job "uses"
(syntax-check)
48-48: "uses" job is scalar node but mapping node is expected
(syntax-check)
49-49: "steps" section is missing in job "with"
(syntax-check)
49-49: "runs-on" section is missing in job "with"
(syntax-check)
50-50: unexpected key "token" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
51-51: unexpected key "slug" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
52-52: unexpected key "files" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
53-53: unexpected key "flags" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
55-55: unexpected key "fail_ci_if_error" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 12-12: trailing spaces
(trailing-spaces)
[error] 31-31: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 56-56: too many blank lines (1 > 0)
(empty-lines)
|
resolved the mentioned issues, please review if any more changes are to be made |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
2 participants
This PR separates unit tests for RequestIdMiddleware and UnhandledExceptionsMiddleware into individual files for improved clarity and maintainability.
Adds/updates test files in api-server/tests/.
Adds pytest to dev dependencies.
Updates the CI workflow to automatically install dependencies with uv and run all tests on every commit and pull request.