Przeglądaj źródła

test(models): Detect missing model test coverage (#22254)

Martin Hauser 21 godzin temu
rodzic
commit
02746d7daa

+ 47 - 0
netbox/core/tests/test_api.py

@@ -1,5 +1,6 @@
 import uuid
 
+from django.contrib.contenttypes.models import ContentType
 from django.urls import reverse
 from django.utils import timezone
 from django_rq import get_queue
@@ -106,6 +107,7 @@ class DataFileTestCase(
 
 
 class ObjectTypeTestCase(APITestCase):
+    model = ObjectType
 
     def test_list_objects(self):
         object_type_count = ObjectType.objects.count()
@@ -121,6 +123,51 @@ class ObjectTypeTestCase(APITestCase):
         self.assertHttpStatus(self.client.get(url, **self.header), status.HTTP_200_OK)
 
 
+class JobTestCase(
+    APIViewTestCases.GetObjectViewTestCase,
+    APIViewTestCases.ListObjectsViewTestCase,
+):
+    model = Job
+    brief_fields = ['completed', 'created', 'status', 'url', 'user']
+
+    @classmethod
+    def setUpTestData(cls):
+        datasource = DataSource.objects.create(
+            name='Data Source 1',
+            type='local',
+            source_url='file:///var/tmp/source1/',
+        )
+        ct = ContentType.objects.get_for_model(DataSource)
+        Job.objects.bulk_create(
+            [
+                Job(
+                    name='Job 1',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='pending',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+                Job(
+                    name='Job 2',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='running',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+                Job(
+                    name='Job 3',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='completed',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+            ]
+        )
+
+
 class BackgroundTaskTestCase(TestCase):
     user_permissions = ()
 

+ 47 - 0
netbox/core/tests/test_views.py

@@ -3,6 +3,7 @@ import urllib.parse
 import uuid
 from datetime import datetime
 
+from django.contrib.contenttypes.models import ContentType
 from django.urls import reverse
 from django.utils import timezone
 from django_rq import get_queue
@@ -104,6 +105,52 @@ class DataFileTestCase(
         DataFile.objects.bulk_create(data_files)
 
 
+class JobTestCase(
+    ViewTestCases.GetObjectViewTestCase,
+    ViewTestCases.ListObjectsViewTestCase,
+    ViewTestCases.DeleteObjectViewTestCase,
+    ViewTestCases.BulkDeleteObjectsViewTestCase,
+):
+    model = Job
+
+    @classmethod
+    def setUpTestData(cls):
+        datasource = DataSource.objects.create(
+            name='Data Source 1',
+            type='local',
+            source_url='file:///var/tmp/source1/',
+        )
+        ct = ContentType.objects.get_for_model(DataSource)
+        Job.objects.bulk_create(
+            [
+                Job(
+                    name='Job 1',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='pending',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+                Job(
+                    name='Job 2',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='running',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+                Job(
+                    name='Job 3',
+                    object_type=ct,
+                    object_id=datasource.pk,
+                    status='completed',
+                    queue_name='default',
+                    job_id=uuid.uuid4(),
+                ),
+            ]
+        )
+
+
 # TODO: Convert to StandardTestCases.Views
 class ObjectChangeTestCase(TestCase):
     user_permissions = (

+ 72 - 0
netbox/extras/tests/test_api.py

@@ -19,6 +19,7 @@ from extras.scripts import BooleanVar, IntegerVar, StringVar
 from extras.scripts import Script as PythonClass
 from users.constants import TOKEN_PREFIX
 from users.models import Group, Token, User
+from utilities.tables import get_table_for_model
 from utilities.testing import APITestCase, APIViewTestCases
 
 
@@ -458,6 +459,77 @@ class SavedFilterTestCase(APIViewTestCases.APIViewTestCase):
             savedfilter.object_types.set([site_type])
 
 
+class TableConfigTestCase(APIViewTestCases.APIViewTestCase):
+    model = TableConfig
+    brief_fields = ['description', 'display', 'id', 'name', 'object_type', 'table', 'url']
+    bulk_update_data = {
+        'description': 'New description',
+        'weight': 999,
+        'enabled': False,
+        'shared': False,
+    }
+
+    @classmethod
+    def setUpTestData(cls):
+        site_type = ObjectType.objects.get_for_model(Site)
+        site_table_name = get_table_for_model(Site).__name__
+
+        users = (
+            User(username='User 1'),
+            User(username='User 2'),
+            User(username='User 3'),
+        )
+        User.objects.bulk_create(users)
+
+        table_configs = (
+            TableConfig(
+                name='Table Config 1',
+                object_type=site_type,
+                table=site_table_name,
+                user=users[0],
+                columns=['name', 'status'],
+            ),
+            TableConfig(
+                name='Table Config 2',
+                object_type=site_type,
+                table=site_table_name,
+                user=users[1],
+                columns=['name', 'region'],
+            ),
+            TableConfig(
+                name='Table Config 3',
+                object_type=site_type,
+                table=site_table_name,
+                user=users[2],
+                columns=['name', 'tenant'],
+            ),
+        )
+        TableConfig.objects.bulk_create(table_configs)
+
+        cls.create_data = [
+            {
+                'object_type': 'dcim.site',
+                'table': site_table_name,
+                'name': 'Table Config 4',
+                'columns': ['name', 'status'],
+                'ordering': ['name'],
+            },
+            {
+                'object_type': 'dcim.site',
+                'table': site_table_name,
+                'name': 'Table Config 5',
+                'columns': ['name', 'region'],
+                'ordering': ['-name'],
+            },
+            {
+                'object_type': 'dcim.site',
+                'table': site_table_name,
+                'name': 'Table Config 6',
+                'columns': ['name', 'tenant'],
+            },
+        ]
+
+
 class BookmarkTestCase(
     APIViewTestCases.GetObjectViewTestCase,
     APIViewTestCases.ListObjectsViewTestCase,

+ 56 - 0
netbox/extras/tests/test_filtersets.py

@@ -790,6 +790,62 @@ class ImageAttachmentTestCase(TestCase, ChangeLoggedFilterSetTests):
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1)
 
 
+class TableConfigTestCase(TestCase, ChangeLoggedFilterSetTests):
+    queryset = TableConfig.objects.all()
+    filterset = TableConfigFilterSet
+    ignore_fields = ('columns', 'ordering')
+
+    @classmethod
+    def setUpTestData(cls):
+        site_ct = ContentType.objects.get_by_natural_key('dcim', 'site')
+        rack_ct = ContentType.objects.get_by_natural_key('dcim', 'rack')
+
+        users = (
+            User(username='user1'),
+            User(username='user2'),
+            User(username='user3'),
+        )
+        User.objects.bulk_create(users)
+
+        TableConfig.objects.bulk_create(
+            [
+                TableConfig(
+                    object_type=site_ct,
+                    table='SiteTable',
+                    name='Table Config 1',
+                    user=users[0],
+                    weight=100,
+                    enabled=True,
+                    shared=True,
+                    columns=['name', 'status'],
+                    ordering=[],
+                ),
+                TableConfig(
+                    object_type=site_ct,
+                    table='SiteTable',
+                    name='Table Config 2',
+                    user=users[1],
+                    weight=200,
+                    enabled=True,
+                    shared=False,
+                    columns=['name', 'region'],
+                    ordering=[],
+                ),
+                TableConfig(
+                    object_type=rack_ct,
+                    table='RackTable',
+                    name='Table Config 3',
+                    user=users[2],
+                    weight=300,
+                    enabled=False,
+                    shared=True,
+                    columns=['name', 'site'],
+                    ordering=[],
+                ),
+            ]
+        )
+
+
 class JournalEntryTestCase(TestCase, ChangeLoggedFilterSetTests):
     queryset = JournalEntry.objects.all()
     filterset = JournalEntryFilterSet

+ 93 - 0
netbox/extras/tests/test_views.py

@@ -273,6 +273,53 @@ class SavedFilterTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         }
 
 
+class TableConfigTestCase(
+    ViewTestCases.GetObjectViewTestCase,
+    ViewTestCases.GetObjectChangelogViewTestCase,
+    ViewTestCases.ListObjectsViewTestCase,
+    ViewTestCases.DeleteObjectViewTestCase,
+    ViewTestCases.BulkDeleteObjectsViewTestCase,
+):
+    # Add/Edit/BulkEdit views require an object_type pre-context from the source
+    # table view, so they are not exercised here.
+    model = TableConfig
+
+    @classmethod
+    def setUpTestData(cls):
+        site_type = ObjectType.objects.get_for_model(Site)
+        users = (
+            User(username='User 1'),
+            User(username='User 2'),
+            User(username='User 3'),
+        )
+        User.objects.bulk_create(users)
+
+        table_configs = (
+            TableConfig(
+                name='Table Config 1',
+                object_type=site_type,
+                table='SiteTable',
+                user=users[0],
+                columns=['name', 'status'],
+            ),
+            TableConfig(
+                name='Table Config 2',
+                object_type=site_type,
+                table='SiteTable',
+                user=users[1],
+                columns=['name', 'region'],
+            ),
+            TableConfig(
+                name='Table Config 3',
+                object_type=site_type,
+                table='SiteTable',
+                user=users[2],
+                columns=['name', 'tenant'],
+            ),
+        )
+        TableConfig.objects.bulk_create(table_configs)
+
+
 class BookmarkTestCase(
     ViewTestCases.DeleteObjectViewTestCase,
     ViewTestCases.ListObjectsViewTestCase,
@@ -321,6 +368,52 @@ class BookmarkTestCase(
         return
 
 
+class ImageAttachmentTestCase(
+    ViewTestCases.GetObjectViewTestCase,
+    ViewTestCases.ListObjectsViewTestCase,
+    ViewTestCases.DeleteObjectViewTestCase,
+    ViewTestCases.BulkDeleteObjectsViewTestCase,
+):
+    # Add/Edit/BulkEdit are omitted: ImageField.save() re-reads the file to
+    # populate image_height / image_width, which fails when fixtures use
+    # placeholder URLs instead of real images on disk.
+    model = ImageAttachment
+
+    @classmethod
+    def setUpTestData(cls):
+        ct = ContentType.objects.get_for_model(Site)
+        site = Site.objects.create(name='Site 1', slug='site-1')
+
+        ImageAttachment.objects.bulk_create(
+            [
+                ImageAttachment(
+                    object_type=ct,
+                    object_id=site.pk,
+                    name='Image Attachment 1',
+                    image='http://example.com/image1.png',
+                    image_height=100,
+                    image_width=100,
+                ),
+                ImageAttachment(
+                    object_type=ct,
+                    object_id=site.pk,
+                    name='Image Attachment 2',
+                    image='http://example.com/image2.png',
+                    image_height=100,
+                    image_width=100,
+                ),
+                ImageAttachment(
+                    object_type=ct,
+                    object_id=site.pk,
+                    name='Image Attachment 3',
+                    image='http://example.com/image3.png',
+                    image_height=100,
+                    image_width=100,
+                ),
+            ]
+        )
+
+
 class ExportTemplateTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     model = ExportTemplate
 

+ 300 - 0
netbox/netbox/tests/test_model_test_coverage.py

@@ -0,0 +1,300 @@
+import inspect
+from importlib import import_module
+from importlib.util import find_spec
+from unittest.mock import MagicMock, patch
+
+from django.test import TestCase
+from django.urls import NoReverseMatch
+
+from core.filtersets import ObjectTypeFilterSet
+from core.models import ObjectType
+from netbox.registry import registry
+from utilities.testing import APITestCase, BaseFilterSetTests, ModelViewTestCase
+from utilities.views import get_action_url
+
+
+def intentional(reason):
+    return f'Intentional non-standard model coverage: {reason}'
+
+
+EXEMPTIONS = {
+    'api': {
+        'core.objectchange': intentional(
+            'Read-only audit log; changelog behavior is primarily tested through '
+            'parent objects rather than standalone CRUD-style model tests.'
+        ),
+        'extras.script': intentional(
+            'Database-backed script definitions expose bespoke run/admin behavior rather than standard CRUD semantics.'
+        ),
+    },
+    'views': {
+        'core.objectchange': intentional(
+            'Read-only audit log; changelog behavior is tested through parent object '
+            'views rather than a standalone ModelViewTestCase.'
+        ),
+        'core.configrevision': intentional('Read-only configuration audit/history model, not a standard CRUD viewset.'),
+        'extras.script': intentional(
+            'Database-backed script definitions use bespoke script admin/run views '
+            'rather than the standard CRUD view pattern.'
+        ),
+    },
+    'filtersets': {
+        'core.configrevision': intentional(
+            'Read-only configuration audit/history model, not standard filterset test coverage.'
+        ),
+        'extras.script': intentional(
+            'Database-backed script definitions use bespoke behavior rather than standard model filtering workflows.'
+        ),
+    },
+}
+
+
+def model_label(model):
+    return f'{model._meta.app_label}.{model._meta.model_name}'
+
+
+def import_optional(module_name):
+    """
+    Import an optional submodule if it exists.
+
+    Returns None when the module (or its parent package) cannot be located.
+    Import errors raised from inside an existing module are propagated so
+    that real bugs do not get silently swallowed.
+    """
+    try:
+        spec = find_spec(module_name)
+    except ModuleNotFoundError as err:
+        if module_name == err.name or module_name.startswith(f'{err.name}.'):
+            return None
+        raise
+
+    if spec is None:
+        return None
+
+    return import_module(module_name)
+
+
+def has_tests(cls):
+    return any(name.startswith('test_') and callable(getattr(cls, name)) for name in dir(cls))
+
+
+def get_queryset_model(cls):
+    queryset = getattr(cls, 'queryset', None)
+    if queryset is not None:
+        return queryset.model
+
+    filterset = getattr(cls, 'filterset', None)
+    if filterset is not None:
+        return filterset._meta.model
+
+    return None
+
+
+class ModelTestCoverageTestCase(TestCase):
+    def get_public_models(self):
+        models = []
+
+        for object_type in ObjectType.objects.public().order_by('app_label', 'model'):
+            if object_type.is_plugin_model:
+                continue
+
+            model = object_type.model_class()
+            if model is not None:
+                models.append(model)
+
+        return models
+
+    def get_app_labels(self, models):
+        return sorted({model._meta.app_label for model in models})
+
+    def collect_covered_models(self, app_labels, module_suffix, base_class, get_model):
+        covered = {}
+
+        for app_label in app_labels:
+            module = import_optional(f'{app_label}.tests.{module_suffix}')
+            if module is None:
+                continue
+
+            for _, cls in inspect.getmembers(module, inspect.isclass):
+                if cls.__module__ != module.__name__:
+                    continue
+                if not issubclass(cls, base_class):
+                    continue
+                if not has_tests(cls):
+                    continue
+
+                model = get_model(cls)
+                if model is not None:
+                    covered.setdefault(model, []).append(f'{cls.__module__}.{cls.__qualname__}')
+
+        return covered
+
+    def has_action_url(self, model, action='list', rest_api=False):
+        try:
+            get_action_url(model, action=action, rest_api=rest_api)
+        except NoReverseMatch:
+            return False
+
+        return True
+
+    def format_exemptions(self, category, labels):
+        return '\n'.join(f'  - {label}: {EXEMPTIONS[category][label]}' for label in labels)
+
+    def assert_coverage(self, category, expected, covered):
+        expected_labels = {model_label(model) for model in expected}
+        covered_labels = {model_label(model) for model in covered}
+        exempted_labels = set(EXEMPTIONS[category])
+
+        stale_exemptions = sorted(exempted_labels - expected_labels)
+        self.assertFalse(
+            stale_exemptions,
+            f'Remove stale {category} coverage exemptions; these models are '
+            f'no longer expected in this category:\n' + self.format_exemptions(category, stale_exemptions),
+        )
+
+        covered_exemptions = sorted(exempted_labels & covered_labels)
+        self.assertFalse(
+            covered_exemptions,
+            f'Remove stale {category} coverage exemptions; these models now '
+            f'have standard test coverage:\n' + self.format_exemptions(category, covered_exemptions),
+        )
+
+        missing = sorted(expected_labels - covered_labels - exempted_labels)
+        self.assertFalse(
+            missing,
+            f'Missing {category} test coverage for:\n' + '\n'.join(f'  - {label}' for label in missing),
+        )
+
+    def test_api_test_cases_exist_for_api_models(self):
+        models = self.get_public_models()
+        app_labels = self.get_app_labels(models)
+
+        expected = {model for model in models if self.has_action_url(model, action='list', rest_api=True)}
+
+        covered = self.collect_covered_models(
+            app_labels,
+            'test_api',
+            APITestCase,
+            lambda cls: getattr(cls, 'model', None),
+        )
+
+        self.assert_coverage('api', expected, covered)
+
+    def test_view_test_cases_exist_for_ui_models(self):
+        models = self.get_public_models()
+        app_labels = self.get_app_labels(models)
+
+        expected = {model for model in models if self.has_action_url(model, action='list')}
+
+        covered = self.collect_covered_models(
+            app_labels,
+            'test_views',
+            ModelViewTestCase,
+            lambda cls: getattr(cls, 'model', None),
+        )
+
+        self.assert_coverage('views', expected, covered)
+
+    def test_filterset_test_cases_exist_for_registered_filtersets(self):
+        models = self.get_public_models()
+        app_labels = self.get_app_labels(models)
+
+        # Defensive import: app filtersets are normally imported during app/test
+        # setup, but loading them here keeps the test self-contained.
+        for app_label in app_labels:
+            import_optional(f'{app_label}.filtersets')
+
+        expected = {model for model in models if model_label(model) in registry['filtersets']}
+
+        covered = self.collect_covered_models(
+            app_labels,
+            'test_filtersets',
+            BaseFilterSetTests,
+            get_queryset_model,
+        )
+
+        self.assert_coverage('filtersets', expected, covered)
+
+
+class HelperFunctionTestCase(TestCase):
+    """
+    Exercise the helper functions directly so their defensive branches are
+    covered even when the main meta-test doesn't trigger them organically.
+    """
+
+    def test_import_optional_returns_module_when_present(self):
+        module = import_optional('netbox.tests.test_model_test_coverage')
+        self.assertIsNotNone(module)
+        self.assertEqual(module.__name__, 'netbox.tests.test_model_test_coverage')
+
+    def test_import_optional_returns_none_for_missing_submodule(self):
+        # Parent package exists, leaf does not -> find_spec returns None.
+        self.assertIsNone(import_optional('netbox.tests.does_not_exist'))
+
+    def test_import_optional_returns_none_for_missing_parent(self):
+        # Parent package itself does not exist -> ModuleNotFoundError swallowed.
+        self.assertIsNone(import_optional('nonexistent_pkg.tests.test_api'))
+
+    def test_import_optional_propagates_unrelated_find_spec_error(self):
+        # find_spec raising ModuleNotFoundError for a different module than the
+        # one requested means a parent package itself failed to import; that
+        # error must propagate so real bugs aren't silently swallowed.
+        with patch(
+            'netbox.tests.test_model_test_coverage.find_spec',
+            side_effect=ModuleNotFoundError(
+                "No module named 'unrelated_dependency'",
+                name='unrelated_dependency',
+            ),
+        ):
+            with self.assertRaises(ModuleNotFoundError):
+                import_optional('netbox.tests.test_model_test_coverage')
+
+    def test_import_optional_propagates_import_module_errors(self):
+        # If find_spec succeeds but import_module raises (e.g. the module
+        # exists on disk but a dependency inside it fails to import), the
+        # error must propagate rather than being treated as "module missing."
+        with (
+            patch(
+                'netbox.tests.test_model_test_coverage.find_spec',
+                return_value=MagicMock(),
+            ),
+            patch(
+                'netbox.tests.test_model_test_coverage.import_module',
+                side_effect=ModuleNotFoundError(
+                    "No module named 'unrelated_dependency'",
+                    name='unrelated_dependency',
+                ),
+            ),
+        ):
+            with self.assertRaises(ModuleNotFoundError):
+                import_optional('netbox.tests.test_model_test_coverage')
+
+    def test_has_tests_detects_test_methods(self):
+        class WithTest:
+            def test_thing(self):
+                pass
+
+        class WithoutTest:
+            def helper(self):
+                pass
+
+        self.assertTrue(has_tests(WithTest))
+        self.assertFalse(has_tests(WithoutTest))
+
+    def test_get_queryset_model_prefers_queryset(self):
+        class Sample:
+            queryset = ObjectType.objects.all()
+
+        self.assertEqual(get_queryset_model(Sample), ObjectType)
+
+    def test_get_queryset_model_falls_back_to_filterset(self):
+        class Sample:
+            filterset = ObjectTypeFilterSet
+
+        self.assertEqual(get_queryset_model(Sample), ObjectType)
+
+    def test_get_queryset_model_returns_none_when_unset(self):
+        class Sample:
+            pass
+
+        self.assertIsNone(get_queryset_model(Sample))