Fix pending binary builds (infra) (#897)

* Fix pending binary builds

* Rolled back comment update

* New test for unknown buildstatus

* Motivate time sleep everywhere

tools/release/lp_build_monitor_recipe.py

@@ -68,6 +68,7 @@ def wait_every_source_build_started(build_recipe: LPSourcePackageRecipe):
                 "  ",
             )
         )
+        # avoid flooding LP with requests
         time.sleep(LP_POLLING_DELAY)
         pending_builds = build_recipe.getPendingBuildInfo()
 
@@ -91,9 +92,14 @@ def get_all_binary_builds(
     builds = build_recipe.daily_build_archive.getBuildRecords(
         source_name=recipe_target
     )
+
+    # date_first_dispatched is filled in once a build is dispatched
+    # and is the actual start time, it can be None if the build is
+    # still waiting to get picked up by a builder
     return list(
         itertools.takewhile(
-            lambda build: build.date_first_dispatched > started_datetime,
+            lambda build: build.date_first_dispatched is None
+            or build.date_first_dispatched >= started_datetime,
             builds,
         )
     )
@@ -111,7 +117,7 @@ def get_all_source_builds(
     # need
     return list(
         itertools.takewhile(
-            lambda build: build.date_first_dispatched > started_datetime,
+            lambda build: build.date_first_dispatched >= started_datetime,
             build_recipe.builds,
         )
     )
@@ -196,15 +202,30 @@ def monitor_retry_builds(builds_to_check: list[LPBuild]) -> list[LPBuild]:
             "Uploading build",
             "Gathering build output",
         ]:
+            print(f"Build ongoing with status '{buildstate}'")
+            print(f"  weblink: {build.web_link}")
             # avoid flooding LP with requests
             time.sleep(LP_POLLING_DELAY)
             builds_to_check.insert(0, build)
-            print(f"Build ongoing with status '{buildstate}'")
+        elif buildstate not in [
+            "Failed to build",
+            "Dependency wait",
+            "Chroot problem",
+            "Failed to upload",
+            "Build for superseded Source",
+            "Cancelling build",
+            "Cancelled build",
+        ]:
+            print(f"Unknown build status '{buildstate}'")
             print(f"  weblink: {build.web_link}")
-        elif build.can_be_retried:
+            # avoid flooding LP with requests
             time.sleep(LP_POLLING_DELAY)
+            builds_to_check.insert(0, build)
+        elif build.can_be_retried:
             print(f"Build failed with status '{buildstate}'")
             print(f"  retrying: {build.web_link}")
+            # avoid flooding LP with requests
+            time.sleep(LP_POLLING_DELAY)
             build.retry()
             builds_to_check.insert(0, build)
         else:

tools/release/test_lp_build_monitor_recipe.py

@@ -1,6 +1,6 @@
 import unittest
 
-from unittest.mock import MagicMock, patch, call
+from unittest.mock import MagicMock, patch
 
 import lp_build_monitor_recipe
 
@@ -14,18 +14,25 @@ def test_get_all_binary_builds(self):
         build_selected = MagicMock()
         build_selected.date_first_dispatched = 10
 
+        build_selected_pending = MagicMock()
+        # pending builds have a None date_first_dispatched
+        # we select them because they are "newer" of the start date
+        # for sure given that they didn't even start yet
+        build_selected_pending.date_first_dispatched = None
+
         build_not_selected = MagicMock()
         build_not_selected.date_first_dispatched = 0
 
         build_recipe.daily_build_archive.getBuildRecords.return_value = [
             build_selected,
+            build_selected_pending,
             build_not_selected,
         ]
         selected = lp_build_monitor_recipe.get_all_binary_builds(
             build_recipe, 6
         )
 
-        self.assertEqual(selected, [build_selected])
+        self.assertEqual(selected, [build_selected, build_selected_pending])
         build_recipe.daily_build_archive.getBuildRecords.assert_called_with(
             source_name="checkbox-ng"
         )
@@ -137,6 +144,29 @@ def lp_refresh_side_effect():
         # each time a failure was detected, the build was retried
         self.assertEqual(build_mock.retry.call_count, 3)
 
+    @patch("time.sleep")
+    def test_monitor_retry_builds_robust(self, time_sleep_mock):
+        build_mock = MagicMock()
+        # A build is updated via the lp_refresh function, lets do the same
+        # here but inject our test values
+        build_status_evolution = [
+            "Successfully built",
+            None # this may be possible for pending builds
+        ]
+
+        def lp_refresh_side_effect():
+            if build_status_evolution:
+                build_mock.buildstate = build_status_evolution.pop()
+
+        build_mock.lp_refresh.side_effect = lp_refresh_side_effect
+
+        lp_build_monitor_recipe.monitor_retry_builds([build_mock])
+
+        # we updated till the build reported a success
+        self.assertEqual(build_mock.lp_refresh.call_count, 2)
+        # we didnt fload LP with requests, waiting once per progress
+        self.assertEqual(time_sleep_mock.call_count, 1)
+
     @patch("time.sleep")
     def test_monitor_retry_builds_more_wait(self, time_sleep_mock):
         build_mock = MagicMock()