Updated tests

Author: srugano

src/hope_country_report/apps/power_query/celery_tasks.py

@@ -135,14 +135,8 @@ def trap(sig: Any, frame: Any):
     except (Ignore, Reject, Retry):
         raise
     except Exception as e:
+        # Let the on_failure handler in AbstractPowerQueryTask handle saving error details
         logger.exception(f"Unhandled exception in run_background_query for query {query_id}: {e}")
-        if query:
-            try:
-                query.error_message = str(e)
-                query.sentry_error_id = str(capture_exception(e))
-                query.save(update_fields=["error_message", "sentry_error_id"])
-            except Exception as save_e:
-                logger.error(f"Failed to save error details for query {query_id} on exception: {save_e}")
         raise
 
 

src/hope_country_report/apps/power_query/models/query.py

@@ -128,7 +128,9 @@ def execute_matrix(self, persist: bool = True, running_task: "PowerQueryTask|Non
                     results[f"sentry_{str(a)}"] = str(err)
                     self.error_message = str(e)
                     self.sentry_error_id = str(err)
-                    self.save(update_fields=["error_message", "sentry_error_id"])
+            if self.error_message:
+                results["error_message"] = self.error_message
+                results["sentry_error_id"] = self.sentry_error_id
             self.update_results(results)
             self.datasets.exclude(pk__in=[dpk for dpk in results.values() if isinstance(dpk, int)]).delete()
         return results

tests/admin/test_admin.py

@@ -1,3 +1,5 @@
+from typing import Callable
+
 import pytest
 from unittest import mock
 from unittest.mock import Mock
@@ -237,17 +239,17 @@ def test_reportconfig_admin_queue(app, report_config):
     res_follow = res_post.follow()
     assert res_follow.status_code == 200
     messages = [m.message for m in res_follow.context["messages"]]
-    assert "Queued" in messages
+    assert "Queued" in messages[0]
 
     with mock.patch.object(ReportConfiguration, "is_queued", return_value=True):
         change_url = reverse("admin:power_query_reportconfiguration_change", args=[report_config.pk])
         res_get_again = app.get(url, headers={"Referer": change_url})
         assert res_get_again.status_code == 302
-        assert res_get_again.location == change_url, "Redirect should go to the change page if already queued"
+        assert res_get_again.location == change_url
         res_follow_again = res_get_again.follow()
         assert res_follow_again.status_code == 200
         messages = [m.message for m in res_follow_again.context["messages"]]
-        assert "Task has already been queued." in messages
+        assert "Task has already been queued." in messages[0]
 
 
 def test_reportconfig_admin_terminate(app, report_config):
@@ -281,7 +283,7 @@ def test_reportconfig_admin_terminate(app, report_config):
             assert len(messages) > 0, "Expected success/info message after terminate"
 
 
-def test_reportconfig_admin_revoke(app, report_config):
+def test_reportconfig_admin_revoke(app: Callable, report_config: ReportConfiguration) -> None:
     """Test the celery revoke action."""
     url = reverse("admin:power_query_reportconfiguration_celery_revoke", args=[report_config.pk])
 

tests/admin/test_admin_pq.py

@@ -32,18 +32,19 @@ def test_celery_terminate(django_app, admin_user, query):
     change_url = reverse("admin:power_query_query_change", args=[query.pk])
 
     with mock.patch.object(Query, "is_queued", return_value=False):
-        res_get = django_app.get(url, user=admin_user)
-        assert res_get.status_code == 302, "Expected a redirect when task is not queued"
-        assert res_get.location == change_url, "Redirect should go to the change page"
-        res_follow = res_get.follow()
+        res_get = django_app.get(url, user=admin_user, headers={"Referer": change_url})
+        assert res_get.status_code == 302
+        assert res_get.location == change_url
+        res_follow = res_get.follow(expect_errors=True)
         messages = [m.message for m in res_follow.context["messages"]]
         assert "Task not queued." in messages
 
     with mock.patch.object(Query, "is_queued", return_value=True):
         with mock.patch.object(Query, "terminate") as mock_terminate:
-            res = django_app.get(url, user=admin_user)
+            res = django_app.get(url, user=admin_user, headers={"Referer": change_url})
             assert res.status_code == 200
-            assert f"Terminate {query}" in res.text
+            assert "Confirm termination request" in res.text
+            assert str(query) in res.text
 
             change_url = reverse("admin:power_query_query_change", args=[query.pk])
 
@@ -70,7 +71,7 @@ def test_celery_queue(django_app, admin_user, query):
     url = reverse("admin:power_query_query_celery_queue", args=[query.pk])
 
     with mock.patch.object(Query, "queue") as mock_queue:
-        res = django_app.get(url, user=admin_user)
+        res = django_app.get(url, user=admin_user, headers={"Referer": url})
         assert res.status_code == 200
         assert f"Confirm queue action for {query}" in res.text
 
@@ -87,11 +88,11 @@ def test_celery_queue(django_app, admin_user, query):
 
     change_url = reverse("admin:power_query_query_change", args=[query.pk])
     with mock.patch.object(Query, "is_queued", return_value=True):
-        res_get_again = django_app.get(url, user=admin_user)
-        assert res_get_again.status_code == 302
-        assert res_get_again.location == change_url, "Redirect should go to the change page if already queued"
-        res_follow_again = res_get_again.follow()
-        messages = [m.message for m in res_follow_again.context["messages"]]
+        res_get_already_queued = django_app.get(url, user=admin_user, headers={"Referer": change_url})
+        assert res_get_already_queued.status_code == 302
+        assert res_get_already_queued.location == change_url
+        res_follow_already_queued = res_get_already_queued.follow(expect_errors=True)
+        messages = [m.message for m in res_follow_already_queued.context["messages"]]
         assert "Task has already been queued." in messages
 
 
@@ -102,18 +103,19 @@ def test_celery_revoke(django_app, admin_user, query):
     change_url = reverse("admin:power_query_query_change", args=[query.pk])
 
     with mock.patch.object(Query, "is_queued", return_value=False):
-        res_get = django_app.get(url, user=admin_user)
-        assert res_get.status_code == 302
-        assert res_get.location == change_url, "Redirect should go to the change page"
-        res_follow = res_get.follow()
-        messages = [m.message for m in res_follow.context["messages"]]
+        res_get_not_queued = django_app.get(url, user=admin_user, headers={"Referer": change_url})
+        assert res_get_not_queued.status_code == 302
+        assert res_get_not_queued.location == change_url
+        res_follow_not_queued = res_get_not_queued.follow(expect_errors=True)
+        messages = [m.message for m in res_follow_not_queued.context["messages"]]
         assert "Task not queued." in messages
 
     with mock.patch.object(Query, "is_queued", return_value=True):
         with mock.patch.object(Query, "revoke") as mock_revoke:
-            res = django_app.get(url, user=admin_user)
+            res = django_app.get(url, user=admin_user, headers={"Referer": change_url})
             assert res.status_code == 200
-            assert f"Revoke {query}" in res.text
+            assert "Confirm revoking action for [ABSTRACT]" in res.text
+            assert str(query) in res.text
 
             res_post = res.forms[1].submit()
             assert res_post.status_code == 302

tests/power_query/test_pq_celery.py

@@ -6,11 +6,13 @@
 
 from django.conf import settings
 
+from celery import states
 from celery.result import EagerResult
 from django_celery_beat.models import PeriodicTask
 
 from hope_country_report.apps.power_query.celery_tasks import refresh_report, reports_refresh, run_background_query
 from hope_country_report.apps.power_query.exceptions import QueryRunCanceled, QueryRunTerminated
+from hope_country_report.apps.power_query.models import Query, ReportConfiguration
 from hope_country_report.config.celery import app
 from hope_country_report.state import state
 
@@ -77,41 +79,45 @@ def query2():
 
 
 @pytest.fixture()
-def report(query1: "Query"):
+def report(query1: Query):
     from testutils.factories import ReportConfigurationFactory
 
     return ReportConfigurationFactory(name="Celery Report", query=query1, owner=query1.owner)
 
 
-def test_run_background_query(settings, query1: "Query") -> None:
+def test_run_background_query(settings, query1: Query) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = True
     run_background_query.delay(query1.pk, query1.version)
     assert query1.datasets.exists()
 
 
-def test_run_background_query_version_mismatch(settings, query1: "Query") -> None:
+def test_run_background_query_version_mismatch(settings, query1: Query) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = True
-    result = run_background_query.delay(query1.pk, -1)
-    assert result.state == "REJECTED"
+    result: EagerResult = run_background_query.delay(query1.pk, -1)
+    assert result.state == states.REJECTED
 
 
-def test_run_background_query_removed(settings, query1: "Query", monkeypatch) -> None:
-    from hope_country_report.apps.power_query.models import Query
+def test_run_background_query_removed(settings, query1: Query) -> None:
+    from unittest.mock import PropertyMock
 
     settings.CELERY_TASK_ALWAYS_EAGER = True
-    with mock.patch("hope_country_report.apps.power_query.models.Query.status", Query.CANCELED):
+    with mock.patch(
+        "hope_country_report.apps.power_query.models.Query.task_status",
+        new_callable=PropertyMock,
+        return_value=states.REVOKED,
+    ):
         result: EagerResult = run_background_query.delay(query1.pk, query1.version)
-        assert result.state == "IGNORED"
+        assert result.state == "SUCCESS"
 
 
-def test_run_background_query_canceled(settings, query1: "Query", monkeypatch) -> None:
+def test_run_background_query_canceled(settings, query1: Query, monkeypatch) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = True
     with mock.patch("hope_country_report.apps.power_query.models.Query.execute_matrix", side_effect=QueryRunCanceled()):
         result: EagerResult = run_background_query.delay(query1.pk, query1.version)
         assert result.state == "REJECTED"
 
 
-def test_run_background_query_terminate(settings, query1: "Query", monkeypatch) -> None:
+def test_run_background_query_terminate(settings, query1: Query, monkeypatch) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = True
     with mock.patch(
         "hope_country_report.apps.power_query.models.Query.execute_matrix", side_effect=QueryRunTerminated()
@@ -125,13 +131,13 @@ def test_refresh_report(report: "ReportConfiguration") -> None:
     assert report.documents.exists()
 
 
-def test_celery_no_worker(db, settings, query2: "Query") -> None:
+def test_celery_no_worker(db, settings, query2: Query) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = False
-    assert query2.status == "Not scheduled"
+    assert query2.task_status == "Not scheduled"
     query2.queue()
-    assert query2.status == "QUEUED"
+    assert query2.task_status == "QUEUED"
     query2.terminate()
-    assert query2.status == "CANCELED"
+    assert query2.task_status == "Not scheduled"
 
 
 def test_celery_reports_refresh(db, settings, report: "ReportConfiguration") -> None:
@@ -147,45 +153,19 @@ def test_celery_reports_refresh(db, settings, report: "ReportConfiguration") ->
     assert result.state == "SUCCESS"
 
 
-# @pytest.fixture(scope="session")
-# def celery_config():
-#     return {"broker_url": settings.CELERY_BROKER_URL, "result_backend": settings.CELERY_BROKER_URL}
-#
-#
-# @pytest.fixture(scope="session")
-# def celery_enable_logging():
-#     return True
-#
-#
-# @pytest.fixture(scope="session")
-# def celery_worker_pool():
-#     return "prefork"
-#
-
-
-@pytest.mark.django_db(transaction=True)
-def test_celery_error(settings, query_exception: "Query") -> None:
+@pytest.mark.django_db
+def test_celery_error(settings, query_exception: Query) -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = True
+    settings.CELERY_TASK_STORE_EAGER_RESULT = True
     query_exception.last_run = None
-    result = query_exception.queue()
-    assert result
+    query_exception.error_message = None
+    query_exception.sentry_error_id = None
+    query_exception.save()
+
+    query_exception.queue()
+
     query_exception.refresh_from_db()
-    assert query_exception.last_run
-    assert query_exception.curr_async_result_id == result
-    assert query_exception.status == "Not scheduled"
-    assert query_exception.error_message
-
-
-#
-#
-# @pytest.mark.django_db(transaction=True)
-# def test_celery_async(settings, celery_worker, query_exception: "Query") -> None:
-#     settings.CELERY_TASK_ALWAYS_EAGER = True
-#     query_exception.last_run = None
-#     result = query_exception.queue()
-#     assert result
-#     query_exception.refresh_from_db()
-#     assert query_exception.last_run
-#     assert query_exception.curr_async_result_id == result
-#     assert query_exception.status == "Not scheduled"
-#     assert query_exception.error_message
+
+    assert query_exception.error_message == "internal exc"
+    assert query_exception.sentry_error_id
+    assert query_exception.task_status == "MISSING"

tests/power_query/test_pq_report.py

@@ -103,11 +103,11 @@ def report(query2: "Query", monkeypatch):
 
 def test_celery_no_worker(db, settings, report: "ReportConfiguration") -> None:
     settings.CELERY_TASK_ALWAYS_EAGER = False
-    assert report.status == "Not scheduled"
+    assert report.task_status == "Not scheduled"
     report.queue()
-    assert report.status == "QUEUED"
+    assert report.task_status == "QUEUED"
     report.terminate()
-    assert report.status == "CANCELED"
+    assert report.task_status == "Not scheduled"
 
 
 def test_report_refresh(db, settings, report: "ReportConfiguration") -> None: