launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/recipe-build-ordering into lp:launchpad.

Commit message:
Sort cancelled-before-starting recipe builds to the end of the build history.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #746140 in Launchpad itself: "Recipe superseded source builds get stuck at top of recent builds list forever"
  https://bugs.launchpad.net/launchpad/+bug/746140

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/recipe-build-ordering/+merge/340534

These queries will be a little slower until the corresponding database branch lands, but they do well enough with other indexes that we don't need to coordinate that too carefully.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/recipe-build-ordering into lp:launchpad.

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2016-10-14 16:16:18 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2018-03-02 18:02:05 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementation of the `SourcePackageRecipe` content type."""
@@ -67,7 +67,10 @@
     IMasterStore,
     IStore,
     )
-from lp.services.database.stormexpr import Greatest
+from lp.services.database.stormexpr import (
+    Greatest,
+    NullsLast,
+    )
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -330,9 +333,9 @@
     def builds(self):
         """See `ISourcePackageRecipe`."""
         order_by = (
-            Desc(Greatest(
+            NullsLast(Desc(Greatest(
                 SourcePackageRecipeBuild.date_started,
-                SourcePackageRecipeBuild.date_finished)),
+                SourcePackageRecipeBuild.date_finished))),
             Desc(SourcePackageRecipeBuild.date_created),
             Desc(SourcePackageRecipeBuild.id))
         return self._getBuilds(None, order_by)
@@ -343,9 +346,9 @@
         filter_term = (
             SourcePackageRecipeBuild.status != BuildStatus.NEEDSBUILD)
         order_by = (
-            Desc(Greatest(
+            NullsLast(Desc(Greatest(
                 SourcePackageRecipeBuild.date_started,
-                SourcePackageRecipeBuild.date_finished)),
+                SourcePackageRecipeBuild.date_finished))),
             Desc(SourcePackageRecipeBuild.id))
         return self._getBuilds(filter_term, order_by)
 

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2017-10-04 01:53:48 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2018-03-02 18:02:05 +0000
@@ -765,6 +765,20 @@
         self.assertEqual([build], list(recipe.completed_builds))
         self.assertEqual([], list(recipe.pending_builds))
 
+    def test_getBuilds_cancelled_never_started_last(self):
+        # A cancelled build that was never even started sorts to the end.
+        recipe = self.makeSourcePackageRecipe()
+        fullybuilt = self.factory.makeSourcePackageRecipeBuild(recipe=recipe)
+        instacancelled = self.factory.makeSourcePackageRecipeBuild(
+            recipe=recipe)
+        fullybuilt.updateStatus(BuildStatus.BUILDING)
+        fullybuilt.updateStatus(BuildStatus.CANCELLED)
+        instacancelled.updateStatus(BuildStatus.CANCELLED)
+        self.assertEqual([fullybuilt, instacancelled], list(recipe.builds))
+        self.assertEqual(
+            [fullybuilt, instacancelled], list(recipe.completed_builds))
+        self.assertEqual([], list(recipe.pending_builds))
+
     def test_setRecipeText_private_base_branch(self):
         source_package_recipe = self.makeSourcePackageRecipe()
         with person_logged_in(source_package_recipe.owner):