From 5c08e2a804dce899da9ba9affe994c2ae9e06316 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 24 Jul 2025 14:15:53 +0200 Subject: [PATCH 1/2] Build: test for healthcheck Continuation of #12332 --- readthedocs/projects/tasks/utils.py | 3 +- readthedocs/rtd_tests/tests/test_project.py | 59 ----------------- .../rtd_tests/tests/test_projects_tasks.py | 64 ++++++++++++++++++- readthedocs/settings/base.py | 1 + readthedocs/settings/test.py | 2 + 5 files changed, 66 insertions(+), 63 deletions(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index 9a1fa8a0184..10714d088cf 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -124,7 +124,8 @@ def finish_unhealthy_builds(): build.save() # Tell Celery to cancel this task in case it's in a zombie state. - app.control.revoke(build.task_id, signal="SIGINT", terminate=True) + if not settings.IS_TESTING: + app.control.revoke(build.task_id, signal="SIGINT", terminate=True) Notification.objects.add( message_id=BuildAppError.BUILD_TERMINATED_DUE_INACTIVITY, diff --git a/readthedocs/rtd_tests/tests/test_project.py b/readthedocs/rtd_tests/tests/test_project.py index a04b14c8b94..998b1575f9d 100644 --- a/readthedocs/rtd_tests/tests/test_project.py +++ b/readthedocs/rtd_tests/tests/test_project.py @@ -577,62 +577,3 @@ def test_user_can_change_project_with_same_lang(self): ) self.assertEqual(resp.status_code, 200) self.assertNotContains(resp, "There is already a") - - -class TestFinishInactiveBuildsTask(TestCase): - fixtures = ["eric", "test_data"] - - def setUp(self): - self.client.login(username="eric", password="test") - self.pip = Project.objects.get(slug="pip") - - self.taggit = Project.objects.get(slug="taggit") - self.taggit.container_time_limit = 7200 # 2 hours - self.taggit.save() - - # Build just started with the default time - self.build_1 = Build.objects.create( - project=self.pip, - version=self.pip.get_stable_version(), - state=BUILD_STATE_CLONING, - ) - - # Build started an hour ago with default time - self.build_2 = Build.objects.create( - project=self.pip, - version=self.pip.get_stable_version(), - state=BUILD_STATE_TRIGGERED, - ) - self.build_2.date = timezone.now() - datetime.timedelta(hours=1) - self.build_2.save() - - # Build started an hour ago with custom time (2 hours) - self.build_3 = Build.objects.create( - project=self.taggit, - version=self.taggit.get_stable_version(), - state=BUILD_STATE_TRIGGERED, - ) - self.build_3.date = timezone.now() - datetime.timedelta(hours=1) - self.build_3.save() - - @pytest.mark.xfail(reason="Fails while we work out Docker time limits", strict=True) - def test_finish_inactive_builds_task(self): - finish_inactive_builds() - - # Legitimate build (just started) not finished - self.build_1.refresh_from_db() - self.assertTrue(self.build_1.success) - self.assertEqual(self.build_1.error, "") - self.assertEqual(self.build_1.state, BUILD_STATE_CLONING) - - # Build with default time finished - self.build_2.refresh_from_db() - self.assertFalse(self.build_2.success) - self.assertNotEqual(self.build_2.error, "") - self.assertEqual(self.build_2.state, BUILD_STATE_FINISHED) - - # Build with custom time not finished - self.build_3.refresh_from_db() - self.assertTrue(self.build_3.success) - self.assertEqual(self.build_3.error, "") - self.assertEqual(self.build_3.state, BUILD_STATE_TRIGGERED) diff --git a/readthedocs/rtd_tests/tests/test_projects_tasks.py b/readthedocs/rtd_tests/tests/test_projects_tasks.py index 5beec0d5bf4..7b77055b340 100644 --- a/readthedocs/rtd_tests/tests/test_projects_tasks.py +++ b/readthedocs/rtd_tests/tests/test_projects_tasks.py @@ -1,12 +1,21 @@ +import datetime + from unittest.mock import patch from django.test import TestCase +from django.utils import timezone from django_dynamic_fixture import get -from readthedocs.builds.constants import BUILD_STATUS_SUCCESS, EXTERNAL +from readthedocs.builds.constants import ( + BUILD_STATUS_SUCCESS, + BUILD_STATE_CLONING, + BUILD_STATE_CANCELLED, + BUILD_STATE_TRIGGERED, + EXTERNAL, +) from readthedocs.builds.models import Build, Version -from readthedocs.projects.models import Project -from readthedocs.projects.tasks.utils import send_external_build_status +from readthedocs.projects.models import Feature, Project +from readthedocs.projects.tasks.utils import finish_unhealthy_builds, send_external_build_status class SendBuildStatusTests(TestCase): @@ -46,3 +55,52 @@ def test_send_external_build_status_with_internal_version(self, send_build_statu ) send_build_status.delay.assert_not_called() + +class TestFinishInactiveBuildsTask(TestCase): + + def test_finish_unhealthy_builds_task(self): + project = get(Project) + feature = get(Feature, feature_id=Feature.BUILD_HEALTHCHECK) + feature.projects.add(project) + + # Build just started with the default time and healthcheck now + build_1 = get( + Build, + project=project, + version=project.get_stable_version(), + state=BUILD_STATE_CLONING, + healthcheck=timezone.now(), + ) + + # Build started an hour ago with default time and healthcheck 59s ago + build_2 = get( + Build, + project=project, + version=project.get_stable_version(), + state=BUILD_STATE_TRIGGERED, + date=timezone.now() - datetime.timedelta(hours=1), + healthcheck=timezone.now() - datetime.timedelta(seconds=59), + ) + + # Build started an hour ago with custom time (2 hours) and healthcheck 15m ago + build_3 = get( + Build, + project=project, + version=project.get_stable_version(), + state=BUILD_STATE_TRIGGERED, + date=timezone.now() - datetime.timedelta(hours=2), + healthcheck=timezone.now() - datetime.timedelta(minutes=15), + ) + + finish_unhealthy_builds() + + build_1.refresh_from_db() + self.assertEqual(build_1.state, BUILD_STATE_CLONING) + + build_2.refresh_from_db() + self.assertEqual(build_2.state, BUILD_STATE_TRIGGERED) + + build_3.refresh_from_db() + self.assertEqual(build_3.state, BUILD_STATE_CANCELLED) + self.assertEqual(build_3.success, False) + self.assertEqual(build_3.notifications.count(), 1) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 3891e8eea57..abf02f7946b 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -49,6 +49,7 @@ class CommunityBaseSettings(Settings): # Debug settings DEBUG = True RTD_FORCE_SHOW_DEBUG_TOOLBAR = False + IS_TESTING = False # Build FTD index for all versions RTD_FILETREEDIFF_ALL = False diff --git a/readthedocs/settings/test.py b/readthedocs/settings/test.py index c32d2befab9..0351fec4106 100644 --- a/readthedocs/settings/test.py +++ b/readthedocs/settings/test.py @@ -15,6 +15,8 @@ class CommunityTestSettings(CommunityBaseSettings): PUBLIC_DOMAIN = "readthedocs.io" DONT_HIT_DB = False + IS_TESTING = True + # Disable password validators on tests AUTH_PASSWORD_VALIDATORS = [] From afa243895d7f4145be91dd97d02e91c8514617c1 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 12 Aug 2025 13:17:43 +0200 Subject: [PATCH 2/2] Use mock instead of a IS_TESTING setting --- readthedocs/projects/tasks/utils.py | 3 +-- readthedocs/rtd_tests/tests/test_projects_tasks.py | 3 ++- readthedocs/settings/base.py | 1 - readthedocs/settings/test.py | 2 -- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index 10714d088cf..9a1fa8a0184 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -124,8 +124,7 @@ def finish_unhealthy_builds(): build.save() # Tell Celery to cancel this task in case it's in a zombie state. - if not settings.IS_TESTING: - app.control.revoke(build.task_id, signal="SIGINT", terminate=True) + app.control.revoke(build.task_id, signal="SIGINT", terminate=True) Notification.objects.add( message_id=BuildAppError.BUILD_TERMINATED_DUE_INACTIVITY, diff --git a/readthedocs/rtd_tests/tests/test_projects_tasks.py b/readthedocs/rtd_tests/tests/test_projects_tasks.py index 7b77055b340..a21b2030426 100644 --- a/readthedocs/rtd_tests/tests/test_projects_tasks.py +++ b/readthedocs/rtd_tests/tests/test_projects_tasks.py @@ -58,7 +58,8 @@ def test_send_external_build_status_with_internal_version(self, send_build_statu class TestFinishInactiveBuildsTask(TestCase): - def test_finish_unhealthy_builds_task(self): + @patch("readthedocs.projects.tasks.utils.app") + def test_finish_unhealthy_builds_task(self, mocked_app): project = get(Project) feature = get(Feature, feature_id=Feature.BUILD_HEALTHCHECK) feature.projects.add(project) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index abf02f7946b..3891e8eea57 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -49,7 +49,6 @@ class CommunityBaseSettings(Settings): # Debug settings DEBUG = True RTD_FORCE_SHOW_DEBUG_TOOLBAR = False - IS_TESTING = False # Build FTD index for all versions RTD_FILETREEDIFF_ALL = False diff --git a/readthedocs/settings/test.py b/readthedocs/settings/test.py index 0351fec4106..c32d2befab9 100644 --- a/readthedocs/settings/test.py +++ b/readthedocs/settings/test.py @@ -15,8 +15,6 @@ class CommunityTestSettings(CommunityBaseSettings): PUBLIC_DOMAIN = "readthedocs.io" DONT_HIT_DB = False - IS_TESTING = True - # Disable password validators on tests AUTH_PASSWORD_VALIDATORS = []