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

@@ -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):
         """

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: {

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()