launchpad-reviewers team mailing list archive

[Colin Watson <mp+419665@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:test-status-after-build-race into launchpad:master.

Commit message:
Fix race in TestWorker.test_status_after_build

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/419665

`TestWorker.test_status_after_build` sends a build command to an external launchpad-buildd worker and inspects the resulting status.  However, the build it dispatches will quickly fail (because we don't have quite a full builder environment here, and nor do we have full control over the external worker's event loop), and so depending on timing we can end up seeing the status as either `BUILDING` or `WAITING`.  The important thing here is that a build command dispatches a build, so accept either possibility.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:test-status-after-build-race into launchpad:master.

diff --git a/lib/lp/buildmaster/tests/test_interactor.py b/lib/lp/buildmaster/tests/test_interactor.py
index 9aa025c..d4f22fd 100644
--- a/lib/lp/buildmaster/tests/test_interactor.py
+++ b/lib/lp/buildmaster/tests/test_interactor.py
@@ -562,15 +562,22 @@ class TestWorker(TestCase):
     @defer.inlineCallbacks
     def test_status_after_build(self):
         # Calling 'status' returns the current status of the worker.  After a
-        # build has been triggered, the status is BUILDING.
+        # build has been triggered, the status is BUILDING.  (It may also be
+        # WAITING if the build finishes before we have a chance to check its
+        # status.)
         worker = self.worker_helper.getClientWorker()
         build_id = 'status-build-id'
         response = yield self.worker_helper.triggerGoodBuild(worker, build_id)
         self.assertEqual([BuilderStatus.BUILDING, build_id], response)
         status = yield worker.status()
-        self.assertEqual(BuilderStatus.BUILDING, status['builder_status'])
+        self.assertIn(
+            status['builder_status'],
+            {BuilderStatus.BUILDING, BuilderStatus.WAITING})
         self.assertEqual(build_id, status['build_id'])
-        self.assertIsInstance(status['logtail'], xmlrpc.client.Binary)
+        # We only see a logtail if the build is still in the BUILDING
+        # status.
+        if 'logtail' in status:
+            self.assertIsInstance(status['logtail'], xmlrpc.client.Binary)
 
     @defer.inlineCallbacks
     def test_ensurepresent_not_there(self):