Add failing test to ensure OpenAPI does not execute lifespan#1167
Add failing test to ensure OpenAPI does not execute lifespan#1167Riddhi8077 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
|
|
📝 WalkthroughWalkthroughA new test file is introduced to validate that OpenAPI generation failures do not crash the application. The test mocks the OpenAPI generation to raise an exception and verifies the app remains stable despite the failure. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@backend/tests/test_openapi_lifespan.py`:
- Line 15: Remove the unused local FastAPI() instance and its import from the
test; instead reference the module-level app used by generate_openapi_json()
(i.e., rely on generate_openapi_json() from the application module or import the
module-level app if needed) so the test actually exercises
generate_openapi_json() rather than a stray local app variable; delete the line
creating app = FastAPI() and the FastAPI import and update imports to call
generate_openapi_json() from the application module.
- Around line 17-23: The test currently uses a redundant try/except with a
boolean flag because generate_openapi_json already swallows exceptions; either
simplify the test to directly call generate_openapi_json() (no try/except/flag)
and let the test fail on unexpected exceptions, or, if the goal is to assert
that generating the OpenAPI schema does not trigger lifespan events, replace the
boolean pattern with instrumentation/mocking of the app's lifespan handler and
assert that the mocked lifespan handler was not called during the call to
generate_openapi_json(); locate generate_openapi_json in main.py and update
backend/tests/test_openapi_lifespan.py accordingly to either remove the
unnecessary try/except and assert nothing (or simply call the function), or set
up a mock/spy for the application lifespan handler and assert it remains
uninvoked after generate_openapi_json().
- Around line 1-2: Remove the unused FastAPI import: the test file imports
FastAPI but never uses it, so delete the FastAPI import token to avoid
unused-import warnings; keep the pytest import for fixtures like monkeypatch and
ensure only pytest remains in the import statement.
| import pytest | ||
| from fastapi import FastAPI |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import.
FastAPI (line 2) is imported but the instance created from it is unused by the test logic. pytest (line 1) is fine since monkeypatch is a pytest fixture.
Proposed fix
import pytest
-from fastapi import FastAPI
from main import generate_openapi_json📝 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.
| import pytest | |
| from fastapi import FastAPI | |
| import pytest | |
| from main import generate_openapi_json |
🤖 Prompt for AI Agents
In `@backend/tests/test_openapi_lifespan.py` around lines 1 - 2, Remove the unused
FastAPI import: the test file imports FastAPI but never uses it, so delete the
FastAPI import token to avoid unused-import warnings; keep the pytest import for
fixtures like monkeypatch and ensure only pytest remains in the import
statement.
| broken_openapi | ||
| ) | ||
|
|
||
| app = FastAPI() |
There was a problem hiding this comment.
Unused FastAPI instance — not connected to generate_openapi_json().
generate_openapi_json() operates on the module-level app object defined in main.py, not on this local instance. This line (and the FastAPI import on line 2) has no effect and is misleading, as it suggests the test exercises this app.
🤖 Prompt for AI Agents
In `@backend/tests/test_openapi_lifespan.py` at line 15, Remove the unused local
FastAPI() instance and its import from the test; instead reference the
module-level app used by generate_openapi_json() (i.e., rely on
generate_openapi_json() from the application module or import the module-level
app if needed) so the test actually exercises generate_openapi_json() rather
than a stray local app variable; delete the line creating app = FastAPI() and
the FastAPI import and update imports to call generate_openapi_json() from the
application module.
|
Opened this PR to submit the initial failing test as per contribution workflow. |
1 participant
This PR adds a focused failing test to verify that generating OpenAPI schema does not trigger application lifespan events.
No functional changes are included yet - this is only the initial test as per contribution workflow.
Summary by CodeRabbit