launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/builds-without-component-0 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/builds-without-component-0/+merge/85400

Do not assume that the build has a component.

    Launchpad bug: https://bugs.launchpad.net/bugs/890103
    Pre-implementation: No one

The build +index view can oops when the template tries to get the
component's name. Builds should have comments, but there are historic
builds in production data that are missing components. The
BinaryPackageBuild's current_component states that it can return None
in cases where the publishing history is missing information. The
view and template must handle this case.

--------------------------------------------------------------------

RULES

    * Change the view to return only the component name and update the
      template to use the component_name
    * Update the view to check of the current_component is None and
      return 'Unknown' for the case.


QA

    * Visit https://qastaing.launchpad.net/ubuntu/+source/sugarcrm/4.5.0h-0ubuntu2/+build/434541/+index
    * Verify the page loads.


LINT

    lib/lp/soyuz/browser/build.py
    lib/lp/soyuz/browser/tests/test_build_views.py
    lib/lp/soyuz/templates/build-index.pt



TEST

    ./bin/test -vv -t  .*_component lp.soyuz.browser.tests.test_build_views


IMPLEMENTATION

Renamed BuildView.component to BuildView.component_name and make it return
the name so that the view does not think. Change the method to check if
the component is None and return Unknown.
    lib/lp/soyuz/browser/build.py
    lib/lp/soyuz/browser/tests/test_build_views.py
    lib/lp/soyuz/templates/build-index.pt
-- 
https://code.launchpad.net/~sinzui/launchpad/builds-without-component-0/+merge/85400
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/builds-without-component-0 into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py	2011-12-02 14:33:58 +0000
+++ lib/lp/soyuz/browser/build.py	2011-12-12 22:00:31 +0000
@@ -302,8 +302,14 @@
         return self.context.buildqueue_record
 
     @cachedproperty
-    def component(self):
-        return self.context.current_component
+    def component_name(self):
+        # Production has some buggy historic builds without
+        # source publications.
+        component = self.context.current_component
+        if component is not None:
+            return component.name
+        else:
+            return 'Unknown'
 
     @cachedproperty
     def files(self):

=== modified file 'lib/lp/soyuz/browser/tests/test_build_views.py'
--- lib/lp/soyuz/browser/tests/test_build_views.py	2011-11-03 10:58:26 +0000
+++ lib/lp/soyuz/browser/tests/test_build_views.py	2011-12-12 22:00:31 +0000
@@ -56,6 +56,25 @@
                 (build, self.empty_request), name="+index")
             self.assertEquals(build_view.user_can_retry_build, expected)
 
+    def test_view_with_component(self):
+        # Production has some buggy builds without source publications.
+        # current_component used by the view returns None in that case.
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+        removeSecurityProxy(archive).require_virtualized = False
+        build = self.factory.makeBinaryPackageBuild(archive=archive)
+        view = create_initialized_view(build, name="+index")
+        self.assertIsNot('Universe', view.component_name)
+
+    def test_view_without_component(self):
+        # Production has some buggy builds without source publications.
+        # current_component used by the view returns None in that case.
+        spph = self.factory.makeSourcePackagePublishingHistory()
+        other_das = self.factory.makeDistroArchSeries()
+        build = spph.sourcepackagerelease.createBuild(
+            other_das, PackagePublishingPocket.RELEASE, spph.archive)
+        view = create_initialized_view(build, name="+index")
+        self.assertIs('Unknown', view.component_name)
+
     def test_build_menu_primary(self):
         # The menu presented in the build page depends on the targeted
         # archive. For instance the 'PPA' action-menu link is not enabled

=== modified file 'lib/lp/soyuz/templates/build-index.pt'
--- lib/lp/soyuz/templates/build-index.pt	2011-11-01 18:14:55 +0000
+++ lib/lp/soyuz/templates/build-index.pt	2011-12-12 22:00:31 +0000
@@ -118,7 +118,7 @@
       </dl>
       <dl>
         <dt>Component:</dt>
-          <dd><span tal:replace="view/component/name">main</span></dd>
+          <dd><span tal:replace="view/component_name">main</span></dd>
       </dl>
     </div>
   </metal:macro>