From 132c5b80cbb130064f7b1dca19ef2c4767989563 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 18 Mar 2021 08:09:58 -0700 Subject: [PATCH 1/7] Make Build models default to `triggered` I'm not sure why we were defaulting them to Finished, but this should help not confuse the badge code, and just be more correct in general. --- .../migrations/0034_build_default_triggered.py | 18 ++++++++++++++++++ readthedocs/builds/models.py | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 readthedocs/builds/migrations/0034_build_default_triggered.py diff --git a/readthedocs/builds/migrations/0034_build_default_triggered.py b/readthedocs/builds/migrations/0034_build_default_triggered.py new file mode 100644 index 00000000000..22f98f5811a --- /dev/null +++ b/readthedocs/builds/migrations/0034_build_default_triggered.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.17 on 2021-03-18 15:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0033_dont_cascade_delete_builds'), + ] + + operations = [ + migrations.AlterField( + model_name='build', + name='state', + field=models.CharField(choices=[('triggered', 'Triggered'), ('cloning', 'Cloning'), ('installing', 'Installing'), ('building', 'Building'), ('uploading', 'Uploading'), ('finished', 'Finished')], default='triggered', max_length=55, verbose_name='State'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 34a7fee2c29..d449070ea4e 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -607,7 +607,7 @@ class Build(models.Model): _('State'), max_length=55, choices=BUILD_STATE, - default='finished', + default=BUILD_STATE_TRIGGERED, db_index=True, ) From 181f9b72b1af097092eea8b5806d06e23f21981b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Thu, 17 Feb 2022 12:05:00 -0300 Subject: [PATCH 2/7] Rename migration to match the number --- ...ild_default_triggered.py => 0040_build_default_triggered.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename readthedocs/builds/migrations/{0034_build_default_triggered.py => 0040_build_default_triggered.py} (90%) diff --git a/readthedocs/builds/migrations/0034_build_default_triggered.py b/readthedocs/builds/migrations/0040_build_default_triggered.py similarity index 90% rename from readthedocs/builds/migrations/0034_build_default_triggered.py rename to readthedocs/builds/migrations/0040_build_default_triggered.py index 22f98f5811a..e88e6fdc525 100644 --- a/readthedocs/builds/migrations/0034_build_default_triggered.py +++ b/readthedocs/builds/migrations/0040_build_default_triggered.py @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ('builds', '0033_dont_cascade_delete_builds'), + ('builds', '0039_migrate_config_data.py'), ] operations = [ From 635afc8ced668381156f677e29642d466f440327 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 26 Jan 2023 10:50:25 -0500 Subject: [PATCH 3/7] Black --- .../0040_build_default_triggered.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/readthedocs/builds/migrations/0040_build_default_triggered.py b/readthedocs/builds/migrations/0040_build_default_triggered.py index e88e6fdc525..06f59edd135 100644 --- a/readthedocs/builds/migrations/0040_build_default_triggered.py +++ b/readthedocs/builds/migrations/0040_build_default_triggered.py @@ -6,13 +6,25 @@ class Migration(migrations.Migration): dependencies = [ - ('builds', '0039_migrate_config_data.py'), + ("builds", "0039_migrate_config_data.py"), ] operations = [ migrations.AlterField( - model_name='build', - name='state', - field=models.CharField(choices=[('triggered', 'Triggered'), ('cloning', 'Cloning'), ('installing', 'Installing'), ('building', 'Building'), ('uploading', 'Uploading'), ('finished', 'Finished')], default='triggered', max_length=55, verbose_name='State'), + model_name="build", + name="state", + field=models.CharField( + choices=[ + ("triggered", "Triggered"), + ("cloning", "Cloning"), + ("installing", "Installing"), + ("building", "Building"), + ("uploading", "Uploading"), + ("finished", "Finished"), + ], + default="triggered", + max_length=55, + verbose_name="State", + ), ), ] From ea800eda03e100c38310d4d609a302bf2ac07b06 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 26 Jan 2023 10:58:31 -0500 Subject: [PATCH 4/7] Update migration --- ...ild_default_triggered.py => 0047_build_default_triggered.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename readthedocs/builds/migrations/{0040_build_default_triggered.py => 0047_build_default_triggered.py} (93%) diff --git a/readthedocs/builds/migrations/0040_build_default_triggered.py b/readthedocs/builds/migrations/0047_build_default_triggered.py similarity index 93% rename from readthedocs/builds/migrations/0040_build_default_triggered.py rename to readthedocs/builds/migrations/0047_build_default_triggered.py index 06f59edd135..9734a2969cb 100644 --- a/readthedocs/builds/migrations/0040_build_default_triggered.py +++ b/readthedocs/builds/migrations/0047_build_default_triggered.py @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ("builds", "0039_migrate_config_data.py"), + ("builds", "0046_identifier_null"), ] operations = [ From 3def401710c1995583bf819744cd4f7a6bd645f5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 26 Jan 2023 17:26:58 -0500 Subject: [PATCH 5/7] Fix tests --- readthedocs/builds/models.py | 4 +- readthedocs/builds/tests/test_views.py | 5 +- readthedocs/rtd_tests/tests/test_api.py | 28 +++++--- .../rtd_tests/tests/test_project_views.py | 68 ++++++++++++++----- .../rtd_tests/tests/test_version_config.py | 11 ++- 5 files changed, 87 insertions(+), 29 deletions(-) diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index f349c0a59f6..86af1ba9c14 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -290,7 +290,9 @@ def config(self): .only('_config') .first() ) - return last_build.config + if last_build: + return last_build.config + return None @property def commit_name(self): diff --git a/readthedocs/builds/tests/test_views.py b/readthedocs/builds/tests/test_views.py index b15399b7482..65469d0ad32 100644 --- a/readthedocs/builds/tests/test_views.py +++ b/readthedocs/builds/tests/test_views.py @@ -68,7 +68,10 @@ def test_cancel_build_from_another_project(self, app): another_user = get(User) another_project = self._get_project(owners=[another_user]) another_build = get( - Build, project=another_project, version=another_project.versions.first() + Build, + project=another_project, + version=another_project.versions.first(), + state=BUILD_STATE_INSTALLING, ) self.client.force_login(another_user) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index 17379205894..de34a19f104 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -41,6 +41,7 @@ from readthedocs.api.v2.views.task_views import get_status_data from readthedocs.builds.constants import ( BUILD_STATE_CLONING, + BUILD_STATE_FINISHED, BUILD_STATE_TRIGGERED, EXTERNAL, EXTERNAL_VERSION_STATE_CLOSED, @@ -510,17 +511,23 @@ def test_make_build_commands(self): def test_get_raw_log_success(self): project = Project.objects.get(pk=1) version = project.versions.first() - build = get(Build, project=project, version=version, builder='foo') + build = get( + Build, + project=project, + version=version, + builder="foo", + state=BUILD_STATE_FINISHED, + ) get( BuildCommandResult, build=build, - command='python setup.py install', - output='Installing dependencies...', + command="python setup.py install", + output="Installing dependencies...", ) get( BuildCommandResult, build=build, - command='git checkout master', + command="git checkout master", output='Switched to branch "master"', ) client = APIClient() @@ -598,14 +605,19 @@ def test_get_raw_log_failure(self): project = Project.objects.get(pk=1) version = project.versions.first() build = get( - Build, project=project, version=version, - builder='foo', success=False, exit_code=1, + Build, + project=project, + version=version, + builder="foo", + success=False, + exit_code=1, + state=BUILD_STATE_FINISHED, ) get( BuildCommandResult, build=build, - command='python setup.py install', - output='Installing dependencies...', + command="python setup.py install", + output="Installing dependencies...", exit_code=1, ) get( diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index 2ec080e8a42..a9f84001b34 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -8,7 +8,7 @@ from django.views.generic.base import ContextMixin from django_dynamic_fixture import get, new -from readthedocs.builds.constants import EXTERNAL +from readthedocs.builds.constants import BUILD_STATE_FINISHED, EXTERNAL from readthedocs.builds.models import Build, Version from readthedocs.integrations.models import GenericAPIWebhook, GitHubWebhook from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation @@ -517,28 +517,56 @@ def test_badge_caching(self): self.assertTrue('no-cache' in res['Cache-Control']) def test_passing_badge(self): - get(Build, project=self.project, version=self.version, success=True) - res = self.client.get(self.badge_url, {'version': self.version.slug}) - self.assertContains(res, 'passing') - self.assertEqual(res['Content-Type'], 'image/svg+xml') + get( + Build, + project=self.project, + version=self.version, + success=True, + state=BUILD_STATE_FINISHED, + ) + res = self.client.get(self.badge_url, {"version": self.version.slug}) + self.assertContains(res, "passing") + self.assertEqual(res["Content-Type"], "image/svg+xml") def test_failing_badge(self): - get(Build, project=self.project, version=self.version, success=False) - res = self.client.get(self.badge_url, {'version': self.version.slug}) - self.assertContains(res, 'failing') + get( + Build, + project=self.project, + version=self.version, + success=False, + state=BUILD_STATE_FINISHED, + ) + res = self.client.get(self.badge_url, {"version": self.version.slug}) + self.assertContains(res, "failing") def test_plastic_failing_badge(self): - get(Build, project=self.project, version=self.version, success=False) - res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'plastic'}) - self.assertContains(res, 'failing') + get( + Build, + project=self.project, + version=self.version, + success=False, + state=BUILD_STATE_FINISHED, + ) + res = self.client.get( + self.badge_url, {"version": self.version.slug, "style": "plastic"} + ) + self.assertContains(res, "failing") # The plastic badge has slightly more rounding self.assertContains(res, 'rx="4"') def test_social_passing_badge(self): - get(Build, project=self.project, version=self.version, success=True) - res = self.client.get(self.badge_url, {'version': self.version.slug, 'style': 'social'}) - self.assertContains(res, 'passing') + get( + Build, + project=self.project, + version=self.version, + success=True, + state=BUILD_STATE_FINISHED, + ) + res = self.client.get( + self.badge_url, {"version": self.version.slug, "style": "social"} + ) + self.assertContains(res, "passing") # The social badge (but not the other badges) has this element self.assertContains(res, 'rlink') @@ -556,9 +584,15 @@ def test_private_version(self): self.version.save() # Without a token, badge is unknown - get(Build, project=self.project, version=self.version, success=True) - res = self.client.get(self.badge_url, {'version': self.version.slug}) - self.assertContains(res, 'unknown') + get( + Build, + project=self.project, + version=self.version, + success=True, + state=BUILD_STATE_FINISHED, + ) + res = self.client.get(self.badge_url, {"version": self.version.slug}) + self.assertContains(res, "unknown") # With an invalid token, the badge is unknown res = self.client.get( diff --git a/readthedocs/rtd_tests/tests/test_version_config.py b/readthedocs/rtd_tests/tests/test_version_config.py index cbae0dc1d7e..015b4b3a09f 100644 --- a/readthedocs/rtd_tests/tests/test_version_config.py +++ b/readthedocs/rtd_tests/tests/test_version_config.py @@ -1,6 +1,7 @@ from django.test import TestCase from django_dynamic_fixture import get +from readthedocs.builds.constants import BUILD_STATE_BUILDING, BUILD_STATE_FINISHED from readthedocs.builds.models import Build, Version from readthedocs.projects.models import Project @@ -16,23 +17,26 @@ def test_get_correct_config(self): project=self.project, version=self.version, _config={'version': 1}, + state=BUILD_STATE_FINISHED, ) build_new = Build.objects.create( project=self.project, version=self.version, _config={'version': 2}, + state=BUILD_STATE_FINISHED, ) build_new_error = Build.objects.create( project=self.project, version=self.version, _config={'version': 3}, success=False, + state=BUILD_STATE_FINISHED, ) build_new_unfinish = Build.objects.create( project=self.project, version=self.version, _config={'version': 4}, - state='building', + state=BUILD_STATE_BUILDING, ) self.assertEqual(self.version.config, {'version': 2}) @@ -42,6 +46,7 @@ def test_get_correct_config_when_same_config(self): project=self.project, version=self.version, _config={}, + state=BUILD_STATE_FINISHED, ) build_old.config = {'version': 1} build_old.save() @@ -51,6 +56,7 @@ def test_get_correct_config_when_same_config(self): project=self.project, version=self.version, _config={}, + state=BUILD_STATE_FINISHED, ) build_new.config = {'version': 1} build_new.save() @@ -61,6 +67,7 @@ def test_get_correct_config_when_same_config(self): version=self.version, _config={}, success=False, + state=BUILD_STATE_FINISHED, ) build_new_error.config = {'version': 3} build_new_error.save() @@ -70,7 +77,7 @@ def test_get_correct_config_when_same_config(self): project=self.project, version=self.version, _config={}, - state='building', + state=BUILD_STATE_BUILDING, ) build_new_unfinish.config = {'version': 1} build_new_unfinish.save() From 54b9fab7113ec5af8ec6c1e5a4a148204c2b6725 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 26 Jan 2023 18:21:35 -0500 Subject: [PATCH 6/7] Update migration --- readthedocs/builds/migrations/0047_build_default_triggered.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/builds/migrations/0047_build_default_triggered.py b/readthedocs/builds/migrations/0047_build_default_triggered.py index 9734a2969cb..df5fde10796 100644 --- a/readthedocs/builds/migrations/0047_build_default_triggered.py +++ b/readthedocs/builds/migrations/0047_build_default_triggered.py @@ -21,6 +21,7 @@ class Migration(migrations.Migration): ("building", "Building"), ("uploading", "Uploading"), ("finished", "Finished"), + ("cancelled", "Cancelled"), ], default="triggered", max_length=55, From b089ec05441d2f5c379126f5b8bb22884204d2f7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 26 Jan 2023 18:47:44 -0500 Subject: [PATCH 7/7] Update migration again --- readthedocs/builds/migrations/0047_build_default_triggered.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/builds/migrations/0047_build_default_triggered.py b/readthedocs/builds/migrations/0047_build_default_triggered.py index df5fde10796..83bd7e1429e 100644 --- a/readthedocs/builds/migrations/0047_build_default_triggered.py +++ b/readthedocs/builds/migrations/0047_build_default_triggered.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.17 on 2021-03-18 15:09 +# Generated by Django 3.2.16 on 2023-01-26 23:46 from django.db import migrations, models @@ -23,6 +23,7 @@ class Migration(migrations.Migration): ("finished", "Finished"), ("cancelled", "Cancelled"), ], + db_index=True, default="triggered", max_length=55, verbose_name="State",