From 1a750a819fec96b9e8be890ae34ab70116d46bd1 Mon Sep 17 00:00:00 2001 From: Cameron Hobbs Date: Fri, 10 Jul 2026 12:34:28 +0100 Subject: [PATCH 1/3] Increase coverage and enforce minimum of 95% --- .github/workflows/ci.yml | 15 ++- eagle/instrumentation/query.py | 2 +- pyproject.toml | 2 +- tests/factories.py | 9 +- tests/test_apps_ready.py | 35 ++++++ tests/test_descriptors_internals.py | 150 ++++++++++++++++++++++ tests/test_instrumentation_models.py | 56 +++++++++ tests/test_instrumentation_registry.py | 15 +++ tests/test_logger.py | 24 ++++ tests/test_project/models.py | 13 ++ tests/test_query_internals.py | 166 +++++++++++++++++++++++++ tests/test_scope_ignore.py | 7 ++ tests/test_scope_internals.py | 25 ++++ tests/test_sinks_normalize_model.py | 13 ++ tests/test_unused.py | 28 ++++- tests/test_unused_location.py | 13 ++ tests/test_unused_marker_guards.py | 41 ++++++ tests/test_unused_tracker.py | 8 ++ 18 files changed, 615 insertions(+), 7 deletions(-) create mode 100644 tests/test_apps_ready.py create mode 100644 tests/test_descriptors_internals.py create mode 100644 tests/test_instrumentation_models.py create mode 100644 tests/test_instrumentation_registry.py create mode 100644 tests/test_logger.py create mode 100644 tests/test_query_internals.py create mode 100644 tests/test_scope_internals.py create mode 100644 tests/test_sinks_normalize_model.py create mode 100644 tests/test_unused_location.py create mode 100644 tests/test_unused_marker_guards.py create mode 100644 tests/test_unused_tracker.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7392d3a..ced9870 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,17 @@ jobs: uses: pre-commit/action@v3.0.1 - name: Test - run: uv run pytest -q --cov --cov-report=xml --cov-report=term-missing + # Invoked via `coverage run` (not `pytest --cov`) so tracing starts at interpreter + # startup: pytest's own config parsing eagerly imports the `filterwarnings` target + # ("error::eagle.exceptions.UnusedRelatedAccess") before pytest-cov would attach, + # which under `pytest --cov` silently drops that early import from measurement. + run: uv run coverage run -m pytest -q + + - name: Coverage report + run: uv run coverage report -m + + - name: Coverage XML + run: uv run coverage xml - name: Generate coverage badge run: uv run genbadge coverage -i coverage.xml -o assets/coverage.svg @@ -47,3 +57,6 @@ jobs: git add assets/coverage.svg git diff --staged --quiet || git commit -m "chore: update coverage badge" git push + + - name: Enforce minimum coverage + run: uv run coverage report --fail-under=95 diff --git a/eagle/instrumentation/query.py b/eagle/instrumentation/query.py index a764191..f54da62 100644 --- a/eagle/instrumentation/query.py +++ b/eagle/instrumentation/query.py @@ -109,7 +109,7 @@ def propagate_prefetch_location(instances: list[models.Model], child_queryset: A child_queryset: The prefetch queryset that will execute against the related model. cache_name: ORM cache key identifying which per-field location to look up; None skips propagation. """ - if cache_name is None or not instances or child_queryset is None: + if cache_name is None or not instances or not isinstance(child_queryset, QuerySet): return if getattr(child_queryset, _EAGLE_LOCATION_ATTR, None) is not None: return diff --git a/pyproject.toml b/pyproject.toml index 4043aba..55046ae 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,11 +59,11 @@ Issues = "https://github.com/xelixdev/django-eagle/issues" [dependency-groups] dev = [ "djangorestframework>=3.16.0", + "coverage>=7.6.0", "factory-boy>=3.3.0", "genbadge[coverage]>=1.1.3", "pyrefly>=1.0.0", "pytest>=9.0.3", - "pytest-cov>=5.0.0", "pytest-django>=4.9.0", ] diff --git a/tests/factories.py b/tests/factories.py index 2a7614e..9bc797e 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -4,7 +4,7 @@ from collision_app.models import Eagle as CollisionEagle from excluded_app.models import Burrow -from test_project.models import Climate, Eagle, Eaglet, Location +from test_project.models import Aerie, Climate, Eagle, Eaglet, Location class ClimateFactory(factory.django.DjangoModelFactory): @@ -45,6 +45,13 @@ class Meta: eagle = factory.SubFactory(EagleFactory) +class AerieFactory(factory.django.DjangoModelFactory): + class Meta: + model = Aerie + + eagle = factory.SubFactory(EagleFactory) + + class BurrowFactory(factory.django.DjangoModelFactory): class Meta: model = Burrow diff --git a/tests/test_apps_ready.py b/tests/test_apps_ready.py new file mode 100644 index 0000000..e633275 --- /dev/null +++ b/tests/test_apps_ready.py @@ -0,0 +1,35 @@ +from django.apps import apps as django_apps +from django.test import override_settings + +import eagle.instrumentation as instrumentation + + +class TestEagleAppConfigReady: + def test_ready_is_noop_when_disabled(self, monkeypatch: object) -> None: + calls: list[str] = [] + monkeypatch.setattr(instrumentation, "patch_orm", lambda: calls.append("patch_orm")) + monkeypatch.setattr(instrumentation, "register_tracked_models", lambda models: calls.append("register")) + + with override_settings(EAGLE_ENABLED=False): + django_apps.get_app_config("eagle").ready() + + assert calls == [] + + def test_ready_skips_contenttypes_eager_when_app_config_missing(self, monkeypatch: object) -> None: + calls: list[str] = [] + monkeypatch.setattr(instrumentation, "make_contenttypes_eager", lambda: calls.append("contenttypes")) + + original_get_app_config = django_apps.get_app_config + + def fake_get_app_config(label: str) -> object: + if label == "contenttypes": + msg = "contenttypes" + raise LookupError(msg) + return original_get_app_config(label) + + eagle_config = original_get_app_config("eagle") + monkeypatch.setattr(django_apps, "get_app_config", fake_get_app_config) + + eagle_config.ready() + + assert calls == [] diff --git a/tests/test_descriptors_internals.py b/tests/test_descriptors_internals.py new file mode 100644 index 0000000..17e10c3 --- /dev/null +++ b/tests/test_descriptors_internals.py @@ -0,0 +1,150 @@ +from types import SimpleNamespace + +import pytest +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType + +from eagle import UnusedRelatedAccess, unused, warn_unused +from eagle.instrumentation import descriptors +from test_project.models import Aerie, Eagle, Sighting +from tests.factories import AerieFactory, EagleFactory + + +class TestEagerPrefetchMixinBase: + def test_default_prefetch_cache_name_is_none(self) -> None: + assert descriptors.EagerPrefetchMixin()._prefetch_cache_name() is None + + +class TestLegacyGetPrefetchQueryset: + def test_delegates_to_super_and_returns_its_result(self) -> None: + class _LegacyBase: + def get_prefetch_queryset(self, instances: list, queryset: object = None) -> tuple: + return ("child-queryset", queryset) + + class _LegacyManager(descriptors.EagerPrefetchMixin, _LegacyBase): + def _prefetch_cache_name(self) -> str: + return "legacy_cache" + + result = _LegacyManager().get_prefetch_queryset([]) + + assert result == ("child-queryset", None) + + +@pytest.mark.django_db +class TestTrackingPrefetchCache: + def test_getitem_marks_consumed_when_instance_is_tracked(self) -> None: + eagle = EagleFactory() + unused.begin_request() + unused.init_state(eagle, location=None) + unused.mark_prefetched([eagle], "previous_locations") + cache = descriptors.TrackingPrefetchCache({"previous_locations": []}, _eagle_instance=eagle) + + assert cache["previous_locations"] == [] + + unused.end_request() + + def test_getitem_skips_marking_when_instance_untracked(self) -> None: + cache = descriptors.TrackingPrefetchCache({"key": "value"}, _eagle_instance=None) + assert cache["key"] == "value" + + +@pytest.mark.django_db +class TestPrefetchCacheDescriptor: + def test_class_level_access_returns_descriptor_itself(self) -> None: + descriptor = Eagle.__dict__["_prefetched_objects_cache"] + assert descriptor.__get__(None, Eagle) is descriptor + + def test_set_does_not_rewrap_existing_tracking_cache(self) -> None: + descriptor = Eagle.__dict__["_prefetched_objects_cache"] + eagle = EagleFactory() + existing = descriptors.TrackingPrefetchCache({}, _eagle_instance=eagle) + + descriptor.__set__(eagle, existing) + + assert eagle.__dict__[descriptor._eagle_storage] is existing + + def test_delete_missing_cache_raises_attribute_error(self) -> None: + descriptor = Eagle.__dict__["_prefetched_objects_cache"] + eagle = EagleFactory() + with pytest.raises(AttributeError): + descriptor.__delete__(eagle) + + def test_delete_existing_cache_removes_it(self) -> None: + descriptor = Eagle.__dict__["_prefetched_objects_cache"] + eagle = EagleFactory() + descriptor.__set__(eagle, {}) + + descriptor.__delete__(eagle) + + assert descriptor._eagle_storage not in eagle.__dict__ + + +class TestCreateEagerRelatedManagerCacheNameFallback: + def test_falls_back_to_related_query_name_without_prefetch_cache_name(self) -> None: + class _StubRelatedManager: + field = SimpleNamespace(related_query_name=lambda: "stub_query_name") + + manager_cls = descriptors.create_eager_related_manager(_StubRelatedManager) + + assert manager_cls()._prefetch_cache_name() == "stub_query_name" + + +@pytest.mark.django_db +class TestForwardOneToOnePrefetch: + def test_forward_o2o_prefetch_unused_warns(self) -> None: + eagle = EagleFactory() + aerie = AerieFactory(eagle=eagle) + with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): + Aerie.objects.prefetch_related("eagle").get(pk=aerie.pk) + assert 'prefetch_related("eagle")' in str(exc_info.value) + + def test_forward_o2o_prefetch_accessed_no_warning(self) -> None: + eagle = EagleFactory() + aerie = AerieFactory(eagle=eagle) + with warn_unused(): + fetched = Aerie.objects.prefetch_related("eagle").get(pk=aerie.pk) + assert fetched.eagle == eagle + + +class TestMakeContenttypesEagerIdempotent: + def test_second_call_does_not_duplicate_registration(self) -> None: + before = len(descriptors._eager_mixins) + descriptors.make_contenttypes_eager() + assert len(descriptors._eager_mixins) == before + + +class TestLegacyGenericForeignKeyGetPrefetchQueryset: + def test_delegates_to_super_and_returns_its_result(self) -> None: + gfk_mixin = next(mixin for stock_class, mixin in descriptors._eager_mixins if stock_class is GenericForeignKey) + + class _LegacyBase: + cache_name = "content_object" + + def get_prefetch_queryset(self, instances: list, queryset: object = None) -> tuple: + return ("child-queryset", queryset) + + class _LegacyGenericForeignKey(gfk_mixin, _LegacyBase): + pass + + result = _LegacyGenericForeignKey().get_prefetch_queryset([]) + + assert result == ("child-queryset", None) + + +@pytest.mark.django_db +class TestGenericForeignKeyTracking: + def _make_sighting(self) -> Sighting: + eagle = EagleFactory() + return Sighting.objects.create(content_type=ContentType.objects.get_for_model(Eagle), object_id=eagle.pk) + + def test_prefetched_generic_foreign_key_unused_warns(self) -> None: + sighting = self._make_sighting() + with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): + Sighting.objects.prefetch_related("content_object").get(pk=sighting.pk) + assert 'prefetch_related("content_object")' in str(exc_info.value) + + def test_prefetched_generic_foreign_key_accessed_no_warning(self) -> None: + sighting = self._make_sighting() + with warn_unused(): + fetched = Sighting.objects.prefetch_related("content_object").get(pk=sighting.pk) + assert fetched.content_object is not None diff --git a/tests/test_instrumentation_models.py b/tests/test_instrumentation_models.py new file mode 100644 index 0000000..f5ae7b2 --- /dev/null +++ b/tests/test_instrumentation_models.py @@ -0,0 +1,56 @@ +from unittest.mock import Mock + +from eagle.instrumentation import models +from test_project.models import Eagle + + +class TestMakeDescriptorEager: + def test_missing_attribute_is_a_noop(self) -> None: + models.make_descriptor_eager(object(), "does_not_exist") + + +class TestMakeRemoteFieldDescriptorEager: + def test_uninstrumented_related_model_is_skipped(self, monkeypatch: object) -> None: + calls: list[object] = [] + monkeypatch.setattr(models, "make_descriptor_eager", lambda *a: calls.append(a)) + + models.make_remote_field_descriptor_eager(None, object, remote_field=Mock()) + + assert calls == [] + + def test_hidden_accessor_is_skipped(self, monkeypatch: object) -> None: + calls: list[object] = [] + monkeypatch.setattr(models, "make_descriptor_eager", lambda *a: calls.append(a)) + remote_field = Mock(get_accessor_name=Mock(return_value=None)) + + models.make_remote_field_descriptor_eager(None, Eagle, remote_field=remote_field) + + remote_field.get_accessor_name.assert_called_once() + assert calls == [] + + def test_visible_accessor_is_made_eager(self, monkeypatch: object) -> None: + calls: list[object] = [] + monkeypatch.setattr(models, "make_descriptor_eager", lambda *a: calls.append(a)) + remote_field = Mock(get_accessor_name=Mock(return_value="visitors")) + + models.make_remote_field_descriptor_eager(None, Eagle, remote_field=remote_field) + + assert calls == [(Eagle, "visitors")] + + +class TestMakePrefetchCacheEager: + def test_second_call_is_idempotent(self) -> None: + models.make_prefetch_cache_eager(Eagle) + first_descriptor = Eagle.__dict__["_prefetched_objects_cache"] + + models.make_prefetch_cache_eager(Eagle) + + assert Eagle.__dict__["_prefetched_objects_cache"] is first_descriptor + + +class TestMakeModelEagerHiddenRelation: + def test_hidden_reverse_relation_is_skipped_without_error(self) -> None: + mates_rel = next(rel for rel in Eagle._meta.related_objects if rel.get_accessor_name() is None) + assert mates_rel.field.name == "mates" + + models.make_model_eager(Eagle) diff --git a/tests/test_instrumentation_registry.py b/tests/test_instrumentation_registry.py new file mode 100644 index 0000000..9c0b34e --- /dev/null +++ b/tests/test_instrumentation_registry.py @@ -0,0 +1,15 @@ +from eagle.instrumentation import registry +from test_project.models import Eagle + + +class TestRegistryClear: + def test_clear_removes_all_registered_models(self) -> None: + original = set(registry._instrumented) + try: + assert registry.is_instrumented(Eagle) is True + registry.clear() + assert registry.is_instrumented(Eagle) is False + finally: + registry.register_tracked_models(original) + + assert registry.is_instrumented(Eagle) is True diff --git a/tests/test_logger.py b/tests/test_logger.py new file mode 100644 index 0000000..afc43a9 --- /dev/null +++ b/tests/test_logger.py @@ -0,0 +1,24 @@ +import importlib +import logging + +import eagle.logger as logger_module + + +class TestLoggerDebugConfig: + def test_eagle_debug_env_enables_debug_basic_config(self, monkeypatch: object) -> None: + calls: list[dict] = [] + monkeypatch.setattr(logging, "basicConfig", lambda **kwargs: calls.append(kwargs)) + monkeypatch.setenv("EAGLE_DEBUG", "1") + + importlib.reload(logger_module) + + assert calls == [{"level": logging.DEBUG}] + + def test_eagle_debug_unset_skips_basic_config(self, monkeypatch: object) -> None: + calls: list[dict] = [] + monkeypatch.setattr(logging, "basicConfig", lambda **kwargs: calls.append(kwargs)) + monkeypatch.delenv("EAGLE_DEBUG", raising=False) + + importlib.reload(logger_module) + + assert calls == [] diff --git a/tests/test_project/models.py b/tests/test_project/models.py index 09d9358..c2071c8 100644 --- a/tests/test_project/models.py +++ b/tests/test_project/models.py @@ -1,3 +1,5 @@ +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType from django.db import models @@ -16,6 +18,7 @@ class Eagle(models.Model): weight = models.PositiveIntegerField() location = models.ForeignKey(Location, models.CASCADE, null=True, related_name="visitors") previous_locations = models.ManyToManyField(Location, related_name="previous_visitors") + mates = models.ManyToManyField("self") def __repr__(self): return f"" @@ -23,3 +26,13 @@ def __repr__(self): class Eaglet(models.Model): eagle = models.OneToOneField(Eagle, models.CASCADE, null=True, related_name="eaglet") + + +class Aerie(models.Model): + eagle = models.OneToOneField(Eagle, models.CASCADE, related_name="aerie") + + +class Sighting(models.Model): + content_type = models.ForeignKey(ContentType, models.CASCADE) + object_id = models.PositiveIntegerField() + content_object = GenericForeignKey("content_type", "object_id") diff --git a/tests/test_query_internals.py b/tests/test_query_internals.py new file mode 100644 index 0000000..a929ad0 --- /dev/null +++ b/tests/test_query_internals.py @@ -0,0 +1,166 @@ +from types import SimpleNamespace +from unittest.mock import Mock + +import pytest +from django.db.models import QuerySet + +from eagle import unused +from eagle.instrumentation import query +from test_project.models import Aerie +from tests.factories import EagleFactory + + +class TestGetUnrestrictedSelectRelatedGetters: + def test_returns_nothing_beyond_max_depth(self) -> None: + getters = list(query.get_unrestricted_select_related_getters(Aerie._meta, max_depth=0)) + assert getters == [] + + +class TestPropagatePrefetchLocation: + def test_skips_when_child_result_is_not_a_queryset(self) -> None: + state = SimpleNamespace(warn_unused_location="captured", warn_unused_locations=None) + instances = [SimpleNamespace(_state=state)] + + query.propagate_prefetch_location(instances, [1, 2, 3], "cache") + + def test_skips_when_child_queryset_already_tagged(self) -> None: + state = SimpleNamespace(warn_unused_location="captured", warn_unused_locations=None) + instances = [SimpleNamespace(_state=state)] + child_queryset = Mock(spec=QuerySet) + child_queryset._eagle_location = "existing" + + query.propagate_prefetch_location(instances, child_queryset, "cache") + + assert child_queryset._eagle_location == "existing" + + def test_skips_when_resolved_location_is_none(self) -> None: + state = SimpleNamespace(warn_unused_location=None, warn_unused_locations=None) + instances = [SimpleNamespace(_state=state)] + child_queryset = Mock(spec=QuerySet) + + query.propagate_prefetch_location(instances, child_queryset, "cache") + + assert not hasattr(child_queryset, "_eagle_location") + + +class TestRecordLocations: + def test_duplicate_names_are_recorded_once(self) -> None: + clone = SimpleNamespace() + + query._record_locations(clone, iter(["previous_locations", "previous_locations"])) + + assert set(clone._eagle_locations) == {"previous_locations"} + + +class TestPrefetcherCacheName: + def test_returns_none_when_prefetcher_has_no_cache_name(self) -> None: + assert query._prefetcher_cache_name(object()) is None + + +OPERATIONS = [ + ("len", lambda tracked: len(tracked)), + ("getitem", lambda tracked: tracked[0]), + ("contains", lambda tracked: 1 in tracked), + ("reversed", lambda tracked: list(reversed(tracked))), + ("eq", lambda tracked: tracked == [1, 2, 3]), + ("repr", lambda tracked: repr(tracked)), + ("count", lambda tracked: tracked.count(1)), + ("index", lambda tracked: tracked.index(1)), +] + + +@pytest.mark.django_db +class TestTrackedPrefetchListConsumption: + @pytest.mark.parametrize(["label", "operation"], OPERATIONS, ids=[label for label, _ in OPERATIONS]) + def test_operation_marks_prefetch_consumed(self, label: str, operation: object) -> None: + eagle = EagleFactory() + unused.begin_request() + unused.init_state(eagle, location=None) + unused.mark_prefetched([eagle], "previous_locations") + tracked = query.TrackedPrefetchList([1, 2, 3], eagle, "previous_locations") + + operation(tracked) + operation(tracked) + + unused.end_request() + + +@pytest.mark.django_db +class TestEagerPrefetchOneLevel: + def test_inactive_collector_returns_original_result_untouched(self, monkeypatch: object) -> None: + sentinel = object() + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) + assert unused.is_active() is False + + result = query._eager_prefetch_one_level([EagleFactory()], Mock(), Mock(), 0) + + assert result is sentinel + + def test_no_instances_returns_original_result_untouched(self, monkeypatch: object) -> None: + sentinel = object() + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) + unused.begin_request() + try: + result = query._eager_prefetch_one_level([], Mock(), Mock(), 0) + finally: + unused.end_request() + assert result is sentinel + + def test_missing_prefetch_cache_name_returns_original_result(self, monkeypatch: object) -> None: + sentinel = object() + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) + lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) + prefetcher = Mock(spec=[]) + + unused.begin_request() + try: + result = query._eager_prefetch_one_level([EagleFactory()], prefetcher, lookup, 0) + finally: + unused.end_request() + assert result is sentinel + + def test_uninitialized_instance_is_skipped(self, monkeypatch: object) -> None: + sentinel = object() + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) + eagle = EagleFactory() + lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) + prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) + + unused.begin_request() + try: + result = query._eager_prefetch_one_level([eagle], prefetcher, lookup, 0) + finally: + unused.end_request() + assert result is sentinel + assert not hasattr(eagle, "attr") + + def test_already_tracked_list_is_not_rewrapped(self, monkeypatch: object) -> None: + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: None) + eagle = EagleFactory() + lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) + prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) + + unused.begin_request() + unused.init_state(eagle, location=None) + existing = query.TrackedPrefetchList([1, 2], eagle, "cache") + eagle.attr = existing + + query._eager_prefetch_one_level([eagle], prefetcher, lookup, 0) + unused.end_request() + + assert eagle.attr is existing + + def test_single_valued_to_attr_marks_consumed_immediately(self, monkeypatch: object) -> None: + monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: None) + eagle = EagleFactory() + eagle.attr = None + lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) + prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) + + unused.begin_request() + unused.init_state(eagle, location=None) + unused.mark_prefetched([eagle], "cache") + + query._eager_prefetch_one_level([eagle], prefetcher, lookup, 0) + + unused.end_request() diff --git a/tests/test_scope_ignore.py b/tests/test_scope_ignore.py index bc6475d..6a3bf71 100644 --- a/tests/test_scope_ignore.py +++ b/tests/test_scope_ignore.py @@ -27,6 +27,13 @@ def test_ignore_wrong_model_still_warns(self, api_client: APIClient, eagle_graph api_client.get(url, {"select_related": "location"}) assert "location" in str(exc_info.value) + @override_settings(EAGLE_WARN_UNUSED_IGNORE=[{"model": "Eagle", "field": "previous_locations"}]) + def test_ignore_matching_model_wrong_field_still_warns(self, api_client: APIClient, eagle_graph: EagleGraph): + url = reverse("eagle-detail", kwargs={"pk": eagle_graph.eagle.pk}) + with pytest.raises(UnusedRelatedAccess) as exc_info: + api_client.get(url, {"select_related": "location"}) + assert "location" in str(exc_info.value) + @override_settings(EAGLE_WARN_UNUSED_IGNORE=[{"location": "*/test_project/*"}]) def test_ignore_by_location_glob(self, api_client: APIClient, eagle_graph: EagleGraph): url = reverse("eagle-detail", kwargs={"pk": eagle_graph.eagle.pk}) diff --git a/tests/test_scope_internals.py b/tests/test_scope_internals.py new file mode 100644 index 0000000..8a3fc75 --- /dev/null +++ b/tests/test_scope_internals.py @@ -0,0 +1,25 @@ +import sysconfig + +from eagle.instrumentation import scope +from test_project.models import Eagle + + +class TestDependencyRoots: + def test_skips_paths_missing_from_sysconfig(self, monkeypatch: object) -> None: + fake_paths = {"purelib": "", "platlib": None, "stdlib": "/fake/stdlib", "platstdlib": "/fake/stdlib"} + monkeypatch.setattr(sysconfig, "get_paths", lambda: fake_paths) + + roots = scope._dependency_roots() + + assert "/fake/stdlib" in roots + + +class TestGetFirstPartyModelsProxySkip: + def test_proxy_models_are_excluded(self) -> None: + class EagleProxy(Eagle): + class Meta: + app_label = "test_project" + proxy = True + + assert EagleProxy not in set(scope.get_first_party_models()) + assert Eagle in set(scope.get_first_party_models()) diff --git a/tests/test_sinks_normalize_model.py b/tests/test_sinks_normalize_model.py new file mode 100644 index 0000000..b30817c --- /dev/null +++ b/tests/test_sinks_normalize_model.py @@ -0,0 +1,13 @@ +from eagle.sinks import _normalize_model +from test_project.models import Location + + +class TestNormalizeModel: + def test_labelled_string_unknown_model_returns_unchanged(self) -> None: + assert _normalize_model("test_project.NoSuchModel") == "test_project.NoSuchModel" + + def test_unique_bare_class_name_resolves_to_label(self) -> None: + assert _normalize_model("Location") == Location._meta.label + + def test_unmatched_bare_class_name_returns_unchanged(self) -> None: + assert _normalize_model("NoSuchClassAnywhere") == "NoSuchClassAnywhere" diff --git a/tests/test_unused.py b/tests/test_unused.py index ed47492..a5e127a 100644 --- a/tests/test_unused.py +++ b/tests/test_unused.py @@ -2,11 +2,11 @@ from django.urls import reverse from rest_framework.test import APIClient -from eagle import UnusedRelatedAccess, unused +from eagle import UnusedRelatedAccess, unused, warn_unused from test_project import views -from test_project.models import Eagle, Location +from test_project.models import Aerie, Eagle, Location from tests.base import BaseRequestTest, EagleGraph -from tests.factories import BurrowFactory, ClimateFactory, EagleFactory, LocationFactory +from tests.factories import AerieFactory, BurrowFactory, ClimateFactory, EagleFactory, LocationFactory class TestWarnUnusedQuerySet(BaseRequestTest): @@ -175,3 +175,25 @@ def test_select_related_within_prefetch_queryset_accessed_no_warning( url = reverse("eagle-detail", kwargs={"pk": eagle_graph.eagle.pk}) response = api_client.get(url, {"prefetch_select": "eaglet:eagle", "access": "eaglet__eagle"}) assert response.status_code == 200 + + +class TestWarnUnusedUnrestrictedSelectRelated(BaseRequestTest): + def test_bare_select_related_auto_discovers_forward_relation_unused(self, eagle_graph: EagleGraph): + aerie = AerieFactory(eagle=eagle_graph.eagle) + with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): + Aerie.objects.select_related().get(pk=aerie.pk) + assert 'select_related("eagle")' in str(exc_info.value) + + def test_bare_select_related_auto_discovers_forward_relation_accessed(self, eagle_graph: EagleGraph): + aerie = AerieFactory(eagle=eagle_graph.eagle) + with warn_unused(): + fetched = Aerie.objects.select_related().get(pk=aerie.pk) + assert fetched.eagle == eagle_graph.eagle + + +class TestWarnUnusedNestedThroughUninstrumentedOwner(BaseRequestTest): + def test_relation_owned_by_excluded_app_model_still_recurses(self, eagle_graph: EagleGraph): + BurrowFactory(eagle=eagle_graph.eagle) + with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): + Eagle.objects.select_related("burrow__eagle").get(pk=eagle_graph.eagle.pk) + assert 'select_related("burrow")' in str(exc_info.value) diff --git a/tests/test_unused_location.py b/tests/test_unused_location.py new file mode 100644 index 0000000..71651c0 --- /dev/null +++ b/tests/test_unused_location.py @@ -0,0 +1,13 @@ +import traceback + +from eagle.unused import location + + +class TestCaptureLocation: + def test_returns_none_when_every_frame_is_eagle_or_django(self, monkeypatch: object) -> None: + fake_frame = traceback.FrameSummary( + filename=location._EAGLE_DIR + "/instrumentation/query.py", lineno=1, name="f" + ) + monkeypatch.setattr(traceback, "extract_stack", lambda: [fake_frame, fake_frame, fake_frame, fake_frame]) + + assert location.capture_location() is None diff --git a/tests/test_unused_marker_guards.py b/tests/test_unused_marker_guards.py new file mode 100644 index 0000000..57d934f --- /dev/null +++ b/tests/test_unused_marker_guards.py @@ -0,0 +1,41 @@ +import pytest + +from eagle import UnusedRelatedAccess, unused +from tests.factories import EagleFactory + + +@pytest.mark.django_db +class TestMarkerGuardsOutsideActiveRequest: + def test_marking_functions_are_inert_outside_a_request(self): + eagle = EagleFactory() + + assert unused.is_active() is False + unused.mark_select_related(eagle, "_location_cache") + unused.mark_prefetched([eagle], "previous_locations") + unused.mark_consumed(eagle, "_location_cache") + assert unused.is_active() is False + + unused.begin_request() + unused.init_state(eagle, location=None) + unused.mark_select_related(eagle, "_location_cache") + with pytest.raises(UnusedRelatedAccess) as exc_info: + unused.end_request() + assert 'select_related("_location_cache")' in str(exc_info.value) + + def test_mark_prefetched_skips_instance_without_initialized_state(self): + eagle = EagleFactory() + unused.begin_request() + unused.mark_prefetched([eagle], "previous_locations") + unused.end_request() + + def test_second_init_state_call_does_not_overwrite_location(self): + eagle = EagleFactory() + unused.begin_request() + unused.init_state(eagle, location="first-location") + unused.init_state(eagle, location="second-location") + unused.mark_select_related(eagle, "_location_cache") + with pytest.raises(UnusedRelatedAccess) as exc_info: + unused.end_request() + message = str(exc_info.value) + assert "first-location" in message + assert "second-location" not in message diff --git a/tests/test_unused_tracker.py b/tests/test_unused_tracker.py new file mode 100644 index 0000000..84636d3 --- /dev/null +++ b/tests/test_unused_tracker.py @@ -0,0 +1,8 @@ +from eagle import unused + + +class TestEndRequestInactive: + def test_end_request_is_noop_when_not_active(self) -> None: + assert unused.is_active() is False + unused.end_request() + assert unused.is_active() is False From 98167251e3afc2187ae36752f3e75bf90cb00969 Mon Sep 17 00:00:00 2001 From: Cameron Hobbs Date: Fri, 10 Jul 2026 12:57:26 +0100 Subject: [PATCH 2/3] Use fixtures, fix ci workflow --- .github/workflows/ci.yml | 4 ++- assets/coverage.svg | 2 +- tests/test_descriptors_internals.py | 43 +++++++++++++++-------------- tests/test_query_internals.py | 30 +++++++++++--------- tests/test_unused.py | 20 +++++++++----- tests/test_unused_marker_guards.py | 13 +++++---- 6 files changed, 64 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ced9870..78fa46a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,9 @@ jobs: run: uv run coverage xml - name: Generate coverage badge - run: uv run genbadge coverage -i coverage.xml -o assets/coverage.svg + run: | + uv run genbadge coverage -i coverage.xml -o assets/coverage.svg + [ -z "$(tail -c1 assets/coverage.svg)" ] || printf '\n' >> assets/coverage.svg - name: Commit coverage badge if: github.event_name == 'push' && github.ref == 'refs/heads/main' diff --git a/assets/coverage.svg b/assets/coverage.svg index 9cf04bb..00d7fc1 100644 --- a/assets/coverage.svg +++ b/assets/coverage.svg @@ -1 +1 @@ -coverage: 72.29%coverage72.29% \ No newline at end of file +coverage: 72.29%coverage72.29% diff --git a/tests/test_descriptors_internals.py b/tests/test_descriptors_internals.py index 17e10c3..6358840 100644 --- a/tests/test_descriptors_internals.py +++ b/tests/test_descriptors_internals.py @@ -32,8 +32,11 @@ def _prefetch_cache_name(self) -> str: @pytest.mark.django_db class TestTrackingPrefetchCache: - def test_getitem_marks_consumed_when_instance_is_tracked(self) -> None: - eagle = EagleFactory() + @pytest.fixture + def eagle(self) -> Eagle: + return EagleFactory() + + def test_getitem_marks_consumed_when_instance_is_tracked(self, eagle: Eagle) -> None: unused.begin_request() unused.init_state(eagle, location=None) unused.mark_prefetched([eagle], "previous_locations") @@ -50,28 +53,29 @@ def test_getitem_skips_marking_when_instance_untracked(self) -> None: @pytest.mark.django_db class TestPrefetchCacheDescriptor: + @pytest.fixture + def eagle(self) -> Eagle: + return EagleFactory() + def test_class_level_access_returns_descriptor_itself(self) -> None: descriptor = Eagle.__dict__["_prefetched_objects_cache"] assert descriptor.__get__(None, Eagle) is descriptor - def test_set_does_not_rewrap_existing_tracking_cache(self) -> None: + def test_set_does_not_rewrap_existing_tracking_cache(self, eagle: Eagle) -> None: descriptor = Eagle.__dict__["_prefetched_objects_cache"] - eagle = EagleFactory() existing = descriptors.TrackingPrefetchCache({}, _eagle_instance=eagle) descriptor.__set__(eagle, existing) assert eagle.__dict__[descriptor._eagle_storage] is existing - def test_delete_missing_cache_raises_attribute_error(self) -> None: + def test_delete_missing_cache_raises_attribute_error(self, eagle: Eagle) -> None: descriptor = Eagle.__dict__["_prefetched_objects_cache"] - eagle = EagleFactory() with pytest.raises(AttributeError): descriptor.__delete__(eagle) - def test_delete_existing_cache_removes_it(self) -> None: + def test_delete_existing_cache_removes_it(self, eagle: Eagle) -> None: descriptor = Eagle.__dict__["_prefetched_objects_cache"] - eagle = EagleFactory() descriptor.__set__(eagle, {}) descriptor.__delete__(eagle) @@ -91,19 +95,19 @@ class _StubRelatedManager: @pytest.mark.django_db class TestForwardOneToOnePrefetch: - def test_forward_o2o_prefetch_unused_warns(self) -> None: - eagle = EagleFactory() - aerie = AerieFactory(eagle=eagle) + @pytest.fixture + def aerie(self) -> Aerie: + return AerieFactory(eagle=EagleFactory()) + + def test_forward_o2o_prefetch_unused_warns(self, aerie: Aerie) -> None: with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Aerie.objects.prefetch_related("eagle").get(pk=aerie.pk) assert 'prefetch_related("eagle")' in str(exc_info.value) - def test_forward_o2o_prefetch_accessed_no_warning(self) -> None: - eagle = EagleFactory() - aerie = AerieFactory(eagle=eagle) + def test_forward_o2o_prefetch_accessed_no_warning(self, aerie: Aerie) -> None: with warn_unused(): fetched = Aerie.objects.prefetch_related("eagle").get(pk=aerie.pk) - assert fetched.eagle == eagle + assert fetched.eagle == aerie.eagle class TestMakeContenttypesEagerIdempotent: @@ -133,18 +137,17 @@ class _LegacyGenericForeignKey(gfk_mixin, _LegacyBase): @pytest.mark.django_db class TestGenericForeignKeyTracking: - def _make_sighting(self) -> Sighting: + @pytest.fixture + def sighting(self) -> Sighting: eagle = EagleFactory() return Sighting.objects.create(content_type=ContentType.objects.get_for_model(Eagle), object_id=eagle.pk) - def test_prefetched_generic_foreign_key_unused_warns(self) -> None: - sighting = self._make_sighting() + def test_prefetched_generic_foreign_key_unused_warns(self, sighting: Sighting) -> None: with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Sighting.objects.prefetch_related("content_object").get(pk=sighting.pk) assert 'prefetch_related("content_object")' in str(exc_info.value) - def test_prefetched_generic_foreign_key_accessed_no_warning(self) -> None: - sighting = self._make_sighting() + def test_prefetched_generic_foreign_key_accessed_no_warning(self, sighting: Sighting) -> None: with warn_unused(): fetched = Sighting.objects.prefetch_related("content_object").get(pk=sighting.pk) assert fetched.content_object is not None diff --git a/tests/test_query_internals.py b/tests/test_query_internals.py index a929ad0..db4ae66 100644 --- a/tests/test_query_internals.py +++ b/tests/test_query_internals.py @@ -6,7 +6,7 @@ from eagle import unused from eagle.instrumentation import query -from test_project.models import Aerie +from test_project.models import Aerie, Eagle from tests.factories import EagleFactory @@ -71,9 +71,12 @@ def test_returns_none_when_prefetcher_has_no_cache_name(self) -> None: @pytest.mark.django_db class TestTrackedPrefetchListConsumption: + @pytest.fixture + def eagle(self) -> Eagle: + return EagleFactory() + @pytest.mark.parametrize(["label", "operation"], OPERATIONS, ids=[label for label, _ in OPERATIONS]) - def test_operation_marks_prefetch_consumed(self, label: str, operation: object) -> None: - eagle = EagleFactory() + def test_operation_marks_prefetch_consumed(self, eagle: Eagle, label: str, operation: object) -> None: unused.begin_request() unused.init_state(eagle, location=None) unused.mark_prefetched([eagle], "previous_locations") @@ -87,12 +90,16 @@ def test_operation_marks_prefetch_consumed(self, label: str, operation: object) @pytest.mark.django_db class TestEagerPrefetchOneLevel: - def test_inactive_collector_returns_original_result_untouched(self, monkeypatch: object) -> None: + @pytest.fixture + def eagle(self) -> Eagle: + return EagleFactory() + + def test_inactive_collector_returns_original_result_untouched(self, eagle: Eagle, monkeypatch: object) -> None: sentinel = object() monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) assert unused.is_active() is False - result = query._eager_prefetch_one_level([EagleFactory()], Mock(), Mock(), 0) + result = query._eager_prefetch_one_level([eagle], Mock(), Mock(), 0) assert result is sentinel @@ -106,7 +113,7 @@ def test_no_instances_returns_original_result_untouched(self, monkeypatch: objec unused.end_request() assert result is sentinel - def test_missing_prefetch_cache_name_returns_original_result(self, monkeypatch: object) -> None: + def test_missing_prefetch_cache_name_returns_original_result(self, eagle: Eagle, monkeypatch: object) -> None: sentinel = object() monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) @@ -114,15 +121,14 @@ def test_missing_prefetch_cache_name_returns_original_result(self, monkeypatch: unused.begin_request() try: - result = query._eager_prefetch_one_level([EagleFactory()], prefetcher, lookup, 0) + result = query._eager_prefetch_one_level([eagle], prefetcher, lookup, 0) finally: unused.end_request() assert result is sentinel - def test_uninitialized_instance_is_skipped(self, monkeypatch: object) -> None: + def test_uninitialized_instance_is_skipped(self, eagle: Eagle, monkeypatch: object) -> None: sentinel = object() monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) - eagle = EagleFactory() lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) @@ -134,9 +140,8 @@ def test_uninitialized_instance_is_skipped(self, monkeypatch: object) -> None: assert result is sentinel assert not hasattr(eagle, "attr") - def test_already_tracked_list_is_not_rewrapped(self, monkeypatch: object) -> None: + def test_already_tracked_list_is_not_rewrapped(self, eagle: Eagle, monkeypatch: object) -> None: monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: None) - eagle = EagleFactory() lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) @@ -150,9 +155,8 @@ def test_already_tracked_list_is_not_rewrapped(self, monkeypatch: object) -> Non assert eagle.attr is existing - def test_single_valued_to_attr_marks_consumed_immediately(self, monkeypatch: object) -> None: + def test_single_valued_to_attr_marks_consumed_immediately(self, eagle: Eagle, monkeypatch: object) -> None: monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: None) - eagle = EagleFactory() eagle.attr = None lookup = Mock(get_current_to_attr=Mock(return_value=("attr", "attr"))) prefetcher = Mock(_prefetch_cache_name=Mock(return_value="cache")) diff --git a/tests/test_unused.py b/tests/test_unused.py index a5e127a..b96153e 100644 --- a/tests/test_unused.py +++ b/tests/test_unused.py @@ -3,6 +3,7 @@ from rest_framework.test import APIClient from eagle import UnusedRelatedAccess, unused, warn_unused +from excluded_app.models import Burrow from test_project import views from test_project.models import Aerie, Eagle, Location from tests.base import BaseRequestTest, EagleGraph @@ -178,22 +179,27 @@ def test_select_related_within_prefetch_queryset_accessed_no_warning( class TestWarnUnusedUnrestrictedSelectRelated(BaseRequestTest): - def test_bare_select_related_auto_discovers_forward_relation_unused(self, eagle_graph: EagleGraph): - aerie = AerieFactory(eagle=eagle_graph.eagle) + @pytest.fixture + def aerie(self, eagle_graph: EagleGraph) -> Aerie: + return AerieFactory(eagle=eagle_graph.eagle) + + def test_bare_select_related_auto_discovers_forward_relation_unused(self, aerie: Aerie): with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Aerie.objects.select_related().get(pk=aerie.pk) assert 'select_related("eagle")' in str(exc_info.value) - def test_bare_select_related_auto_discovers_forward_relation_accessed(self, eagle_graph: EagleGraph): - aerie = AerieFactory(eagle=eagle_graph.eagle) + def test_bare_select_related_auto_discovers_forward_relation_accessed(self, aerie: Aerie): with warn_unused(): fetched = Aerie.objects.select_related().get(pk=aerie.pk) - assert fetched.eagle == eagle_graph.eagle + assert fetched.eagle == aerie.eagle class TestWarnUnusedNestedThroughUninstrumentedOwner(BaseRequestTest): - def test_relation_owned_by_excluded_app_model_still_recurses(self, eagle_graph: EagleGraph): - BurrowFactory(eagle=eagle_graph.eagle) + @pytest.fixture + def burrow(self, eagle_graph: EagleGraph) -> Burrow: + return BurrowFactory(eagle=eagle_graph.eagle) + + def test_relation_owned_by_excluded_app_model_still_recurses(self, burrow: Burrow, eagle_graph: EagleGraph): with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Eagle.objects.select_related("burrow__eagle").get(pk=eagle_graph.eagle.pk) assert 'select_related("burrow")' in str(exc_info.value) diff --git a/tests/test_unused_marker_guards.py b/tests/test_unused_marker_guards.py index 57d934f..8002991 100644 --- a/tests/test_unused_marker_guards.py +++ b/tests/test_unused_marker_guards.py @@ -1,14 +1,17 @@ import pytest from eagle import UnusedRelatedAccess, unused +from test_project.models import Eagle from tests.factories import EagleFactory @pytest.mark.django_db class TestMarkerGuardsOutsideActiveRequest: - def test_marking_functions_are_inert_outside_a_request(self): - eagle = EagleFactory() + @pytest.fixture + def eagle(self) -> Eagle: + return EagleFactory() + def test_marking_functions_are_inert_outside_a_request(self, eagle: Eagle) -> None: assert unused.is_active() is False unused.mark_select_related(eagle, "_location_cache") unused.mark_prefetched([eagle], "previous_locations") @@ -22,14 +25,12 @@ def test_marking_functions_are_inert_outside_a_request(self): unused.end_request() assert 'select_related("_location_cache")' in str(exc_info.value) - def test_mark_prefetched_skips_instance_without_initialized_state(self): - eagle = EagleFactory() + def test_mark_prefetched_skips_instance_without_initialized_state(self, eagle: Eagle) -> None: unused.begin_request() unused.mark_prefetched([eagle], "previous_locations") unused.end_request() - def test_second_init_state_call_does_not_overwrite_location(self): - eagle = EagleFactory() + def test_second_init_state_call_does_not_overwrite_location(self, eagle: Eagle) -> None: unused.begin_request() unused.init_state(eagle, location="first-location") unused.init_state(eagle, location="second-location") From 957ae528d33c0093777eb1b657875890e7b3d704 Mon Sep 17 00:00:00 2001 From: Cameron Hobbs Date: Fri, 10 Jul 2026 13:05:13 +0100 Subject: [PATCH 3/3] Use fixture mixins --- tests/base.py | 50 +++++++++++++++++++++++++++-- tests/factories.py | 9 +++++- tests/test_descriptors_internals.py | 32 +++--------------- tests/test_query_internals.py | 16 ++------- tests/test_unused.py | 20 ++++-------- tests/test_unused_marker_guards.py | 9 ++---- 6 files changed, 72 insertions(+), 64 deletions(-) diff --git a/tests/base.py b/tests/base.py index 677075e..62cb59f 100644 --- a/tests/base.py +++ b/tests/base.py @@ -4,8 +4,17 @@ from rest_framework.test import APIClient from eagle import unused -from test_project.models import Climate, Eagle, Eaglet, Location -from tests.factories import ClimateFactory, EagleFactory, EagletFactory, LocationFactory +from excluded_app.models import Burrow +from test_project.models import Aerie, Climate, Eagle, Eaglet, Location, Sighting +from tests.factories import ( + AerieFactory, + BurrowFactory, + ClimateFactory, + EagleFactory, + EagletFactory, + LocationFactory, + SightingFactory, +) @dataclass(frozen=True) @@ -53,6 +62,43 @@ def api_client(self) -> APIClient: return APIClient() +@pytest.mark.django_db +class EagleFixtureMixin: + """Supplies a plain ``eagle`` fixture to any test class that needs a single tracked instance.""" + + @pytest.fixture + def eagle(self) -> Eagle: + """Create a single eagle via the default factory.""" + return EagleFactory() + + +class AerieFixtureMixin(EagleFixtureMixin): + """Supplies an ``aerie`` fixture: a non-nullable forward one-to-one owned by the eagle fixture.""" + + @pytest.fixture + def aerie(self, eagle: Eagle) -> Aerie: + """Create an Aerie whose forward one-to-one points at the eagle fixture.""" + return AerieFactory(eagle=eagle) + + +class BurrowFixtureMixin(EagleFixtureMixin): + """Supplies a ``burrow`` fixture: a reverse one-to-one from excluded_app, owned by the eagle fixture.""" + + @pytest.fixture + def burrow(self, eagle: Eagle) -> Burrow: + """Create a Burrow (an excluded_app model) whose one-to-one points at the eagle fixture.""" + return BurrowFactory(eagle=eagle) + + +class SightingFixtureMixin(EagleFixtureMixin): + """Supplies a ``sighting`` fixture: a GenericForeignKey pointing at the eagle fixture.""" + + @pytest.fixture + def sighting(self, eagle: Eagle) -> Sighting: + """Create a Sighting whose content_object GenericForeignKey resolves to the eagle fixture.""" + return SightingFactory(content_object=eagle) + + @pytest.mark.django_db class MayAccessHelperTestBase: """Base for tests of the may_access/mark_considered helpers run inside a tracking request.""" diff --git a/tests/factories.py b/tests/factories.py index 9bc797e..3570efd 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -4,7 +4,7 @@ from collision_app.models import Eagle as CollisionEagle from excluded_app.models import Burrow -from test_project.models import Aerie, Climate, Eagle, Eaglet, Location +from test_project.models import Aerie, Climate, Eagle, Eaglet, Location, Sighting class ClimateFactory(factory.django.DjangoModelFactory): @@ -60,6 +60,13 @@ class Meta: depth = 3 +class SightingFactory(factory.django.DjangoModelFactory): + class Meta: + model = Sighting + + content_object = factory.SubFactory(EagleFactory) + + class CollisionEagleFactory(factory.django.DjangoModelFactory): class Meta: model = CollisionEagle diff --git a/tests/test_descriptors_internals.py b/tests/test_descriptors_internals.py index 6358840..1afa916 100644 --- a/tests/test_descriptors_internals.py +++ b/tests/test_descriptors_internals.py @@ -2,12 +2,11 @@ import pytest from django.contrib.contenttypes.fields import GenericForeignKey -from django.contrib.contenttypes.models import ContentType from eagle import UnusedRelatedAccess, unused, warn_unused from eagle.instrumentation import descriptors from test_project.models import Aerie, Eagle, Sighting -from tests.factories import AerieFactory, EagleFactory +from tests.base import AerieFixtureMixin, EagleFixtureMixin, SightingFixtureMixin class TestEagerPrefetchMixinBase: @@ -30,12 +29,7 @@ def _prefetch_cache_name(self) -> str: assert result == ("child-queryset", None) -@pytest.mark.django_db -class TestTrackingPrefetchCache: - @pytest.fixture - def eagle(self) -> Eagle: - return EagleFactory() - +class TestTrackingPrefetchCache(EagleFixtureMixin): def test_getitem_marks_consumed_when_instance_is_tracked(self, eagle: Eagle) -> None: unused.begin_request() unused.init_state(eagle, location=None) @@ -51,12 +45,7 @@ def test_getitem_skips_marking_when_instance_untracked(self) -> None: assert cache["key"] == "value" -@pytest.mark.django_db -class TestPrefetchCacheDescriptor: - @pytest.fixture - def eagle(self) -> Eagle: - return EagleFactory() - +class TestPrefetchCacheDescriptor(EagleFixtureMixin): def test_class_level_access_returns_descriptor_itself(self) -> None: descriptor = Eagle.__dict__["_prefetched_objects_cache"] assert descriptor.__get__(None, Eagle) is descriptor @@ -93,12 +82,7 @@ class _StubRelatedManager: assert manager_cls()._prefetch_cache_name() == "stub_query_name" -@pytest.mark.django_db -class TestForwardOneToOnePrefetch: - @pytest.fixture - def aerie(self) -> Aerie: - return AerieFactory(eagle=EagleFactory()) - +class TestForwardOneToOnePrefetch(AerieFixtureMixin): def test_forward_o2o_prefetch_unused_warns(self, aerie: Aerie) -> None: with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Aerie.objects.prefetch_related("eagle").get(pk=aerie.pk) @@ -135,13 +119,7 @@ class _LegacyGenericForeignKey(gfk_mixin, _LegacyBase): assert result == ("child-queryset", None) -@pytest.mark.django_db -class TestGenericForeignKeyTracking: - @pytest.fixture - def sighting(self) -> Sighting: - eagle = EagleFactory() - return Sighting.objects.create(content_type=ContentType.objects.get_for_model(Eagle), object_id=eagle.pk) - +class TestGenericForeignKeyTracking(SightingFixtureMixin): def test_prefetched_generic_foreign_key_unused_warns(self, sighting: Sighting) -> None: with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Sighting.objects.prefetch_related("content_object").get(pk=sighting.pk) diff --git a/tests/test_query_internals.py b/tests/test_query_internals.py index db4ae66..a1f0bc7 100644 --- a/tests/test_query_internals.py +++ b/tests/test_query_internals.py @@ -7,7 +7,7 @@ from eagle import unused from eagle.instrumentation import query from test_project.models import Aerie, Eagle -from tests.factories import EagleFactory +from tests.base import EagleFixtureMixin class TestGetUnrestrictedSelectRelatedGetters: @@ -69,12 +69,7 @@ def test_returns_none_when_prefetcher_has_no_cache_name(self) -> None: ] -@pytest.mark.django_db -class TestTrackedPrefetchListConsumption: - @pytest.fixture - def eagle(self) -> Eagle: - return EagleFactory() - +class TestTrackedPrefetchListConsumption(EagleFixtureMixin): @pytest.mark.parametrize(["label", "operation"], OPERATIONS, ids=[label for label, _ in OPERATIONS]) def test_operation_marks_prefetch_consumed(self, eagle: Eagle, label: str, operation: object) -> None: unused.begin_request() @@ -88,12 +83,7 @@ def test_operation_marks_prefetch_consumed(self, eagle: Eagle, label: str, opera unused.end_request() -@pytest.mark.django_db -class TestEagerPrefetchOneLevel: - @pytest.fixture - def eagle(self) -> Eagle: - return EagleFactory() - +class TestEagerPrefetchOneLevel(EagleFixtureMixin): def test_inactive_collector_returns_original_result_untouched(self, eagle: Eagle, monkeypatch: object) -> None: sentinel = object() monkeypatch.setattr(query, "_original_prefetch_one_level", lambda *a: sentinel) diff --git a/tests/test_unused.py b/tests/test_unused.py index b96153e..0706a36 100644 --- a/tests/test_unused.py +++ b/tests/test_unused.py @@ -6,8 +6,8 @@ from excluded_app.models import Burrow from test_project import views from test_project.models import Aerie, Eagle, Location -from tests.base import BaseRequestTest, EagleGraph -from tests.factories import AerieFactory, BurrowFactory, ClimateFactory, EagleFactory, LocationFactory +from tests.base import AerieFixtureMixin, BaseRequestTest, BurrowFixtureMixin, EagleGraph +from tests.factories import BurrowFactory, ClimateFactory, EagleFactory, LocationFactory class TestWarnUnusedQuerySet(BaseRequestTest): @@ -178,11 +178,7 @@ def test_select_related_within_prefetch_queryset_accessed_no_warning( assert response.status_code == 200 -class TestWarnUnusedUnrestrictedSelectRelated(BaseRequestTest): - @pytest.fixture - def aerie(self, eagle_graph: EagleGraph) -> Aerie: - return AerieFactory(eagle=eagle_graph.eagle) - +class TestWarnUnusedUnrestrictedSelectRelated(AerieFixtureMixin): def test_bare_select_related_auto_discovers_forward_relation_unused(self, aerie: Aerie): with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): Aerie.objects.select_related().get(pk=aerie.pk) @@ -194,12 +190,8 @@ def test_bare_select_related_auto_discovers_forward_relation_accessed(self, aeri assert fetched.eagle == aerie.eagle -class TestWarnUnusedNestedThroughUninstrumentedOwner(BaseRequestTest): - @pytest.fixture - def burrow(self, eagle_graph: EagleGraph) -> Burrow: - return BurrowFactory(eagle=eagle_graph.eagle) - - def test_relation_owned_by_excluded_app_model_still_recurses(self, burrow: Burrow, eagle_graph: EagleGraph): +class TestWarnUnusedNestedThroughUninstrumentedOwner(BurrowFixtureMixin): + def test_relation_owned_by_excluded_app_model_still_recurses(self, burrow: Burrow, eagle: Eagle): with pytest.raises(UnusedRelatedAccess) as exc_info, warn_unused(): - Eagle.objects.select_related("burrow__eagle").get(pk=eagle_graph.eagle.pk) + Eagle.objects.select_related("burrow__eagle").get(pk=eagle.pk) assert 'select_related("burrow")' in str(exc_info.value) diff --git a/tests/test_unused_marker_guards.py b/tests/test_unused_marker_guards.py index 8002991..1a5494d 100644 --- a/tests/test_unused_marker_guards.py +++ b/tests/test_unused_marker_guards.py @@ -2,15 +2,10 @@ from eagle import UnusedRelatedAccess, unused from test_project.models import Eagle -from tests.factories import EagleFactory +from tests.base import EagleFixtureMixin -@pytest.mark.django_db -class TestMarkerGuardsOutsideActiveRequest: - @pytest.fixture - def eagle(self) -> Eagle: - return EagleFactory() - +class TestMarkerGuardsOutsideActiveRequest(EagleFixtureMixin): def test_marking_functions_are_inert_outside_a_request(self, eagle: Eagle) -> None: assert unused.is_active() is False unused.mark_select_related(eagle, "_location_cache")