launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/same-queue-query-fmr into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~abentley/launchpad/same-queue-query-fmr/+merge/118428

= Summary =
test_find_misssing_ready seems to fail because the contents of the queue vary between queries.

== Proposed fix ==
When the queue has been determined to be in the correct state, use that query directly instead of re-querying the queue.

== Pre-implementation notes ==
Discussed with deryck

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

== Implementation details ==
Implement getFMR, as a variant of assertQueueSize that uses FindMissingReady to query the queue, and returns the instance that had a successful query.

== 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/tests/test_celeryjob.py
-- 
https://code.launchpad.net/~abentley/launchpad/same-queue-query-fmr/+merge/118428
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/same-queue-query-fmr into lp:launchpad.

=== modified file 'lib/lp/services/job/tests/test_celeryjob.py'
--- lib/lp/services/job/tests/test_celeryjob.py	2012-08-02 15:44:50 +0000
+++ lib/lp/services/job/tests/test_celeryjob.py	2012-08-06 20:58:22 +0000
@@ -42,6 +42,20 @@
         self.addCleanup(drain_celery_queues)
         return job
 
+    def getFMR(self, job_source, expected_len):
+        """Get a FindMissingReady object when job queue reaches expected size.
+
+        Fails if the queue never reaches the expected size.
+        """
+        from lp.services.job.celeryjob import FindMissingReady
+        for x in range(600):
+            find_missing = FindMissingReady(job_source)
+            if len(find_missing.queue_contents) == expected_len:
+                return find_missing
+            sleep(0.1)
+        self.fail('Queue size did not reach %d; still at %d' %
+                  (expected_len, len(find_missing.queue_contents)))
+
     def assertQueueSize(self, app, queues, expected_len):
         """Assert the message queue (eventually) reaches the specified size.
 
@@ -63,23 +77,19 @@
 
     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))
+        find_missing_ready_obj = self.getFMR(BranchScanJob, 0)
+        self.assertEqual([job], find_missing_ready_obj.find_missing_ready())
         job.runViaCelery()
-        self.assertQueueSize(self.CeleryRunJob.app,
-                             [BranchScanJob.task_queue], 1)
-        # XXX AaronBentley: 2012-08-01 bug=1031018: Extra diagnostic info to
-        # help diagnose this hard-to-reproduce failure.
-        find_missing_ready_obj = FindMissingReady(BranchScanJob)
+        find_missing_ready_obj = self.getFMR(BranchScanJob, 1)
         missing_ready = find_missing_ready_obj.find_missing_ready()
         try:
             self.assertEqual([], missing_ready)
         except:
+            # XXX AaronBentley: 2012-08-01 bug=1031018: Extra diagnostic info
+            # to help diagnose this hard-to-reproduce failure.
             self.addTextDetail('queued_job_ids',
                                '%r' % (find_missing_ready_obj.queued_job_ids))
             self.addTextDetail('queue_contents',
@@ -89,9 +99,8 @@
                 missing_ready]))
             raise
         drain_celery_queues()
-        self.assertQueueSize(self.CeleryRunJob.app,
-                             [BranchScanJob.task_queue], 0)
-        self.assertEqual([job], self.find_missing_ready(BranchScanJob))
+        find_missing_ready_obj = self.getFMR(BranchScanJob, 0)
+        self.assertEqual([job], find_missing_ready_obj.find_missing_ready())
 
     def test_run_missing_ready_not_enabled(self):
         """run_missing_ready does nothing if the class isn't enabled."""