Build: cancel old builds

readthedocs/builds/tests/test_trigger_build.py

@@ -0,0 +1,95 @@
+from unittest import mock
+
+import django_dynamic_fixture as fixture
+import pytest
+
+from readthedocs.builds.constants import (
+    BUILD_FINAL_STATES,
+    BUILD_STATE_BUILDING,
+    BUILD_STATE_CANCELLED,
+    BUILD_STATE_CLONING,
+    BUILD_STATE_FINISHED,
+    BUILD_STATE_INSTALLING,
+    BUILD_STATE_TRIGGERED,
+    BUILD_STATE_UPLOADING,
+)
+from readthedocs.builds.models import Build, Version
+from readthedocs.core.utils import trigger_build
+from readthedocs.projects.models import Feature, Project
+
+
+@pytest.mark.django_db
+class TestCancelOldBuilds:
+    @pytest.fixture(autouse=True)
+    def setup(self):
+        self.project = fixture.get(Project)
+        self.version = fixture.get(Version, project=self.project)
+        self.feature = fixture.get(Feature, feature_id=Feature.CANCEL_OLD_BUILDS)
+        self.feature.projects.add(self.project)
+
+    @pytest.mark.parametrize(
+        "state",
+        [
+            BUILD_STATE_TRIGGERED,
+            BUILD_STATE_CLONING,
+            BUILD_STATE_INSTALLING,
+            BUILD_STATE_BUILDING,
+            BUILD_STATE_UPLOADING,
+        ],
+    )
+    @mock.patch("readthedocs.core.utils.cancel_build")
+    @mock.patch("readthedocs.projects.tasks.builds.update_docs_task")
+    def test_cancel_old_running_build(self, update_docs_task, cancel_build, state):
+        # Create a running build
+        build = fixture.get(
+            Build, project=self.project, version=self.version, state=state
+        )
+
+        builds_count_before = Build.objects.count()
+
+        # Trigger a new build for the same project/version
+        result = trigger_build(
+            project=self.project,
+            version=self.version,
+        )
+
+        triggered_build = Build.objects.first()
+        builds_count_after = Build.objects.count()
+
+        cancel_build.assert_called_once_with(build)
+        assert result == (mock.ANY, triggered_build)
+        assert builds_count_before == builds_count_after - 1
+        assert update_docs_task.signature.called
+        assert update_docs_task.signature().apply_async.called
+
+    @pytest.mark.parametrize(
+        "state",
+        [
+            BUILD_STATE_CANCELLED,
+            BUILD_STATE_FINISHED,
+        ],
+    )
+    @mock.patch("readthedocs.core.utils.cancel_build")
+    @mock.patch("readthedocs.projects.tasks.builds.update_docs_task")
+    def test_not_cancel_old_finished_build(self, update_docs_task, cancel_build, state):
+        # Create a running build
+        build = fixture.get(
+            Build, project=self.project, version=self.version, state=state
+        )
+
+        builds_count_before = Build.objects.count()
+
+        # Trigger a new build for the same project/version
+        result = trigger_build(
+            project=self.project,
+            version=self.version,
+        )
+
+        triggered_build = Build.objects.first()
+        builds_count_after = Build.objects.count()
+
+        cancel_build.assert_not_called()
+        assert result == (mock.ANY, triggered_build)
+        assert builds_count_before == builds_count_after - 1
+        assert update_docs_task.signature.called
+        assert update_docs_task.signature().apply_async.called

readthedocs/builds/tests/test_views.py

@@ -16,7 +16,7 @@
 from readthedocs.projects.models import Project
 
 
-@mock.patch("readthedocs.builds.views.app")
+@mock.patch("readthedocs.core.utils.app")
 @override_settings(RTD_ALLOW_ORGANIZATIONS=False)
 class CancelBuildViewTests(TestCase):
     def setUp(self):

readthedocs/builds/views.py

@@ -1,6 +1,5 @@
 """Views for builds app."""
 
-import signal
 import textwrap
 from urllib.parse import urlparse
 
@@ -23,10 +22,9 @@
 from readthedocs.builds.filters import BuildListFilter
 from readthedocs.builds.models import Build, Version
 from readthedocs.core.permissions import AdminPermission
-from readthedocs.core.utils import trigger_build
-from readthedocs.doc_builder.exceptions import BuildAppError, BuildCancelled
+from readthedocs.core.utils import cancel_build, trigger_build
+from readthedocs.doc_builder.exceptions import BuildAppError
 from readthedocs.projects.models import Project
-from readthedocs.worker import app
 
 log = structlog.get_logger(__name__)
 
@@ -167,33 +165,7 @@ def post(self, request, project_slug, build_pk):
         if not AdminPermission.is_admin(request.user, project):
             return HttpResponseForbidden()
 
-        # NOTE: `terminate=True` is required for the child to attend our call
-        # immediately when it's running the build. Otherwise, it finishes the
-        # task. However, to revoke a task that has not started yet, we don't
-        # need it.
-        if build.state == BUILD_STATE_TRIGGERED:
-            # Since the task won't be executed at all, we need to update the
-            # Build object here.
-            terminate = False
-            build.state = BUILD_STATE_CANCELLED
-            build.success = False
-            build.error = BuildCancelled.message
-            build.length = 0
-            build.save()
-        else:
-            # In this case, we left the update of the Build object to the task
-            # itself to be executed in the `on_failure` handler.
-            terminate = True
-
-        log.warning(
-            'Canceling build.',
-            project_slug=project.slug,
-            version_slug=build.version.slug,
-            build_id=build.pk,
-            build_task_id=build.task_id,
-            terminate=terminate,
-        )
-        app.control.revoke(build.task_id, signal=signal.SIGINT, terminate=terminate)
+        cancel_build(build)
 
         return HttpResponseRedirect(
             reverse('builds_detail', args=[project.slug, build.pk]),