test: add database metadata and YOLO mapping coverage#1343
test: add database metadata and YOLO mapping coverage#1343priyanshunitr wants to merge 1 commit into
Conversation
WalkthroughAdds pytest coverage for SQLite-backed metadata persistence and YOLO class mapping behavior, using temporary databases and monkeypatched database paths. ChangesDatabase persistence test coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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_metadata.py`:
- Around line 118-128: The current test in
test_update_metadata_rolls_back_when_json_serialization_fails only exercises
json.dumps failure before any database writes, so it does not verify
transactional rollback. Update this test to trigger an exception after
db_update_metadata has started its DELETE/INSERT path in
metadata_db.db_update_metadata, such as by mocking a failing execute or commit
on the database connection, then assert db_get_metadata still returns the
original_metadata. Keep the test focused on the rollback behavior in
backend/app/database/metadata.py and cover the critical transactional path the
PR is meant to protect.
In `@backend/tests/test_yolo_mapping.py`:
- Around line 45-64: Add a regression test for stale YOLO mappings in
db_create_YOLO_classes_table by first seeding the database with existing class
rows, then shrinking yolo_mapping_db.class_names to a shorter list or [] and
calling the method again. Use fetch_mappings on yolo_database to assert removed
classes are actually deleted, not just overwritten, so the test covers the
replacement/clear behavior that currently leaks stale class IDs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e4c5a742-ddec-4199-ad79-ab4b1fd40c8a
📒 Files selected for processing (2)
backend/tests/test_metadata.pybackend/tests/test_yolo_mapping.py
| # Validates that update failures roll back without deleting previous metadata. | ||
| def test_update_metadata_rolls_back_when_json_serialization_fails(metadata_database): | ||
| metadata_db.db_create_metadata_table() | ||
| original_metadata = {"safe": True} | ||
|
|
||
| assert metadata_db.db_update_metadata(original_metadata) is True | ||
|
|
||
| with pytest.raises(TypeError): | ||
| metadata_db.db_update_metadata({"bad": object()}) | ||
|
|
||
| assert metadata_db.db_get_metadata() == original_metadata No newline at end of file |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
This doesn't actually test rollback after a write begins.
backend/app/database/metadata.py serializes with json.dumps() before it runs DELETE/INSERT, so object() fails before any database state changes. That means this test only proves serialization failure leaves the old row untouched; it will not catch a regression in the transactional rollback path the PR summary calls out. Please force an error after the delete/insert sequence starts (for example by injecting a failing execute/commit) and then assert the original metadata is restored. As per path instructions, verify that all critical functionality is covered by tests.
🤖 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_metadata.py` around lines 118 - 128, The current test in
test_update_metadata_rolls_back_when_json_serialization_fails only exercises
json.dumps failure before any database writes, so it does not verify
transactional rollback. Update this test to trigger an exception after
db_update_metadata has started its DELETE/INSERT path in
metadata_db.db_update_metadata, such as by mocking a failing execute or commit
on the database connection, then assert db_get_metadata still returns the
original_metadata. Keep the test focused on the rollback behavior in
backend/app/database/metadata.py and cover the critical transactional path the
PR is meant to protect.
Source: Path instructions
| # Validates that existing class IDs are replaced when class names change. | ||
| def test_create_yolo_classes_table_replaces_existing_class_names( | ||
| yolo_database, monkeypatch | ||
| ): | ||
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["old-person", "old-car"]) | ||
| yolo_mapping_db.db_create_YOLO_classes_table() | ||
|
|
||
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["person", "car"]) | ||
| yolo_mapping_db.db_create_YOLO_classes_table() | ||
|
|
||
| assert fetch_mappings(yolo_database) == [(0, "person"), (1, "car")] | ||
|
|
||
|
|
||
| # Validates that table creation succeeds when no YOLO classes are configured. | ||
| def test_create_yolo_classes_table_handles_empty_class_list(yolo_database, monkeypatch): | ||
| monkeypatch.setattr(yolo_mapping_db, "class_names", []) | ||
|
|
||
| yolo_mapping_db.db_create_YOLO_classes_table() | ||
|
|
||
| assert fetch_mappings(yolo_database) == [] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add a shrink/clear regression case for stale mappings.
These tests only cover replacement when the new class_names list has the same length, plus an empty list on a fresh database. In backend/app/database/yolo_mapping.py, db_create_YOLO_classes_table() never deletes rows that disappear from class_names, so a populated table followed by ["person"] or [] will leave stale class IDs behind. Since the PR objective explicitly includes replacement behavior and empty class lists, please add a test that starts with existing rows and then shrinks or clears the list. As per path instructions, verify that all critical functionality is covered by tests.
Suggested regression test
+def test_create_yolo_classes_table_removes_stale_rows_when_class_list_shrinks(
+ yolo_database, monkeypatch
+):
+ monkeypatch.setattr(yolo_mapping_db, "class_names", ["person", "car"])
+ yolo_mapping_db.db_create_YOLO_classes_table()
+
+ monkeypatch.setattr(yolo_mapping_db, "class_names", ["person"])
+ yolo_mapping_db.db_create_YOLO_classes_table()
+
+ assert fetch_mappings(yolo_database) == [(0, "person")]📝 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.
| # Validates that existing class IDs are replaced when class names change. | |
| def test_create_yolo_classes_table_replaces_existing_class_names( | |
| yolo_database, monkeypatch | |
| ): | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["old-person", "old-car"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["person", "car"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| assert fetch_mappings(yolo_database) == [(0, "person"), (1, "car")] | |
| # Validates that table creation succeeds when no YOLO classes are configured. | |
| def test_create_yolo_classes_table_handles_empty_class_list(yolo_database, monkeypatch): | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", []) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| assert fetch_mappings(yolo_database) == [] | |
| # Validates that existing class IDs are replaced when class names change. | |
| def test_create_yolo_classes_table_replaces_existing_class_names( | |
| yolo_database, monkeypatch | |
| ): | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["old-person", "old-car"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["person", "car"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| assert fetch_mappings(yolo_database) == [(0, "person"), (1, "car")] | |
| # Validates that table creation succeeds when no YOLO classes are configured. | |
| def test_create_yolo_classes_table_handles_empty_class_list(yolo_database, monkeypatch): | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", []) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| assert fetch_mappings(yolo_database) == [] | |
| def test_create_yolo_classes_table_removes_stale_rows_when_class_list_shrinks( | |
| yolo_database, monkeypatch | |
| ): | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["person", "car"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| monkeypatch.setattr(yolo_mapping_db, "class_names", ["person"]) | |
| yolo_mapping_db.db_create_YOLO_classes_table() | |
| assert fetch_mappings(yolo_database) == [(0, "person")] |
🤖 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_yolo_mapping.py` around lines 45 - 64, Add a regression
test for stale YOLO mappings in db_create_YOLO_classes_table by first seeding
the database with existing class rows, then shrinking
yolo_mapping_db.class_names to a shorter list or [] and calling the method
again. Use fetch_mappings on yolo_database to assert removed classes are
actually deleted, not just overwritten, so the test covers the replacement/clear
behavior that currently leaks stale class IDs.
Source: Path instructions
1 participant
Addressed Issues:
Fixes #1238
Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
Additional Notes:
Adds focused backend test coverage for:
backend/app/database/metadata.pybackend/app/database/yolo_mapping.pyThe tests cover metadata table creation, default/empty/invalid metadata states, metadata updates with own and existing cursors, rollback behavior, YOLO class mapping insertion, idempotency, replacement behavior, empty class lists, and duplicate class names.
This PR keeps the scope limited to the metadata and YOLO mapping database modules discussed in #1238.