[test]: Backend unit test for creating a record set from multiple entry points#8309
[test]: Backend unit test for creating a record set from multiple entry points#8309rijulpoudel wants to merge 3 commits into
Conversation
|
Warning One or more dependencies are approaching or past End-of-Life. |
📝 WalkthroughWalkthroughAdds ChangesRecordset creation test coverage
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specifyweb/specify/tests/test_record_sets.py (1)
62-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptimize query evaluation and strengthen assertions.
You can optimize memory usage by evaluating only the
idfield usingvalues_list, rather than loading full model instances. Additionally, you could verify that the specific record IDs match, ensuring parity with the first test case.♻️ Proposed refactor
- matching_cos = Collectionobject.objects.filter( - collection=self.collection - ) - - Recordsetitem.objects.bulk_create([ - Recordsetitem(recordset=recordset, recordid=co.id) - for co in matching_cos - ]) - - self.assertEqual( - recordset.recordsetitems.count(), - matching_cos.count(), - ) + expected_ids = set( + Collectionobject.objects.filter( + collection=self.collection + ).values_list("id", flat=True) + ) + + Recordsetitem.objects.bulk_create([ + Recordsetitem(recordset=recordset, recordid=co_id) + for co_id in expected_ids + ]) + + self.assertEqual(recordset.recordsetitems.count(), len(expected_ids)) + + rs_item_ids = set( + recordset.recordsetitems.values_list("recordid", flat=True) + ) + self.assertEqual(rs_item_ids, expected_ids)🤖 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 `@specifyweb/specify/tests/test_record_sets.py` around lines 62 - 74, Update the matching_cos query in this test to retrieve only collection object IDs via values_list, and use those IDs when building Recordsetitem instances. Replace or supplement the count-only assertion with an assertion that the created recordset item IDs exactly match the queried IDs, preserving the existing parity check.
🤖 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.
Nitpick comments:
In `@specifyweb/specify/tests/test_record_sets.py`:
- Around line 62-74: Update the matching_cos query in this test to retrieve only
collection object IDs via values_list, and use those IDs when building
Recordsetitem instances. Replace or supplement the count-only assertion with an
assertion that the created recordset item IDs exactly match the queried IDs,
preserving the existing parity check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 274c0905-a71d-490e-b7be-5cf327ea5a02
📒 Files selected for processing (1)
specifyweb/specify/tests/test_record_sets.py
Fixes #8308
Summary by CodeRabbit