From a89feaf856bb242003cac6c5463f5230902b3543 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 21 May 2026 16:35:59 -0400 Subject: [PATCH] Closes #22090: Extend test cases to analyze the number of SQL queries executed --- docs/development/getting-started.md | 12 ++ netbox/circuits/tests/query_counts.json | 24 ++++ netbox/core/tests/query_counts.json | 8 ++ netbox/dcim/tests/query_counts.json | 83 +++++++++++ netbox/extras/tests/query_counts.json | 37 +++++ netbox/ipam/tests/query_counts.json | 37 +++++ netbox/tenancy/tests/query_counts.json | 14 ++ netbox/users/tests/query_counts.json | 14 ++ netbox/utilities/testing/__init__.py | 1 + netbox/utilities/testing/api.py | 4 +- netbox/utilities/testing/query_counts.py | 134 ++++++++++++++++++ netbox/utilities/testing/views.py | 5 +- netbox/virtualization/tests/query_counts.json | 16 +++ netbox/vpn/tests/query_counts.json | 22 +++ netbox/wireless/tests/query_counts.json | 8 ++ 15 files changed, 417 insertions(+), 2 deletions(-) create mode 100644 netbox/circuits/tests/query_counts.json create mode 100644 netbox/core/tests/query_counts.json create mode 100644 netbox/dcim/tests/query_counts.json create mode 100644 netbox/extras/tests/query_counts.json create mode 100644 netbox/ipam/tests/query_counts.json create mode 100644 netbox/tenancy/tests/query_counts.json create mode 100644 netbox/users/tests/query_counts.json create mode 100644 netbox/utilities/testing/query_counts.py create mode 100644 netbox/virtualization/tests/query_counts.json create mode 100644 netbox/vpn/tests/query_counts.json create mode 100644 netbox/wireless/tests/query_counts.json diff --git a/docs/development/getting-started.md b/docs/development/getting-started.md index 993229d95..0aa550a03 100644 --- a/docs/development/getting-started.md +++ b/docs/development/getting-started.md @@ -186,6 +186,18 @@ This is handy for instances where just a few tests are failing and you want to r !!! info NetBox uses [django-rich](https://github.com/adamchainz/django-rich) to enhance Django's default `test` management command. +### SQL Query Count Baselines + +The shared list-test mixins assert the number of SQL queries each list endpoint performs against a baseline checked in alongside the tests. This guards against the accidental introduction of new queries (e.g. N+1 patterns) when a queryset, serializer, or table changes. Baselines are stored per app at `netbox//tests/query_counts.json`, keyed by `:`. + +If a list test fails with a message like `Query count for dcim/site:list_objects changed: expected 16, got 18`, first investigate whether the change is expected. If the new count is correct (e.g. you intentionally added a `prefetch_related`, or removed one), regenerate the baseline: + +```no-highlight +UPDATE_QUERY_COUNTS=1 python manage.py test --keepdb +``` + +`UPDATE_QUERY_COUNTS` mode requires serial execution; do not combine it with `--parallel`. You can target a single test, app, or the full suite — only the keys exercised by the run are updated. Review the resulting diff in the JSON files as part of the PR; a reviewer should be able to see and reason about every query-count change. + ## Submitting Pull Requests Once you're happy with your work and have verified that all tests pass, commit your changes and push it upstream to your fork. Always provide descriptive (but not excessively verbose) commit messages. Be sure to prefix your commit message with the word "Fixes" or "Closes" and the relevant issue number (with a hash mark). This tells GitHub to automatically close the referenced issue once the commit has been merged. diff --git a/netbox/circuits/tests/query_counts.json b/netbox/circuits/tests/query_counts.json new file mode 100644 index 000000000..408dfdca0 --- /dev/null +++ b/netbox/circuits/tests/query_counts.json @@ -0,0 +1,24 @@ +{ + "circuit:api_list_objects": 16, + "circuit:list_objects_with_permission": 22, + "circuitgroup:api_list_objects": 13, + "circuitgroup:list_objects_with_permission": 20, + "circuitgroupassignment:api_list_objects": 17, + "circuitgroupassignment:list_objects_with_permission": 26, + "circuittermination:api_list_objects": 18, + "circuittermination:list_objects_with_permission": 24, + "circuittype:api_list_objects": 13, + "circuittype:list_objects_with_permission": 20, + "provider:api_list_objects": 15, + "provider:list_objects_with_permission": 20, + "provideraccount:api_list_objects": 14, + "provideraccount:list_objects_with_permission": 21, + "providernetwork:api_list_objects": 14, + "providernetwork:list_objects_with_permission": 21, + "virtualcircuit:api_list_objects": 16, + "virtualcircuit:list_objects_with_permission": 24, + "virtualcircuittermination:api_list_objects": 17, + "virtualcircuittermination:list_objects_with_permission": 23, + "virtualcircuittype:api_list_objects": 13, + "virtualcircuittype:list_objects_with_permission": 20 +} diff --git a/netbox/core/tests/query_counts.json b/netbox/core/tests/query_counts.json new file mode 100644 index 000000000..c6a476cd7 --- /dev/null +++ b/netbox/core/tests/query_counts.json @@ -0,0 +1,8 @@ +{ + "datafile:api_list_objects": 10, + "datafile:list_objects_with_permission": 18, + "datasource:api_list_objects": 12, + "datasource:list_objects_with_permission": 20, + "job:api_list_objects": 12, + "job:list_objects_with_permission": 19 +} diff --git a/netbox/dcim/tests/query_counts.json b/netbox/dcim/tests/query_counts.json new file mode 100644 index 000000000..1d7cdda72 --- /dev/null +++ b/netbox/dcim/tests/query_counts.json @@ -0,0 +1,83 @@ +{ + "cable:api_list_objects": 24, + "cable:list_objects_with_permission": 24, + "cablebundle:api_list_objects": 13, + "cablebundle:list_objects_with_permission": 20, + "cabletermination:api_list_objects": 16, + "consoleport:api_list_objects": 14, + "consoleport:list_objects_with_permission": 21, + "consoleporttemplate:api_list_objects": 11, + "consoleserverport:api_list_objects": 14, + "consoleserverport:list_objects_with_permission": 21, + "consoleserverporttemplate:api_list_objects": 11, + "device:api_list_objects": 20, + "device:list_objects_with_permission": 25, + "devicebay:api_list_objects": 14, + "devicebay:list_objects_with_permission": 21, + "devicebaytemplate:api_list_objects": 11, + "devicerole:api_list_objects": 13, + "devicerole:list_objects_with_permission": 20, + "devicetype:api_list_objects": 14, + "devicetype:list_objects_with_permission": 21, + "frontport:api_list_objects": 15, + "frontport:list_objects_with_permission": 25, + "frontporttemplate:api_list_objects": 12, + "interface:api_list_objects": 23, + "interface:list_objects_with_permission": 21, + "interfacetemplate:api_list_objects": 11, + "inventoryitem:api_list_objects": 20, + "inventoryitem:list_objects_with_permission": 23, + "inventoryitemrole:api_list_objects": 13, + "inventoryitemrole:list_objects_with_permission": 20, + "inventoryitemtemplate:api_list_objects": 13, + "location:api_list_objects": 15, + "location:list_objects_with_permission": 22, + "macaddress:api_list_objects": 17, + "macaddress:list_objects_with_permission": 24, + "manufacturer:api_list_objects": 13, + "manufacturer:list_objects_with_permission": 20, + "module:api_list_objects": 18, + "module:list_objects_with_permission": 24, + "modulebay:api_list_objects": 15, + "modulebay:list_objects_with_permission": 21, + "modulebaytemplate:api_list_objects": 11, + "moduletype:api_list_objects": 14, + "moduletype:list_objects_with_permission": 21, + "moduletypeprofile:api_list_objects": 13, + "moduletypeprofile:list_objects_with_permission": 20, + "platform:api_list_objects": 13, + "platform:list_objects_with_permission": 21, + "powerfeed:api_list_objects": 15, + "powerfeed:list_objects_with_permission": 22, + "poweroutlet:api_list_objects": 14, + "poweroutlet:list_objects_with_permission": 22, + "poweroutlettemplate:api_list_objects": 11, + "powerpanel:api_list_objects": 15, + "powerpanel:list_objects_with_permission": 22, + "powerport:api_list_objects": 14, + "powerport:list_objects_with_permission": 21, + "powerporttemplate:api_list_objects": 11, + "rack:api_list_objects": 17, + "rack:list_objects_with_permission": 29, + "rackgroup:api_list_objects": 13, + "rackgroup:list_objects_with_permission": 20, + "rackreservation:api_list_objects": 15, + "rackreservation:list_objects_with_permission": 23, + "rackrole:api_list_objects": 13, + "rackrole:list_objects_with_permission": 20, + "racktype:api_list_objects": 14, + "racktype:list_objects_with_permission": 21, + "rearport:api_list_objects": 15, + "rearport:list_objects_with_permission": 22, + "rearporttemplate:api_list_objects": 12, + "region:api_list_objects": 13, + "region:list_objects_with_permission": 20, + "site:api_list_objects": 16, + "site:list_objects_with_permission": 22, + "sitegroup:api_list_objects": 13, + "sitegroup:list_objects_with_permission": 20, + "virtualchassis:api_list_objects": 16, + "virtualchassis:list_objects_with_permission": 21, + "virtualdevicecontext:api_list_objects": 14, + "virtualdevicecontext:list_objects_with_permission": 20 +} diff --git a/netbox/extras/tests/query_counts.json b/netbox/extras/tests/query_counts.json new file mode 100644 index 000000000..a6807bd29 --- /dev/null +++ b/netbox/extras/tests/query_counts.json @@ -0,0 +1,37 @@ +{ + "bookmark:api_list_objects": 12, + "bookmark:list_objects_with_permission": 17, + "configcontext:api_list_objects": 22, + "configcontext:list_objects_with_permission": 16, + "configcontextprofile:api_list_objects": 12, + "configcontextprofile:list_objects_with_permission": 19, + "configtemplate:api_list_objects": 10, + "configtemplate:list_objects_with_permission": 17, + "customfield:api_list_objects": 10, + "customfield:list_objects_with_permission": 19, + "customfieldchoiceset:api_list_objects": 9, + "customfieldchoiceset:list_objects_with_permission": 16, + "customlink:api_list_objects": 10, + "customlink:list_objects_with_permission": 18, + "eventrule:api_list_objects": 16, + "eventrule:list_objects_with_permission": 23, + "exporttemplate:api_list_objects": 10, + "exporttemplate:list_objects_with_permission": 19, + "imageattachment:api_list_objects": 11, + "imageattachment:list_objects_with_permission": 21, + "journalentry:api_list_objects": 15, + "journalentry:list_objects_with_permission": 24, + "notification:api_list_objects": 12, + "notificationgroup:api_list_objects": 11, + "notificationgroup:list_objects_with_permission": 18, + "savedfilter:api_list_objects": 10, + "savedfilter:list_objects_with_permission": 19, + "subscription:api_list_objects": 12, + "tableconfig:api_list_objects": 11, + "tableconfig:list_objects_with_permission": 19, + "tag:api_list_objects": 10, + "tag:list_objects_with_permission": 18, + "taggeditem:api_list_objects": 12, + "webhook:api_list_objects": 13, + "webhook:list_objects_with_permission": 20 +} diff --git a/netbox/ipam/tests/query_counts.json b/netbox/ipam/tests/query_counts.json new file mode 100644 index 000000000..f0863dc09 --- /dev/null +++ b/netbox/ipam/tests/query_counts.json @@ -0,0 +1,37 @@ +{ + "aggregate:api_list_objects": 14, + "aggregate:list_objects_with_permission": 24, + "asn:api_list_objects": 18, + "asn:list_objects_with_permission": 31, + "asnrange:api_list_objects": 15, + "asnrange:list_objects_with_permission": 22, + "fhrpgroup:api_list_objects": 14, + "fhrpgroup:list_objects_with_permission": 21, + "fhrpgroupassignment:api_list_objects": 18, + "ipaddress:api_list_objects": 14, + "ipaddress:list_objects_with_permission": 21, + "iprange:api_list_objects": 13, + "iprange:list_objects_with_permission": 20, + "prefix:api_list_objects": 13, + "prefix:list_objects_with_permission": 29, + "rir:api_list_objects": 13, + "rir:list_objects_with_permission": 20, + "role:api_list_objects": 13, + "role:list_objects_with_permission": 20, + "routetarget:api_list_objects": 13, + "routetarget:list_objects_with_permission": 21, + "service:api_list_objects": 16, + "service:list_objects_with_permission": 21, + "servicetemplate:api_list_objects": 13, + "servicetemplate:list_objects_with_permission": 20, + "vlan:api_list_objects": 15, + "vlan:list_objects_with_permission": 24, + "vlangroup:api_list_objects": 13, + "vlangroup:list_objects_with_permission": 25, + "vlantranslationpolicy:api_list_objects": 12, + "vlantranslationpolicy:list_objects_with_permission": 20, + "vlantranslationrule:api_list_objects": 12, + "vlantranslationrule:list_objects_with_permission": 21, + "vrf:api_list_objects": 15, + "vrf:list_objects_with_permission": 20 +} diff --git a/netbox/tenancy/tests/query_counts.json b/netbox/tenancy/tests/query_counts.json new file mode 100644 index 000000000..0beb74b26 --- /dev/null +++ b/netbox/tenancy/tests/query_counts.json @@ -0,0 +1,14 @@ +{ + "contact:api_list_objects": 14, + "contact:list_objects_with_permission": 21, + "contactassignment:api_list_objects": 17, + "contactassignment:list_objects_with_permission": 25, + "contactgroup:api_list_objects": 14, + "contactgroup:list_objects_with_permission": 20, + "contactrole:api_list_objects": 13, + "contactrole:list_objects_with_permission": 20, + "tenant:api_list_objects": 14, + "tenant:list_objects_with_permission": 21, + "tenantgroup:api_list_objects": 14, + "tenantgroup:list_objects_with_permission": 20 +} diff --git a/netbox/users/tests/query_counts.json b/netbox/users/tests/query_counts.json new file mode 100644 index 000000000..697e0c65c --- /dev/null +++ b/netbox/users/tests/query_counts.json @@ -0,0 +1,14 @@ +{ + "group:api_list_objects": 10, + "group:list_objects_with_permission": 17, + "objectpermission:api_list_objects": 14, + "objectpermission:list_objects_with_permission": 18, + "owner:api_list_objects": 11, + "owner:list_objects_with_permission": 19, + "ownergroup:api_list_objects": 9, + "ownergroup:list_objects_with_permission": 17, + "token:api_list_objects": 10, + "token:list_objects_with_permission": 17, + "user:api_list_objects": 12, + "user:list_objects_with_permission": 17 +} diff --git a/netbox/utilities/testing/__init__.py b/netbox/utilities/testing/__init__.py index ebb4f83e6..e51b0f8bc 100644 --- a/netbox/utilities/testing/__init__.py +++ b/netbox/utilities/testing/__init__.py @@ -1,6 +1,7 @@ from .api import * from .base import * from .filtersets import * +from .query_counts import * from .tables import * from .utils import * from .views import * diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py index d058a3836..15383b415 100644 --- a/netbox/utilities/testing/api.py +++ b/netbox/utilities/testing/api.py @@ -22,6 +22,7 @@ from users.models import ObjectPermission, Token, User from utilities.api import get_graphql_type_for_model from .base import ModelTestCase +from .query_counts import assert_expected_query_count from .utils import disable_logging, disable_warnings, get_random_string __all__ = ( @@ -193,7 +194,8 @@ class APIViewTestCases: obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) # Try GET to permitted objects - response = self.client.get(self._get_list_url(), **self.header) + with assert_expected_query_count(self, 'api_list_objects'): + response = self.client.get(self._get_list_url(), **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(len(response.data['results']), 2) diff --git a/netbox/utilities/testing/query_counts.py b/netbox/utilities/testing/query_counts.py new file mode 100644 index 000000000..fcd1e8920 --- /dev/null +++ b/netbox/utilities/testing/query_counts.py @@ -0,0 +1,134 @@ +import json +import os +import sys +import threading +from contextlib import contextmanager +from pathlib import Path + +try: + import fcntl +except ImportError: + fcntl = None + +from django.apps import apps as django_apps +from django.db import connection +from django.test.utils import CaptureQueriesContext + +__all__ = ( + 'assert_expected_query_count', +) + +UPDATE_ENV_VAR = 'UPDATE_QUERY_COUNTS' +BASELINE_FILENAME = 'query_counts.json' + +_loaded_baselines = {} +_lock = threading.Lock() + + +def _is_update_mode(): + return bool(os.environ.get(UPDATE_ENV_VAR)) + + +def _is_parallel_test_run(): + # Heuristic: inspects sys.argv for Django's --parallel flag. This is + # sufficient for the project's standard `manage.py test` invocations but + # will not detect parallelism introduced by other test runners. + for arg in sys.argv: + if arg == '--parallel' or arg.startswith('--parallel='): + return True + return False + + +def _baseline_path(app_label): + app_config = django_apps.get_app_config(app_label) + return Path(app_config.path) / 'tests' / BASELINE_FILENAME + + +def _load_baseline(app_label): + with _lock: + if app_label in _loaded_baselines: + return _loaded_baselines[app_label] + path = _baseline_path(app_label) + if path.exists(): + with path.open() as f: + data = json.load(f) + else: + data = {} + _loaded_baselines[app_label] = data + return data + + +def _record_update(app_label, key, count): + # Write the baseline file synchronously rather than buffering until process + # exit, so updates are not lost if the runner terminates via os._exit() or + # a signal. An OS-level exclusive lock (where available) protects against + # concurrent processes — e.g. two simultaneous update-mode invocations — + # clobbering one another's writes. + with _lock: + path = _baseline_path(app_label) + path.parent.mkdir(parents=True, exist_ok=True) + with path.open('a+') as f: + if fcntl is not None: + fcntl.flock(f.fileno(), fcntl.LOCK_EX) + f.seek(0) + content = f.read() + existing = json.loads(content) if content else {} + existing[key] = count + f.seek(0) + f.truncate() + json.dump(existing, f, indent=2, sort_keys=True) + f.write('\n') + + +@contextmanager +def assert_expected_query_count(test_case, name): + """ + Assert that the wrapped block performs the number of SQL queries recorded + in the per-app baseline file (`/tests/query_counts.json`). + + The baseline key is `:`, derived from `test_case.model`. + + When the `UPDATE_QUERY_COUNTS` environment variable is set, the assertion + is skipped and the observed count is written back to the baseline file + immediately. Update mode requires serial test execution (no --parallel). + """ + model = test_case.model + app_label = model._meta.app_label + model_name = model._meta.model_name + key = f'{model_name}:{name}' + + if _is_update_mode(): + if _is_parallel_test_run(): + raise RuntimeError( + f"{UPDATE_ENV_VAR}=1 cannot be combined with --parallel; " + f"re-run serially to regenerate query-count baselines." + ) + ctx = CaptureQueriesContext(connection) + with ctx: + yield + _record_update(app_label, key, len(ctx.captured_queries)) + return + + baseline = _load_baseline(app_label) + if key not in baseline: + test_case.fail( + f"No query-count baseline recorded for {app_label}/{key}. " + f"Re-run with {UPDATE_ENV_VAR}=1 (serially) to record it." + ) + + expected = baseline[key] + ctx = CaptureQueriesContext(connection) + with ctx: + yield + actual = len(ctx.captured_queries) + if actual != expected: + sample = '\n'.join( + f" {i + 1}. {q['sql'][:240]}" + for i, q in enumerate(ctx.captured_queries) + ) + test_case.fail( + f"Query count for {app_label}/{key} changed: " + f"expected {expected}, got {actual}. " + f"If this change is intentional, re-run with {UPDATE_ENV_VAR}=1 " + f"to update the baseline.\nObserved queries:\n{sample}" + ) diff --git a/netbox/utilities/testing/views.py b/netbox/utilities/testing/views.py index f0aa24598..ac24c61d5 100644 --- a/netbox/utilities/testing/views.py +++ b/netbox/utilities/testing/views.py @@ -15,6 +15,7 @@ from netbox.models.features import ChangeLoggingMixin, CustomFieldsMixin from users.models import ObjectPermission from .base import ModelTestCase +from .query_counts import assert_expected_query_count from .utils import add_custom_field_data, disable_warnings, get_random_string, post_data __all__ = ( @@ -470,7 +471,9 @@ class ViewTestCases: obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model)) # Try GET with model-level permission - self.assertHttpStatus(self.client.get(self._get_url('list')), 200) + with assert_expected_query_count(self, 'list_objects_with_permission'): + response = self.client.get(self._get_url('list')) + self.assertHttpStatus(response, 200) def test_list_objects_with_constrained_permission(self): instance1, instance2 = self._get_queryset().all()[:2] diff --git a/netbox/virtualization/tests/query_counts.json b/netbox/virtualization/tests/query_counts.json new file mode 100644 index 000000000..3c65f818d --- /dev/null +++ b/netbox/virtualization/tests/query_counts.json @@ -0,0 +1,16 @@ +{ + "cluster:api_list_objects": 16, + "cluster:list_objects_with_permission": 22, + "clustergroup:api_list_objects": 13, + "clustergroup:list_objects_with_permission": 20, + "clustertype:api_list_objects": 13, + "clustertype:list_objects_with_permission": 20, + "virtualdisk:api_list_objects": 14, + "virtualdisk:list_objects_with_permission": 21, + "virtualmachine:api_list_objects": 18, + "virtualmachine:list_objects_with_permission": 24, + "virtualmachinetype:api_list_objects": 14, + "virtualmachinetype:list_objects_with_permission": 21, + "vminterface:api_list_objects": 19, + "vminterface:list_objects_with_permission": 21 +} diff --git a/netbox/vpn/tests/query_counts.json b/netbox/vpn/tests/query_counts.json new file mode 100644 index 000000000..7c5b85216 --- /dev/null +++ b/netbox/vpn/tests/query_counts.json @@ -0,0 +1,22 @@ +{ + "ikepolicy:api_list_objects": 14, + "ikepolicy:list_objects_with_permission": 21, + "ikeproposal:api_list_objects": 13, + "ikeproposal:list_objects_with_permission": 20, + "ipsecpolicy:api_list_objects": 14, + "ipsecpolicy:list_objects_with_permission": 21, + "ipsecprofile:api_list_objects": 15, + "ipsecprofile:list_objects_with_permission": 22, + "ipsecproposal:api_list_objects": 13, + "ipsecproposal:list_objects_with_permission": 20, + "l2vpn:api_list_objects": 15, + "l2vpn:list_objects_with_permission": 20, + "l2vpntermination:api_list_objects": 16, + "l2vpntermination:list_objects_with_permission": 25, + "tunnel:api_list_objects": 14, + "tunnel:list_objects_with_permission": 21, + "tunnelgroup:api_list_objects": 13, + "tunnelgroup:list_objects_with_permission": 20, + "tunneltermination:api_list_objects": 18, + "tunneltermination:list_objects_with_permission": 28 +} diff --git a/netbox/wireless/tests/query_counts.json b/netbox/wireless/tests/query_counts.json new file mode 100644 index 000000000..af3619eee --- /dev/null +++ b/netbox/wireless/tests/query_counts.json @@ -0,0 +1,8 @@ +{ + "wirelesslan:api_list_objects": 13, + "wirelesslan:list_objects_with_permission": 21, + "wirelesslangroup:api_list_objects": 13, + "wirelesslangroup:list_objects_with_permission": 21, + "wirelesslink:api_list_objects": 18, + "wirelesslink:list_objects_with_permission": 24 +}