From 23d75daeb133c3e2056719b21ac96ffc3b2930e7 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 18 Mar 2024 12:38:35 +0100 Subject: [PATCH] Build: always reset the build before building (#11213) * Build: always reset the build before building Closes #11131 * Mock the APIv2 reset endpoint for testing * Update tests --- readthedocs/projects/tasks/builds.py | 12 ++++--- readthedocs/projects/tests/mockers.py | 5 +++ .../projects/tests/test_build_tasks.py | 36 ++++++++++--------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/readthedocs/projects/tasks/builds.py b/readthedocs/projects/tasks/builds.py index 4d3e38b8bb7..dea50133ce9 100644 --- a/readthedocs/projects/tasks/builds.py +++ b/readthedocs/projects/tasks/builds.py @@ -447,10 +447,14 @@ def before_start(self, task_id, args, kwargs): self._reset_build() def _reset_build(self): - # Reset build only if it has some commands already. - if self.data.build.get("commands"): - log.info("Resetting build.") - self.data.api_client.build(self.data.build["id"]).reset.post() + # Always reset the build before starting. + # We used to only reset it when it has at least one command executed already. + # However, with the introduction of the new notification system, + # it could have a notification attached (e.g. Max concurrency build) + # that needs to be removed from the build. + # See https://github.com/readthedocs/readthedocs.org/issues/11131 + log.info("Resetting build.") + self.data.api_client.build(self.data.build["id"]).reset.post() def on_failure(self, exc, task_id, args, kwargs, einfo): """ diff --git a/readthedocs/projects/tests/mockers.py b/readthedocs/projects/tests/mockers.py index 4dae7d137de..6831e036358 100644 --- a/readthedocs/projects/tests/mockers.py +++ b/readthedocs/projects/tests/mockers.py @@ -208,6 +208,11 @@ def _mock_api(self): status_code=201, ) + self.requestsmock.post( + f"{settings.SLUMBER_API_HOST}/api/v2/build/{self.build.pk}/reset/", + status_code=201, + ) + self.requestsmock.get( f"{settings.SLUMBER_API_HOST}/api/v2/build/concurrent/?project__slug={self.project.slug}", json=lambda request, context: { diff --git a/readthedocs/projects/tests/test_build_tasks.py b/readthedocs/projects/tests/test_build_tasks.py index ed1b4b9b7dd..26a2750c7e7 100644 --- a/readthedocs/projects/tests/test_build_tasks.py +++ b/readthedocs/projects/tests/test_build_tasks.py @@ -297,9 +297,9 @@ def test_build_updates_documentation_type(self, load_yaml_config): self._trigger_update_docs_task() # Update version state - assert self.requests_mock.request_history[7]._request.method == "PATCH" - assert self.requests_mock.request_history[7].path == "/api/v2/version/1/" - assert self.requests_mock.request_history[7].json() == { + assert self.requests_mock.request_history[8]._request.method == "PATCH" + assert self.requests_mock.request_history[8].path == "/api/v2/version/1/" + assert self.requests_mock.request_history[8].json() == { "addons": False, "build_data": None, "built": True, @@ -477,8 +477,12 @@ def test_successful_build( # TODO: assert the verb and the path for each API call as well + # Build reset + assert self.requests_mock.request_history[3]._request.method == "POST" + assert self.requests_mock.request_history[3].path == "/api/v2/build/1/reset/" + # Update build state: cloning - assert self.requests_mock.request_history[3].json() == { + assert self.requests_mock.request_history[4].json() == { "id": 1, "state": "cloning", "commit": "a1b2c3", @@ -487,7 +491,7 @@ def test_successful_build( } # Update build state: installing - assert self.requests_mock.request_history[4].json() == { + assert self.requests_mock.request_history[5].json() == { "id": 1, "state": "installing", "commit": "a1b2c3", @@ -550,7 +554,7 @@ def test_successful_build( }, } # Update build state: building - assert self.requests_mock.request_history[5].json() == { + assert self.requests_mock.request_history[6].json() == { "id": 1, "state": "building", "commit": "a1b2c3", @@ -560,7 +564,7 @@ def test_successful_build( "error": "", } # Update build state: uploading - assert self.requests_mock.request_history[6].json() == { + assert self.requests_mock.request_history[7].json() == { "id": 1, "state": "uploading", "commit": "a1b2c3", @@ -570,9 +574,9 @@ def test_successful_build( "error": "", } # Update version state - assert self.requests_mock.request_history[7]._request.method == "PATCH" - assert self.requests_mock.request_history[7].path == "/api/v2/version/1/" - assert self.requests_mock.request_history[7].json() == { + assert self.requests_mock.request_history[8]._request.method == "PATCH" + assert self.requests_mock.request_history[8].path == "/api/v2/version/1/" + assert self.requests_mock.request_history[8].json() == { "addons": False, "build_data": None, "built": True, @@ -582,11 +586,11 @@ def test_successful_build( "has_htmlzip": True, } # Set project has valid clone - assert self.requests_mock.request_history[8]._request.method == "PATCH" - assert self.requests_mock.request_history[8].path == "/api/v2/project/1/" - assert self.requests_mock.request_history[8].json() == {"has_valid_clone": True} + assert self.requests_mock.request_history[9]._request.method == "PATCH" + assert self.requests_mock.request_history[9].path == "/api/v2/project/1/" + assert self.requests_mock.request_history[9].json() == {"has_valid_clone": True} # Update build state: finished, success and builder - assert self.requests_mock.request_history[9].json() == { + assert self.requests_mock.request_history[10].json() == { "id": 1, "state": "finished", "commit": "a1b2c3", @@ -598,8 +602,8 @@ def test_successful_build( "error": "", } - assert self.requests_mock.request_history[10]._request.method == "POST" - assert self.requests_mock.request_history[10].path == "/api/v2/revoke/" + assert self.requests_mock.request_history[11]._request.method == "POST" + assert self.requests_mock.request_history[11].path == "/api/v2/revoke/" assert BuildData.objects.all().exists()