-
Notifications
You must be signed in to change notification settings - Fork 566
added image api tests #1084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
added image api tests #1084
Conversation
Signed-off-by: Pranjali Bhardwaj <pranjalisharma6543@gmail.com>
|
|
📝 WalkthroughWalkthroughThe pull request enhances exception handling in the Images API toggle-favourite route to explicitly re-raise HTTP exceptions while preserving generic error handling, adds conditional database setup skipping in test configuration, and introduces comprehensive test coverage for the Images API routes with 222 new test lines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/routes/images.py (1)
108-114: PotentialAttributeErrorif image is not found after toggle.If
db_toggle_image_favourite_statusreturnsTruebut the image is subsequently not found indb_get_all_images()(e.g., race condition or data inconsistency),imagewill beNoneand line 114 will raiseAttributeError: 'NoneType' object has no attribute 'get'.Proposed fix to handle the None case
image = next( (img for img in db_get_all_images() if img["id"] == image_id), None ) + if image is None: + raise HTTPException( + status_code=404, detail="Image not found after toggle" + ) return { "success": True, "image_id": image_id, - "isFavourite": image.get("isFavourite", False), + "isFavourite": image.get("isFavourite", False), }
🧹 Nitpick comments (1)
backend/tests/test_images.py (1)
62-74: Unused fixturemock_db_image_untagged.This fixture is defined but not used in any of the current tests. Consider removing it to avoid dead code, or add tests that exercise the untagged image scenario (e.g., verifying that
tags=Noneis handled correctly in the response).
Issue: #1083
How I added tests for the Images API
I used test_albums.py as a reference so everything stays consistent:
Same setup: imports → FastAPI app → include_router("/images") → TestClient, Same mocking style, Same structure: fixtures at the top, then test classes, Same pattern for requests (client.get / client.post) and assertions on status + response
_make_metadata() – creates minimal metadata needed by the API, mock_db_image – one sample image returned by db_get_all_images, mock_db_image_untagged – same image but with isTagged=False, These mocks make sure responses match the API schema without using a real DB.
GET /images/:
Success case (returns one image)
Empty list case
Tagged filter (tagged=True / False)
Internal error (returns 500 with proper message)
POST /images/toggle-favourite
Success case (favourite toggled)
Image not found (404) and Internal error (500) and Missing image_id (422 validation error)
Each test also checks that the correct DB/helper functions were called.
The session DB setup in conftest.py was failing with a disk I/O error, so image tests were not running.
Since these tests are fully mocked and don’t need a real DB:
I added a flag: PICTOPY_SKIP_DB_SETUP=1
When this is set, DB creation is skipped and only TEST_MODE is enabled
This allows image tests to run cleanly in this environment.
output:
All 9 new tests are passing. The route fix ensures “image not found” correctly returns 404 instead of 500.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.