Merge lp:~abentley/launchpad/better-find-missing-ready-error into lp:launchpad

[Aaron Bentley]

More detailed failures in test_find_missing_ready

= Summary =
Add more detail to test_find_missing_ready to help diagnose failures.

== Proposed fix ==
Instead of re-calling list_queues for extra info, use the exact same data as find_missing_ready used.

== Pre-implementation notes ==
Discussed with Deryck

== LOC Rationale ==
I have a LOC credit of 1888

== Implementation details ==
Convert find_missing_ready to FindMissingReady, so that we examine its instance variables.

== Tests ==
bin/test -t test_find_missing_ready

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/job/celeryjob.py
  lib/lp/services/job/tests/test_celeryjob.py

[Abel Deuring (adeuring)]

looks good, just a minor nitpick: I think that

+ from lp.services.job.celeryjob import FindMissingReady

could be moved to the start f the method definition.

review: Approve (code)

=== modified file 'lib/lp/services/job/celeryjob.py'
--- lib/lp/services/job/celeryjob.py	2012-07-26 14:31:45 +0000
+++ lib/lp/services/job/celeryjob.py	2012-08-02 15:48:23 +0000
@@ -71,14 +71,25 @@
     ignore_result = True
 
 
+class FindMissingReady:
+
+    def __init__(self, job_source):
+        from lp.services.job.celeryjob import CeleryRunJob
+        from lazr.jobrunner.celerytask import list_queued
+        self.job_source = job_source
+        self.queue_contents = list_queued(CeleryRunJob.app,
+                                          [job_source.task_queue])
+        self.queued_job_ids = set(task[1][0][0] for task in
+                                  self.queue_contents)
+
+    def find_missing_ready(self):
+        return [job for job in self.job_source.iterReady()
+                if job.job_id not in self.queued_job_ids]
+
+
 def find_missing_ready(job_source):
     """Find ready jobs that are not queued."""
-    from lp.services.job.celeryjob import CeleryRunJob
-    from lazr.jobrunner.celerytask import list_queued
-    queued_job_ids = set(task[1][0][0] for task in list_queued(
-        CeleryRunJob.app, [job_source.task_queue]))
-    return [job for job in job_source.iterReady() if job.job_id not in
-            queued_job_ids]
+    return FindMissingReady(job_source).find_missing_ready()
 
 
 class RunMissingReady(Task):

=== modified file 'lib/lp/services/job/tests/test_celeryjob.py'
--- lib/lp/services/job/tests/test_celeryjob.py	2012-08-01 15:06:50 +0000
+++ lib/lp/services/job/tests/test_celeryjob.py	2012-08-02 15:48:23 +0000
@@ -4,7 +4,11 @@
 from cStringIO import StringIO
 import sys
 from time import sleep
+
+from testtools.content import Content
+from testtools.content_type import UTF8_TEXT
 from lazr.jobrunner.bin.clear_queues import clear_queues
+
 from lp.code.model.branchjob import BranchScanJob
 from lp.scripts.helpers import TransactionFreeOperation
 from lp.services.features.testing import FeatureFixture
@@ -53,24 +57,37 @@
         self.fail('Queue size did not reach %d; still at %d' %
                   (expected_len, actual_len))
 
+    def addTextDetail(self, name, text):
+        content = Content(UTF8_TEXT, lambda: text)
+        self.addDetail(name, content)
+
     def test_find_missing_ready(self):
         """A job which is ready but not queued is "missing"."""
+        from lp.services.job.celeryjob import FindMissingReady
         job = self.createMissingJob()
+        self.addTextDetail(
+            'job_info', 'job.id: %d, job.job_id: %d' % (job.id, job.job_id))
         self.assertQueueSize(self.CeleryRunJob.app,
                              [BranchScanJob.task_queue], 0)
         self.assertEqual([job], self.find_missing_ready(BranchScanJob))
         job.runViaCelery()
         self.assertQueueSize(self.CeleryRunJob.app,
                              [BranchScanJob.task_queue], 1)
-        #self.assertEqual([], self.find_missing_ready(BranchScanJob))
         # XXX AaronBentley: 2012-08-01 bug=1031018: Extra diagnostic info to
         # help diagnose this hard-to-reproduce failure.
-        if self.find_missing_ready(BranchScanJob) != []:
-            from lazr.jobrunner.celerytask import list_queued
-            contents = list_queued(
-                self.CeleryRunJob.app, [BranchScanJob.task_queue])
-            self.fail('queue: %r, job.id: %d, job.job_id: %d' %
-                      (contents, job.id, job.job_id))
+        find_missing_ready_obj = FindMissingReady(BranchScanJob)
+        missing_ready = find_missing_ready_obj.find_missing_ready()
+        try:
+            self.assertEqual([], missing_ready)
+        except:
+            self.addTextDetail('queued_job_ids',
+                               '%r' % (find_missing_ready_obj.queued_job_ids))
+            self.addTextDetail('queue_contents',
+                               repr(find_missing_ready_obj.queue_contents))
+            self.addTextDetail(
+                'missing_ready_job_id', repr([missing.job_id for missing in
+                missing_ready]))
+            raise
         drain_celery_queues()
         self.assertQueueSize(self.CeleryRunJob.app,
                              [BranchScanJob.task_queue], 0)