Merge pull request #1577 from rtfd/build-failure-email

Fix email notifications on build failures

Author: ericholscher

readthedocs/projects/tasks.py

@@ -142,6 +142,9 @@ def run(self, pk, version_pk=None, build_pk=None, record=True, docker=False,
                     epub=outcomes['epub'],
                 )
 
+        if self.build_env.failed:
+            self.send_notifications()
+
     @staticmethod
     def get_project(project_pk):
         """
@@ -424,6 +427,10 @@ def build_docs_class(self, builder_class):
         builder.move()
         return success
 
+    def send_notifications(self):
+        """Send notifications on build failure"""
+        send_notifications.delay(self.version.pk, build_pk=self.build['id'])
+
 
 update_docs = celery_app.tasks[UpdateDocsTask.name]
 
@@ -543,8 +550,6 @@ def finish_build(version_pk, build_pk, hostname=None, html=False,
     update_static_metadata.delay(version.project.pk)
     fileify.delay(version.pk, commit=build.commit)
     update_search.delay(version.pk, commit=build.commit)
-    if not html and version.slug != STABLE and build.exit_code != 423:
-        send_notifications.delay(version.pk, build_pk=build.pk)
 
 
 @task(queue='web')

readthedocs/rtd_tests/mocks/mock_api.py

@@ -77,13 +77,20 @@ def __init__(self, repo):
     def project(self, x):
         return ProjectData()
 
+    def build(self, x):
+        return mock.Mock(**{'get.return_value': {'state': 'triggered'}})
+
+    def command(self, x):
+        return mock.Mock(**{'get.return_value': {}})
+
 
 @contextmanager
 def mock_api(repo):
     api_mock = MockApi(repo)
-    with (
-            mock.patch('readthedocs.restapi.client.api', api_mock) and
-            mock.patch('readthedocs.api.client.api', api_mock) and
-            mock.patch('readthedocs.projects.tasks.api_v2', api_mock) and
-            mock.patch('readthedocs.projects.tasks.api_v1', api_mock)):
+    with mock.patch('readthedocs.restapi.client.api', api_mock), \
+            mock.patch('readthedocs.api.client.api', api_mock), \
+            mock.patch('readthedocs.projects.tasks.api_v2', api_mock), \
+            mock.patch('readthedocs.projects.tasks.api_v1', api_mock), \
+            mock.patch('readthedocs.doc_builder.environments.api_v1', api_mock), \
+            mock.patch('readthedocs.doc_builder.environments.api_v2', api_mock):
         yield api_mock

readthedocs/rtd_tests/tests/test_celery.py

@@ -5,8 +5,10 @@
 from tempfile import mkdtemp
 
 from django.contrib.auth.models import User
+from django_dynamic_fixture import get
 from mock import patch, MagicMock
 
+from readthedocs.builds.models import Build
 from readthedocs.projects.models import Project
 from readthedocs.projects import tasks
 
@@ -66,10 +68,14 @@ def test_clear_artifacts(self):
     @patch('readthedocs.projects.tasks.UpdateDocsTask.setup_vcs',
            new=MagicMock)
     def test_update_docs(self):
-        with mock_api(self.repo):
-            update_docs = tasks.UpdateDocsTask()
-            result = update_docs.delay(self.project.pk, record=False,
-                                       intersphinx=False)
+        build = get(Build, project=self.project,
+                    version=self.project.versions.first())
+        with mock_api(self.repo) as mapi:
+            result = tasks.update_docs.delay(
+                self.project.pk,
+                build_pk=build.pk,
+                record=False,
+                intersphinx=False)
         self.assertTrue(result.successful())
 
     def test_update_imported_doc(self):

readthedocs/templates/projects/email/build_failed.html

@@ -9,8 +9,14 @@
 {% endblock %}
 
 {% block content %}
+  {% if build.error %}
+    <p>
+      Error: {{ build.error }}
+    </p>
+  {% endif %}
+
   <p>
-    You can see what went wrong here:
+    You can find out more about this failure here:
   </p>
 
   <p>

readthedocs/templates/projects/email/build_failed.txt

@@ -1,11 +1,11 @@
 {% extends "core/email/common.txt" %}
 {% block salutation %}Build Failed for {{ project.name }} ({{ version.verbose_name }}){% endblock %}
 {% block content %}
-
-
-
-
-You can see what went wrong here:
+{% if build.error %}
+Error:
+{{ build.error }}
+{% endif %}
+You can find out more about this failure here:
 {{ build_url }}
 
 If you have questions, a good place to start is the FAQ: