Merge lp:~cjwatson/launchpad/api-build-builder into lp:launchpad

[Colin Watson]

Export the builder property on builds.

== Summary ==

As I noted in bug 975579, it's annoying that the API doesn't expose which builder was used for a given build.

== Proposed fix ==

Export it.

== Implementation details ==

I tidied up some tests a bit to get my LoC delta to zero. Along the way I happened to notice that one of the tests in TestBinaryPackageBuild.test_queueBuild was useless (failUnless when it should have been failUnlessEqual, or better assertEqual), so I fixed that.

== Tests ==

bin/test -vvct test_binarypackagebuild

== Demo and Q/A ==

Load the object corresponding to any successful build in the API, and check that its builder property is correct. Likewise for a build in needs-build state (builder should be None).

== lint ==

None.

[Benji York (benji)]

This branch looks good. The removal of the "fail*" assertions is a nice touch.

review: Approve (code)

=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py	2012-01-02 11:21:14 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py	2012-04-10 10:35:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -209,9 +209,10 @@
                           "is dispatched the first time and not changed in "
                           "subsequent build attempts.")))
 
-    builder = Reference(
-        title=_("Builder"), schema=IBuilder, required=False, readonly=True,
-        description=_("The builder assigned to this job."))
+    builder = exported(
+        Reference(
+            title=_("Builder"), schema=IBuilder, required=False, readonly=True,
+            description=_("The builder assigned to this job.")))
 
     buildqueue_record = Reference(
         # Really IBuildQueue, set in _schema_circular_imports to avoid

=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
--- lib/lp/soyuz/stories/webservice/xx-builds.txt	2012-01-31 01:19:45 +0000
+++ lib/lp/soyuz/stories/webservice/xx-builds.txt	2012-04-10 10:35:22 +0000
@@ -42,6 +42,7 @@
     >>> pprint_entry(builds['entries'][0])
     arch_tag: u'i386'
     archive_link: u'http://.../beta/~cprov/+archive/ppa'
+    builder_link: u'http://.../beta/builders/bob'
     can_be_cancelled: False
     can_be_rescored: False
     can_be_retried: True
@@ -74,6 +75,7 @@
     archive_link: u'http://.../~cprov/+archive/ppa'
     build_log_url:
     u'http://.../~cprov/+archive/ppa/+build/26/+files/netapplet-1.0.0.tar.gz'
+    builder_link: u'http://.../builders/bob'
     buildstate: u'Failed to build'
     can_be_cancelled: False
     can_be_rescored: False

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2012-02-09 23:09:36 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2012-04-10 10:35:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test Build features."""
@@ -83,9 +83,9 @@
         bq = self.build.queueBuild()
         self.assertProvides(bq, IBuildQueue)
         self.assertProvides(bq.specific_job, IBuildPackageJob)
-        self.failUnlessEqual(self.build.is_virtualized, bq.virtualized)
-        self.failIfEqual(None, bq.processor)
-        self.failUnless(bq, self.build.buildqueue_record)
+        self.assertEqual(self.build.is_virtualized, bq.virtualized)
+        self.assertIsNotNone(bq.processor)
+        self.assertEqual(bq, self.build.buildqueue_record)
 
     def test_getBuildCookie(self):
         # A build cookie is made up of the job type and record id.
@@ -93,7 +93,7 @@
         Store.of(self.build).flush()
         cookie = self.build.getBuildCookie()
         expected_cookie = "PACKAGEBUILD-%d" % self.build.id
-        self.assertEquals(expected_cookie, cookie)
+        self.assertEqual(expected_cookie, cookie)
 
     def test_estimateDuration(self):
         # Without previous builds, a negligable package size estimate is 60s
@@ -124,7 +124,7 @@
         # the distribution source package release when the context
         # is not a PPA or a copy archive.
         self.addFakeBuildLog()
-        self.failUnlessEqual(
+        self.assertEqual(
             'http://launchpad.dev/ubuntutest/+source/'
             'gedit/666/+build/%d/+files/mybuildlog.txt' % (
                 self.build.package_build.build_farm_job.id),
@@ -137,7 +137,7 @@
         ppa_owner = self.factory.makePerson(name="joe")
         removeSecurityProxy(self.build).archive = self.factory.makeArchive(
             owner=ppa_owner, name="myppa")
-        self.failUnlessEqual(
+        self.assertEqual(
             'http://launchpad.dev/~joe/'
             '+archive/myppa/+build/%d/+files/mybuildlog.txt' % (
                 self.build.build_farm_job.id),
@@ -150,7 +150,7 @@
         store = Store.of(build_farm_job)
         store.flush()
 
-        self.failUnlessEqual(self.build, build_farm_job.getSpecificJob())
+        self.assertEqual(self.build, build_farm_job.getSpecificJob())
 
     def test_adapt_from_build_farm_job_prefetching(self):
         # The package_build is prefetched for efficiency.
@@ -173,8 +173,7 @@
         # If getSpecificJob is called on the binary build it is a noop.
         store = Store.of(self.build)
         store.flush()
-        self.assertStatementCount(
-            0, self.build.getSpecificJob)
+        self.assertStatementCount(0, self.build.getSpecificJob)
 
     def test_getUploader(self):
         # For ACL purposes the uploader is the changes file signer.
@@ -182,7 +181,7 @@
         class MockChanges:
             signer = "Somebody <[e-mail]>"
 
-        self.assertEquals("Somebody <[e-mail]>",
+        self.assertEqual("Somebody <[e-mail]>",
             self.build.getUploader(MockChanges()))
 
     def test_can_be_cancelled(self):
@@ -301,7 +300,12 @@
         depwait_build = self._setupSimpleDepwaitContext()
         self.layer.txn.commit()
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, '')
+        self.assertEqual(depwait_build.dependencies, '')
+
+    def assertRaisesUnparsableDependencies(self, depwait_build, dependencies):
+        depwait_build.dependencies = dependencies
+        self.assertRaises(
+            UnparsableDependencies, depwait_build.updateDependencies)
 
     def testInvalidDependencies(self):
         # Calling `IBinaryPackageBuild.updateDependencies` on a build with
@@ -310,24 +314,16 @@
         depwait_build = self._setupSimpleDepwaitContext()
 
         # None is not a valid dependency values.
-        depwait_build.dependencies = None
-        self.assertRaises(
-            UnparsableDependencies, depwait_build.updateDependencies)
+        self.assertRaisesUnparsableDependencies(depwait_build, None)
 
         # Missing 'name'.
-        depwait_build.dependencies = u'(>> version)'
-        self.assertRaises(
-            UnparsableDependencies, depwait_build.updateDependencies)
+        self.assertRaisesUnparsableDependencies(depwait_build, u'(>> version)')
 
         # Missing 'version'.
-        depwait_build.dependencies = u'name (>>)'
-        self.assertRaises(
-            UnparsableDependencies, depwait_build.updateDependencies)
+        self.assertRaisesUnparsableDependencies(depwait_build, u'name (>>)')
 
-        # Missing comman between dependencies.
-        depwait_build.dependencies = u'name1 name2'
-        self.assertRaises(
-            UnparsableDependencies, depwait_build.updateDependencies)
+        # Missing comma between dependencies.
+        self.assertRaisesUnparsableDependencies(depwait_build, u'name1 name2')
 
     def testBug378828(self):
         # `IBinaryPackageBuild.updateDependencies` copes with the
@@ -344,7 +340,7 @@
 
         self.layer.txn.commit()
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, '')
+        self.assertEqual(depwait_build.dependencies, '')
 
     def testVersionedDependencies(self):
         # `IBinaryPackageBuild.updateDependencies` supports versioned
@@ -357,10 +353,10 @@
 
         depwait_build.dependencies = u'dep-bin (>> 666)'
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, u'dep-bin (>> 666)')
+        self.assertEqual(depwait_build.dependencies, u'dep-bin (>> 666)')
         depwait_build.dependencies = u'dep-bin (>= 666)'
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, u'')
+        self.assertEqual(depwait_build.dependencies, u'')
 
     def testVersionedDependencyOnOldPublication(self):
         # `IBinaryPackageBuild.updateDependencies` doesn't just consider
@@ -376,10 +372,10 @@
 
         depwait_build.dependencies = u'dep-bin (= 666)'
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, u'')
+        self.assertEqual(depwait_build.dependencies, u'')
         depwait_build.dependencies = u'dep-bin (= 999)'
         depwait_build.updateDependencies()
-        self.assertEquals(depwait_build.dependencies, u'')
+        self.assertEqual(depwait_build.dependencies, u'')
 
 
 class BaseTestCaseWithThreeBuilds(TestCaseWithFactory):
@@ -426,8 +422,7 @@
 
     def test_getByBuildFarmJob_returns_none_when_missing(self):
         sprb = self.factory.makeSourcePackageRecipeBuild()
-        self.assertIs(
-            None,
+        self.assertIsNone(
             getUtility(IBinaryPackageBuildSet).getByBuildFarmJob(
                 sprb.build_farm_job))
 
@@ -598,8 +593,7 @@
         expected = self.build.can_be_cancelled
         entry_url = api_url(self.build)
         logout()
-        entry = self.webservice.get(
-            entry_url, api_version='devel').jsonBody()
+        entry = self.webservice.get(entry_url, api_version='devel').jsonBody()
         self.assertEqual(expected, entry['can_be_cancelled'])
 
     def test_cancel_is_exported(self):
@@ -607,25 +601,30 @@
         build_url = api_url(self.build)
         self.build.queueBuild()
         logout()
-        entry = self.webservice.get(
-            build_url, api_version='devel').jsonBody()
+        entry = self.webservice.get(build_url, api_version='devel').jsonBody()
         response = self.webservice.named_post(
             entry['self_link'], 'cancel', api_version='devel')
         self.assertEqual(200, response.status)
-        entry = self.webservice.get(
-            build_url, api_version='devel').jsonBody()
+        entry = self.webservice.get(build_url, api_version='devel').jsonBody()
         self.assertEqual(BuildStatus.CANCELLED.title, entry['buildstate'])
 
     def test_cancel_security(self):
         # Check that unauthorised users cannot call cancel()
         build_url = api_url(self.build)
-        person = self.factory.makePerson()
         webservice = webservice_for_person(
-            person, permission=OAuthPermission.WRITE_PUBLIC)
+            self.factory.makePerson(), permission=OAuthPermission.WRITE_PUBLIC)
         logout()
 
-        entry = webservice.get(
-            build_url, api_version='devel').jsonBody()
+        entry = webservice.get(build_url, api_version='devel').jsonBody()
         response = webservice.named_post(
             entry['self_link'], 'cancel', api_version='devel')
         self.assertEqual(401, response.status)
+
+    def test_builder_is_exported(self):
+        # The builder property is exported.
+        removeSecurityProxy(self.build).builder = self.factory.makeBuilder()
+        build_url = api_url(self.build)
+        builder_url = api_url(self.build.builder)
+        logout()
+        entry = self.webservice.get(build_url, api_version='devel').jsonBody()
+        self.assertEndsWith(entry['builder_link'], builder_url)