De-duplicate builds

readthedocs/builds/migrations/0023_add_status_code.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.12 on 2020-06-03 15:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0022_migrate_protected_versions'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='build',
+            name='status_code',
+            field=models.BooleanField(blank=True, default=None, null=True, verbose_name='Status code'),
+        ),
+    ]

readthedocs/builds/models.py

@@ -615,6 +615,7 @@ class Build(models.Model):
         choices=BUILD_STATE,
         default='finished',
     )
+    status_code = models.BooleanField(_('Status code'), null=True, default=None, blank=True)
     date = models.DateTimeField(_('Date'), auto_now_add=True)
     success = models.BooleanField(_('Success'), default=True)
 

readthedocs/core/utils/__init__.py

@@ -175,6 +175,8 @@ def prepare_build(
                 commit=commit,
             ).exclude(
                 state=BUILD_STATE_FINISHED,
+            ).exclude(
+                pk=build.pk,
             ).exists()
         )
     else:
@@ -194,8 +196,9 @@ def prepare_build(
             commit,
         )
         build.error = DuplicatedBuildError.message
+        build.status_code = DuplicatedBuildError.status_code
+        build.exit_code = DuplicatedBuildError.exit_code
         build.success = False
-        build.exit_code = 1
         build.state = BUILD_STATE_FINISHED
         build.save()
 

readthedocs/doc_builder/exceptions.py

@@ -59,6 +59,8 @@ class BuildMaxConcurrencyError(BuildEnvironmentError):
 
 class DuplicatedBuildError(BuildEnvironmentError):
     message = ugettext_noop('Duplicated build.')
+    exit_code = 1
+    status_code = 0
 
 
 class BuildEnvironmentWarning(BuildEnvironmentException):

readthedocs/projects/tasks.py

@@ -543,10 +543,7 @@ def run(
             self.commit = commit
             self.config = None
 
-            if all([
-                    self.build['state'] == BUILD_STATE_FINISHED,
-                    self.build['error'] == DuplicatedBuildError.message,
-            ]):
+            if self.build['status_code'] == DuplicatedBuildError.status_code:
                 log.warning(
                     'NOOP: build is marked as duplicated. project=%s version=%s build=%s commit=%s',
                     self.project.slug,

readthedocs/rtd_tests/tests/test_builds.py

@@ -18,8 +18,10 @@
     GENERIC_EXTERNAL_VERSION_NAME
 )
 from readthedocs.builds.models import Build, Version
+from readthedocs.core.utils import trigger_build
 from readthedocs.doc_builder.config import load_yaml_config
 from readthedocs.doc_builder.environments import LocalBuildEnvironment
+from readthedocs.doc_builder.exceptions import DuplicatedBuildError
 from readthedocs.doc_builder.python_environments import Virtualenv
 from readthedocs.oauth.models import RemoteRepository
 from readthedocs.projects.models import EnvironmentVariable, Project
@@ -747,3 +749,106 @@ def test_get_commit_url_internal_version(self):
             sha=build.commit
         )
         self.assertEqual(build.get_commit_url(), expected_url)
+
+
+
+@mock.patch('readthedocs.projects.tasks.update_docs_task')
+class DeDuplicateBuildTests(TestCase):
+
+    def setUp(self):
+        self.project = get(Project)
+        self.version = get(
+            Version,
+            project=self.project
+        )
+
+    def test_trigger_duplicated_build_by_commit(self, update_docs_task):
+        """
+        Trigger a build for the same commit twice.
+
+        The second build should be marked as NOOP.
+        """
+        self.assertEqual(Build.objects.count(), 0)
+        trigger_build(self.project, self.version, commit='a1b2c3')
+        self.assertEqual(Build.objects.count(), 1)
+        build = Build.objects.first()
+        self.assertEqual(build.state, 'triggered')
+
+        trigger_build(self.project, self.version, commit='a1b2c3')
+        self.assertEqual(Build.objects.count(), 2)
+        build = Build.objects.first()
+        self.assertEqual(build.error, DuplicatedBuildError.message)
+        self.assertEqual(build.success, False)
+        self.assertEqual(build.exit_code, DuplicatedBuildError.exit_code)
+        self.assertEqual(build.status_code, DuplicatedBuildError.status_code)
+        self.assertEqual(build.state, 'finished')
+
+    def test_trigger_duplicated_finshed_build_by_commit(self, update_docs_task):
+        """
+        Trigger a build for the same commit twice.
+
+        The second build should not be marked as NOOP if the previous
+        duplicated builds are in 'finished' state.
+        """
+        self.assertEqual(Build.objects.count(), 0)
+        trigger_build(self.project, self.version, commit='a1b2c3')
+        self.assertEqual(Build.objects.count(), 1)
+
+        # Mark the build as finished
+        build = Build.objects.first()
+        build.state = 'finished'
+        build.save()
+        build.refresh_from_db()
+
+        trigger_build(self.project, self.version, commit='a1b2c3')
+        self.assertEqual(Build.objects.count(), 2)
+        build = Build.objects.first()
+        self.assertEqual(build.state, 'triggered')
+        self.assertIsNone(build.status_code)
+
+    def test_trigger_duplicated_build_by_version(self, update_docs_task):
+        """
+        Trigger a build for the same version.
+
+        The second build should be marked as NOOP if there is already a build
+        for the same project and version on 'triggered' state.
+        """
+        self.assertEqual(Build.objects.count(), 0)
+        trigger_build(self.project, self.version, commit=None)
+        self.assertEqual(Build.objects.count(), 1)
+        build = Build.objects.first()
+        self.assertEqual(build.state, 'triggered')
+
+        trigger_build(self.project, self.version, commit=None)
+        self.assertEqual(Build.objects.count(), 2)
+        build = Build.objects.first()
+        self.assertEqual(build.error, DuplicatedBuildError.message)
+        self.assertEqual(build.success, False)
+        self.assertEqual(build.exit_code, DuplicatedBuildError.exit_code)
+        self.assertEqual(build.status_code, DuplicatedBuildError.status_code)
+        self.assertEqual(build.state, 'finished')
+
+    def test_trigger_duplicated_non_triggered_build_by_version(self, update_docs_task):
+        """
+        Trigger a build for the same version.
+
+        The second build should not be marked as NOOP because the previous build
+        for the same project and version is on 'building' state (any non 'triggered')
+        """
+        self.assertEqual(Build.objects.count(), 0)
+        trigger_build(self.project, self.version, commit=None)
+        self.assertEqual(Build.objects.count(), 1)
+        build = Build.objects.first()
+        self.assertEqual(build.state, 'triggered')
+
+        # Mark the build as building
+        build = Build.objects.first()
+        build.state = 'building'
+        build.save()
+        build.refresh_from_db()
+
+        trigger_build(self.project, self.version, commit=None)
+        self.assertEqual(Build.objects.count(), 2)
+        build = Build.objects.first()
+        self.assertEqual(build.state, 'triggered')
+        self.assertIsNone(build.status_code)