Merge lp:~mwhudson/launchpad/bmj-iterReady-missing-term into lp:launchpad

[Michael Hudson-Doyle]

Add a missing term to the query for ready branchmergeproposal jobs.

[Michael Hudson-Doyle (mwhudson)]

While working on fixing this query for another branch of mine, I noticed that it's missing a join between the "job" and the "branch" sides of the query. Here's a test case and the fix.

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 merge approved

Thanks for catching this!

Michael Hudson wrote:
> Michael Hudson has proposed merging lp:~mwhudson/launchpad/bmj-iterReady-missing-term into lp:launchpad/devel.
>
> Requested reviews:
> Aaron Bentley (abentley)
>
>
> While working on fixing this query for another branch of mine, I noticed that it's missing a join between the "job" and the "branch" sides of the query. Here's a test case and the fix.
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkrwnocACgkQ0F+nu1YWqI19MwCfZgA/WNhs9DaBlPspKWwH+Ej4
KhkAn1baqBPWOm5yThIveu0HcWIPkTr3
=cg11
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py	2009-09-16 13:47:41 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py	2009-11-03 21:15:22 +0000
@@ -173,6 +173,8 @@
             And(BranchMergeProposalJob.job_type == klass.class_job_type,
                 BranchMergeProposalJob.job == Job.id,
                 Job.id.is_in(Job.ready_jobs),
+                BranchMergeProposalJob.branch_merge_proposal
+                    == BranchMergeProposal.id,
                 BranchMergeProposal.source_branch == Branch.id,
                 # A proposal isn't considered ready if it has no revisions,
                 # or if it is hosted but pending a mirror.
@@ -180,7 +182,6 @@
                 Or(Branch.next_mirror_time == None,
                    Branch.branch_type != BranchType.HOSTED)
                 ))
-        jobs.config(distinct=True)
         return (klass(job) for job in jobs)
 
     def getOopsVars(self):

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposals.py'
--- lib/lp/code/model/tests/test_branchmergeproposals.py	2009-10-05 14:17:48 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposals.py	2009-11-03 21:15:22 +0000
@@ -1331,10 +1331,26 @@
         """Skip Jobs with a hosted source branch that needs mirroring."""
         # Suppress events to avoid creating a MergeProposalCreatedJob early.
         bmp = capture_events(self.factory.makeBranchMergeProposal)[0]
+        self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)
         bmp.source_branch.requestMirror()
         job = MergeProposalCreatedJob.create(bmp)
         self.assertEqual([], list(MergeProposalCreatedJob.iterReady()))
 
+    def test_iterReady_joins_properly(self):
+        """An up-to-date branch does not cause a job for a needs-mirroring
+        branch to be returned."""
+        # Suppress events to avoid creating MergeProposalCreatedJobs early.
+        bmp = capture_events(self.factory.makeBranchMergeProposal)[0]
+        bmp2 = capture_events(self.factory.makeBranchMergeProposal)[0]
+        # Give both branches some revisions.
+        self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)
+        self.factory.makeRevisionsForBranch(bmp2.source_branch, count=1)
+        # Request a mirror and create a job for one of them.
+        bmp.source_branch.requestMirror()
+        MergeProposalCreatedJob.create(bmp)
+        # No jobs are ready.
+        self.assertEqual([], list(MergeProposalCreatedJob.iterReady()))
+
     def test_iterReady_includes_mirrored_needing_mirror(self):
         """Skip Jobs with a hosted source branch that needs mirroring."""
         source_branch = self.factory.makeProductBranch(