Add systematic testing for model tables #11241

Open
opened 2025-12-29 21:42:22 +01:00 by adam · 0 comments
Owner

Originally created by @jnovinger on GitHub (May 30, 2025).

Proposed Changes

Much like we have thorough test suites for new models and model views, I propose that we implement something similar for model tables (that is table classes associated with models that build off of django-tables2) and apply it to all new and existing model table classes.

The first concrete use case for a table test suite is: testing whether all fields/columns that satisfy bool(<column>.orderable) be tested to ensure that

  1. they do actually resulted in an ordered table data set and
  2. they do not break rendering of the table while trying to evaluate the data set (most likely an instance of Django's QuerySet).

You can view a prototype of this specific test in the work I did to fix bug #19487 (and beloew).

class CircuitTerminationTableTest(TestCase):
    def test_every_orderable_field_does_not_throw_exception(self):
        terminations = CircuitTermination.objects.all()
        disallowed = {'actions', }

        orderable_columns = [
            column.name for column in CircuitTerminationTable(terminations).columns
            if column.orderable and column.name not in disallowed
        ]
        fake_request = RequestFactory().get("/")

        for col in orderable_columns:
            for dir in ('-', ''):
                table = CircuitTerminationTable(terminations)
                table.order_by = f'{dir}{col}'
                table.as_html(fake_request)

My expectation is that once the new column ordering testing is implemented, there will be subsequent tickets needed to remedy issues discovered in other model table by the test. I also expect that we will find more use cases for this test suite that will help us identify more issues with table early.

Justification

We've seen a number of issues over the years with certain table fields not being marked as orderable = False when they should have been. Common occurrences are table fields that display GFK values, calculated (outside of the database) columns, and even columns that don't exist on the model.

Most often, this results in an instance of Django's FieldError being thrown and the page crashing. When this happens it results in poor user experience. While we've been very lucky to have an active community that is happy to report these bugs, it still has drawbacks.

Beyond the poor experience noted above, it puts the onus on the user to report the issue to the NetBox project, meaning that there is a decent chance the bug is never reported (and even when it does get reported, it's often difficult for the reporter to provide enough details for a maintainer to reproduce, resulting in stale and closed issues). In those cases where it does get reported, it requires project maintainer time and resources to triage the bug, develop a fix, and release the fix in a new version. Finally, the user still has to wait for their NetBox instance to be updated to the new version that contains the fix, resulting in continued poor experience.

With this addition to our testing suite we can identify these issue early, ideally during development time, well before they are discovered by end users of NetBox. As a practical example, a test like the one demonstrated above, but adapted for TunnelTerminationTable, resulted in identifying #19610.

TODO

  • Implement test suite such that it can easily be inherited from to provide the included tests to whichever model table is associated.
  • Implement the first concrete test, ensuring that all fields that are indicated to be orderable, are in fact orderable
  • Add new test_tables.py modules to NetBox apps as needed (they may already exist in some cases) and create a concrete test class for each model table in the app.
    • As testing for each model table is enabled, file bug issues for and whitelist any offending columns.
    • New bug issues should link back to this housekeeping issue and be tagged with the topic: industrialization label.
    • Work to fix these new issues should include removing fixed up fields from the tests whitelist.
Originally created by @jnovinger on GitHub (May 30, 2025). ### Proposed Changes Much like we have thorough test suites for new models and model views, I propose that we implement something similar for model tables (that is table classes associated with models that build off of django-tables2) and apply it to all new and existing model table classes. The first concrete use case for a table test suite is: testing whether all fields/columns that satisfy `bool(<column>.orderable)` be tested to ensure that 1. they do actually resulted in an ordered table data set and 2. they do not break rendering of the table while trying to evaluate the data set (most likely an instance of Django's `QuerySet`). You can view a [prototype of this specific test](https://github.com/netbox-community/netbox/pull/19600/commits/0c2ac033dd5f40baf4143e71eca4c0b139cfe425) in the work I did to fix bug #19487 (and beloew). ```python class CircuitTerminationTableTest(TestCase): def test_every_orderable_field_does_not_throw_exception(self): terminations = CircuitTermination.objects.all() disallowed = {'actions', } orderable_columns = [ column.name for column in CircuitTerminationTable(terminations).columns if column.orderable and column.name not in disallowed ] fake_request = RequestFactory().get("/") for col in orderable_columns: for dir in ('-', ''): table = CircuitTerminationTable(terminations) table.order_by = f'{dir}{col}' table.as_html(fake_request) ``` My expectation is that once the new column ordering testing is implemented, there will be subsequent tickets needed to remedy issues discovered in other model table by the test. I also expect that we will find more use cases for this test suite that will help us identify more issues with table early. ### Justification We've seen a number of issues over the years with certain table fields not being marked as `orderable = False` when they should have been. Common occurrences are table fields that display GFK values, calculated (outside of the database) columns, and even columns that don't exist on the model. Most often, this results in an instance of Django's `FieldError` being thrown and the page crashing. When this happens it results in poor user experience. While we've been very lucky to have an active community that is happy to report these bugs, it still has drawbacks. Beyond the poor experience noted above, it puts the onus on the user to report the issue to the NetBox project, meaning that there is a decent chance the bug is never reported (and even when it does get reported, it's often difficult for the reporter to provide enough details for a maintainer to reproduce, resulting in stale and closed issues). In those cases where it does get reported, it requires project maintainer time and resources to triage the bug, develop a fix, and release the fix in a new version. Finally, the user still has to wait for their NetBox instance to be updated to the new version that contains the fix, resulting in continued poor experience. With this addition to our testing suite we can identify these issue early, ideally during development time, well before they are discovered by end users of NetBox. As a practical example, a test like the one demonstrated above, but adapted for `TunnelTerminationTable`, resulted in identifying #19610. ### TODO - Implement test suite such that it can easily be inherited from to provide the included tests to whichever model table is associated. - Implement the first concrete test, ensuring that all fields that are indicated to be orderable, are in fact orderable - Add new `test_tables.py` modules to NetBox apps as needed (they may already exist in some cases) and create a concrete test class for each model table in the app. - As testing for each model table is enabled, file bug issues for and whitelist any offending columns. - New bug issues should link back to this housekeeping issue and be tagged with the `topic: industrialization` label. - Work to fix these new issues should include removing fixed up fields from the tests whitelist.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#11241