ci: run backend pytest on backend changes#295
Conversation
github-actions
Bot
added
the
needs linked issue
PR Context Summary
Suggested issue links
Use |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a backend health check smoke test and integrates pytest execution into the CI workflow. The smoke test validates that the FastAPI ChangesBackend smoke test and CI pytest step
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review Unable to check for correctness in 4345aa0. You can customize Macroscope's approvability policy. Learn more. |
|
Hey @divgunjan! Saw your work on GSSoC 2026. We are building TermUI, a TypeScript terminal UI framework with React-style hooks and JSX, rendered entirely in the terminal. We have 74 unassigned GSSoC issues open, including well-scoped bug fixes with clear repro steps. Your TypeScript background transfers directly. Karanjot, TermUI maintainer |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/test_smoke.py`:
- Around line 4-12: The TestClient is created at module scope which can skip
ASGI lifespan events; update the test_health function to instantiate
TestClient(app) inside a context manager (use with TestClient(app) as client:)
so startup/shutdown lifespans run and the test is isolated; modify references to
the module-level client to use the local client variable within test_health
(function name: test_health, symbol: TestClient and client).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fd55d5a-b7cc-4a78-9df9-1f073bed6acf
📒 Files selected for processing (2)
backend/tests/test_smoke.pyfrontend/next-env.d.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/next-env.d.ts
| client = TestClient(app) | ||
|
|
||
|
|
||
| def test_health(): | ||
| resp = client.get("/health") | ||
| assert resp.status_code == 200 | ||
| data = resp.json() | ||
| assert isinstance(data, dict) | ||
| assert data.get("status") == "healthy" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For FastAPI 0.135.1 / Starlette TestClient, is lifespan (startup/shutdown) guaranteed when TestClient is instantiated at module scope without using with TestClient(app) as client?
💡 Result:
No, lifespan startup and shutdown events are not guaranteed to run if you instantiate TestClient without using the context manager (the with statement) [1][2][3]. When you use TestClient(app) as a standalone object without the with block, the application's lifespan lifecycle events (such as database connection initialization or cleanup) are skipped [1][3]. The TestClient is specifically designed to trigger these events only when entered as a context manager (e.g., with TestClient(app) as client:), which ensures the ASGI lifespan cycle is properly managed [4][2]. If your application relies on lifespan events for configuration, state setup, or resource management, failing to use the context manager will result in these resources not being initialized, frequently causing tests to fail with errors like KeyErrors or missing database connections [1][5][3]. To ensure correct behavior, you should always wrap the TestClient in a with statement or use it within a pytest fixture [1][3].
Citations:
- 1: https://openillumi.com/en/en-fastapi-lifespan-test-client-fix/
- 2: https://starlette.dev/testclient/
- 3: https://thecodeforge.io/python/fastapi-testing-pytest/
- 4: https://fastapi.tiangolo.com/advanced/testing-events/
- 5: https://stackoverflow.com/questions/78845218/fastapi-testclient-not-starting-lifetime-in-test
Use a per-test TestClient context to ensure ASGI lifespan runs.
Instantiating TestClient(app) at module scope can skip application startup/shutdown lifespan events; Starlette runs lifespan reliably only when TestClient is used via a context manager. Instantiate it inside the test for isolation.
Suggested change
-client = TestClient(app)
-
-
def test_health():
- resp = client.get("/health")
- assert resp.status_code == 200
- data = resp.json()
- assert isinstance(data, dict)
- assert data.get("status") == "healthy"
+ with TestClient(app) as client:
+ resp = client.get("/health")
+ assert resp.status_code == 200
+ data = resp.json()
+ assert isinstance(data, dict)
+ assert data.get("status") == "healthy"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_smoke.py` around lines 4 - 12, The TestClient is created
at module scope which can skip ASGI lifespan events; update the test_health
function to instantiate TestClient(app) inside a context manager (use with
TestClient(app) as client:) so startup/shutdown lifespans run and the test is
isolated; modify references to the module-level client to use the local client
variable within test_health (function name: test_health, symbol: TestClient and
client).
|
@Abhash-Chakraborty can u look into this pr and check for any conflicts that have not been resolved? |
|
can you link this pr to issue. @divgunjan |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
97-100: ⚡ Quick winAdd a timeout to
backend-checkto prevent stuck CI runs.Now that full pytest runs in CI, set
timeout-minuteson the job to cap hangs from stalled tests/network calls.⏱️ Suggested tweak
backend-check: needs: detect-changes if: needs.detect-changes.outputs.backend == 'true' || needs.detect-changes.outputs.shared == 'true' || needs.detect-changes.outputs.uncategorized == 'true' runs-on: ubuntu-latest + timeout-minutes: 20 defaults: run: working-directory: ./backend🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 97 - 100, The backend-check GitHub Actions job lacks a timeout and can hang; add a timeout-minutes key to the backend-check job definition (the job named "backend-check") to cap CI run time (e.g., timeout-minutes: 60) directly under the job-level keys so the workflow aborts stalled pytest/test-network hangs; ensure the new key is aligned with other job keys (alongside needs and runs-on) in the backend-check job block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1-20: The workflow uses default GITHUB_TOKEN permissions; add an
explicit top-level permissions block with least privilege (e.g., permissions:
contents: read) and then raise permissions only in jobs that need more (for
example in jobs like frontend-check, backend-check, compose-check) by adding a
job-level permissions override; modify the workflow YAML to include the
top-level permissions key and adjust individual job permissions where necessary
to avoid broad default token scope.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 97-100: The backend-check GitHub Actions job lacks a timeout and
can hang; add a timeout-minutes key to the backend-check job definition (the job
named "backend-check") to cap CI run time (e.g., timeout-minutes: 60) directly
under the job-level keys so the workflow aborts stalled pytest/test-network
hangs; ensure the new key is aligned with other job keys (alongside needs and
runs-on) in the backend-check job block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ed773d3-2a26-43bd-acb6-0dd996392170
📒 Files selected for processing (1)
.github/workflows/ci.yml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
97-100: ⚡ Quick winAdd a timeout to
backend-checkto prevent stuck CI runs.Now that full pytest runs in CI, set
timeout-minuteson the job to cap hangs from stalled tests/network calls.⏱️ Suggested tweak
backend-check: needs: detect-changes if: needs.detect-changes.outputs.backend == 'true' || needs.detect-changes.outputs.shared == 'true' || needs.detect-changes.outputs.uncategorized == 'true' runs-on: ubuntu-latest + timeout-minutes: 20 defaults: run: working-directory: ./backend🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 97 - 100, The backend-check GitHub Actions job lacks a timeout and can hang; add a timeout-minutes key to the backend-check job definition (the job named "backend-check") to cap CI run time (e.g., timeout-minutes: 60) directly under the job-level keys so the workflow aborts stalled pytest/test-network hangs; ensure the new key is aligned with other job keys (alongside needs and runs-on) in the backend-check job block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1-20: The workflow uses default GITHUB_TOKEN permissions; add an
explicit top-level permissions block with least privilege (e.g., permissions:
contents: read) and then raise permissions only in jobs that need more (for
example in jobs like frontend-check, backend-check, compose-check) by adding a
job-level permissions override; modify the workflow YAML to include the
top-level permissions key and adjust individual job permissions where necessary
to avoid broad default token scope.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 97-100: The backend-check GitHub Actions job lacks a timeout and
can hang; add a timeout-minutes key to the backend-check job definition (the job
named "backend-check") to cap CI run time (e.g., timeout-minutes: 60) directly
under the job-level keys so the workflow aborts stalled pytest/test-network
hangs; ensure the new key is aligned with other job keys (alongside needs and
runs-on) in the backend-check job block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ed773d3-2a26-43bd-acb6-0dd996392170
📒 Files selected for processing (1)
.github/workflows/ci.yml
🛑 Comments failed to post (1)
.github/workflows/ci.yml (1)
1-20:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet explicit least-privilege GitHub token permissions.
This workflow relies on default token permissions, which are broader than needed. Add an explicit
permissionsblock (for example,contents: read) at workflow level, then elevate per-job only if required.🔐 Minimal hardening patch
name: CI +permissions: + contents: read + on: push: branches: ["main"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.name: CI permissions: contents: read # Trigger matrix (which paths wake which jobs): # # Path pattern | frontend-check | backend-check | compose-check # --------------------------------|----------------|---------------|-------------- # frontend/** | yes | no | no # backend/** | no | yes | no # .github/workflows/** | yes | yes | yes # docker-compose*.yml | yes | yes | yes # root/shared non-doc config | yes | yes | no # docs/**, *.md, ISSUE_TEMPLATE | no | no | no on: push: branches: ["main"] pull_request: branches: ["main"] jobs:🧰 Tools
🪛 zizmor (1.25.2)
[warning] 1-150: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 20, The workflow uses default GITHUB_TOKEN permissions; add an explicit top-level permissions block with least privilege (e.g., permissions: contents: read) and then raise permissions only in jobs that need more (for example in jobs like frontend-check, backend-check, compose-check) by adding a job-level permissions override; modify the workflow YAML to include the top-level permissions key and adjust individual job permissions where necessary to avoid broad default token scope.Source: Linters/SAST tools
Abhash-Chakraborty
added
backend
ChatGPT Codex Connector
|
I cleaned this PR up so it is now scoped to the backend pytest CI step plus a small Validated locally:
Keeping this as |
3 participants
Summary
Briefly describe the change and why it is needed.
Fixes #
Type of change
What changed
Screenshots / recordings (for UI changes)
Attach before/after screenshots or a short video.
How to test
List exact steps/commands used to validate this PR.
Checklist
GSSoC'26 checklist
Summary by CodeRabbit
Tests
Chores