Closes #21766: Add reusable tests for sortable table columns#21836
Conversation
|
@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration. |
|
Claude finished @pheus's task in 3m 28s —— View job Code Review
A few items worth considering: 1. Tests run against empty querysets — ordering bugs with actual data won't be caught
2. Loss of The pre-existing tests in 3. Misleading
queryset_sources = [
('FHRPGroupAssignmentTable', FHRPGroupAssignment.objects.all()),
]The first element of each tuple is used as the 4. Fictitious view-name labels for
5.
|
209c60e to
b8b9cf1
Compare
|
@claude /review
3/4. Also agreed on the manual queryset labels. I’ll rename those to reflect the actual source more clearly, or use neutral queryset labels where there isn’t a real list view.
|
|
Claude finished @pheus's task in 1m 24s —— View job Code Review
All five items from the previous review have been addressed:
Nothing further to flag. The helper is clean and the per-app tests are well-structured. |
jeremystretch
left a comment
There was a problem hiding this comment.
Nice work @pheus, really clean! Approving this with two minor administrative points for your consideration.
netbox/circuits/tests/test_tables.py
Outdated
| table = CircuitTable | ||
|
|
||
|
|
||
| @tag("regression") |
There was a problem hiding this comment.
Given that we're moving to a new testing mechanism for tables, IMO it makes sense to remove these tags. (It's safe to ignore Claude's earlier feedback.)
netbox/utilities/testing/tables.py
Outdated
| base classes directly. | ||
| """ | ||
|
|
||
| class OrderableColumnsTestCase(ModelTableTestCase): |
There was a problem hiding this comment.
Should we give this a more generic name? Seems reasonable to assume we'll add tests unrelated to ordering in the future.
|
That makes sense to me on both points. I’ll drop the regression tags from the migrated table tests. I’ll also rename |
Introduce `TableTestCases.StandardTableTestCase`, a shared base class for model-backed table smoke tests. It currently discovers sortable columns from list-view querysets and verifies that each renders without exceptions in both ascending and descending order. Add per-table smoke tests across circuits, core, dcim, extras, ipam, tenancy, users, virtualization, vpn, and wireless apps. Fixes #21766
b8b9cf1 to
19e21ba
Compare
Fixes: #21766
Introduce
TableTestCases.OrderableColumnsTestCase, a reusable base class for table ordering smoke tests.The new helper discovers sortable columns from list-view querysets and verifies that each one renders without exceptions when ordered in both ascending and descending directions. This gives us a consistent way to catch unsupported ordering across model-backed tables without duplicating the same test logic in each app.
This PR also adds per-table smoke tests across circuits, core, dcim, extras, ipam, tenancy, users, virtualization, vpn, and wireless.