Launchpad itself

Merge lp:~abentley/launchpad/incremental-diff-job into lp:launchpad/db-devel

  1. incremental-diff-job
  2. Merge into db-devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 9950
Proposed branch: lp:~abentley/launchpad/incremental-diff-job
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~abentley/launchpad/display-incremental
Diff against target: 673 lines (+301/-66)
9 files modified
lib/lp/code/configure.zcml (+5/-0)
lib/lp/code/interfaces/branchmergeproposal.py (+20/-0)
lib/lp/code/model/branch.py (+12/-4)
lib/lp/code/model/branchmergeproposal.py (+11/-0)
lib/lp/code/model/branchmergeproposaljob.py (+93/-14)
lib/lp/code/model/tests/test_branchmergeproposaljobs.py (+69/-3)
lib/lp/code/model/tests/test_diff.py (+45/-44)
lib/lp/code/scripts/tests/test_merge_proposal_jobs.py (+12/-1)
lib/lp/codehosting/scanner/tests/test_bzrsync.py (+34/-0)
To merge this branch: bzr merge lp:~abentley/launchpad/incremental-diff-job
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+36881@code.launchpad.net

Commit message

Implement jobs for generating incremental diffs.

Description of the change

= Summary =
Implement jobs for generating incremental diffs for merge proposals, and create
these jobs when the merge proposal source banch tip changes.

== Proposed fix ==
See above

== Pre-implementation notes ==
Discussed with Thumper

== Implementation details ==
As a driveby, I extracted commit_file and create_example_merge so that they
could be reused more easily, without necessarily deriving from DiffTestCase.

I also added a job_type parameter to BranchMergeProposalJobSource.iterReady, to
make testing easier.

There were also a few lint fixes, like removing a semicolon.

== Tests ==
bin/test -t TestGenerateIncrementalDiffJob -t test_script_runs

== Demo and Q/A ==
Create a merge proposal. After the initial diff has been generated, push new
changes to the source branch. As well as updating the diff, this should show a
diff from the old branch tip to the new branch tip.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/codehosting/scanner/tests/test_bzrsync.py
  lib/lp/code/browser/configure.zcml
  lib/lp/code/configure.zcml
  lib/lp/code/model/tests/test_branchmergeproposaljobs.py
  lib/lp/code/interfaces/branchmergeproposal.py
  lib/lp/code/model/branchmergeproposal.py
  lib/lp/code/model/tests/test_branchmergeproposal.py
  lib/lp/code/browser/tests/test_branchmergeproposal.py
  lib/lp/code/scripts/tests/test_merge_proposal_jobs.py
  lib/lp/code/model/diff.py
  lib/lp/testing/factory.py
  lib/lp/code/model/tests/test_diff.py
  lib/lp/code/templates/incrementaldiffcomment-body.pt
  lib/lp/code/model/branch.py
  lib/lp/code/model/branchmergeproposaljob.py
  lib/lp/code/browser/branchmergeproposal.py
  utilities/sourcedeps.conf
  lib/lp/code/templates/incrementaldiffcomment-header.pt
  lib/lp/code/interfaces/diff.py

./lib/lp/codehosting/scanner/tests/test_bzrsync.py
     618: E202 whitespace before ')'
     635: E231 missing whitespace after ','
./lib/lp/code/model/tests/test_branchmergeproposal.py
     192: E231 missing whitespace after ','
     194: E231 missing whitespace after ','
./lib/lp/code/scripts/tests/test_merge_proposal_jobs.py
      15: E202 whitespace before ')'
./lib/lp/code/model/diff.py
     166: E301 expected 1 blank line, found 0
./lib/lp/code/model/branchmergeproposaljob.py
       5: E303 too many blank lines (3)
     292: E202 whitespace before ')'
./lib/lp/code/browser/branchmergeproposal.py
     168: E301 expected 1 blank line, found 0
     178: E301 expected 1 blank line, found 0
     201: E202 whitespace before '}'
    1471: E301 expected 1 blank line, found 0

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

Hi Aaron,

Nice branch, just a few things that need fixing before it lands:

 * On line 94 of the diff, you need to indent the closing parenthesis by 4 more spaces.
 * On line 302 you need to add a \n after the opening bracket of the clauses list. You also need to fix the indentation of the list items so that they're four-spaces in from clauses = [ line.
 * You need to add docstrings / comments for the last four tests in TestGenerateIncrementalDiffJob.

review: Approve (code)
Revision history for this message
Brad Crittenden (bac) wrote :

Is this approved branch ready to land or are you blocked? If you have abandoned the work please change the MP status to "Rejected."

Revision history for this message
Aaron Bentley (abentley) wrote :

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

On 11/04/2010 11:09 PM, Brad Crittenden wrote:
> Is this approved branch ready to land or are you blocked? If you have abandoned the work please change the MP status to "Rejected."

This branch was blocked. See my "Mysterious storm bug" post. Then Jamu
and I found a solution. Then there was another bug relating to test
isolation with feature flags, which Tim was able to solve. I merged
Tim's changes from devel, but those brought with them actual windmill
failures introduced by Ian Booth. When those were resolved, I merged
again and attempted to land, only to discover we were in testfix mode
due to spurious windmill failures on db-devel. I forced a build and
when it passed, I attempted to land the branch, but the landing failed
due to conflicts. That was after EOD yesterday. Today I plan to
resolve the conflicts and land it.

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

iEYEARECAAYFAkzUBC4ACgkQ0F+nu1YWqI3a/ACcCcBLY4NCzM5jkqtJsYsxNGI4
6LQAn2JpFS1GQaqzDOaMlOdfU8d1f3Ll
=KAU9
-----END PGP SIGNATURE-----

Revision history for this message
Aaron Bentley (abentley) wrote :

I tried to land once again, and once again we are in testfix mode due to the same spurious windmill failure.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/configure.zcml'
2--- lib/lp/code/configure.zcml 2010-11-05 14:03:28 +0000
3+++ lib/lp/code/configure.zcml 2010-11-05 14:03:38 +0000
4@@ -307,6 +307,11 @@
5 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJobSource"/>
6 </securedutility>
7
8+ <class
9+ class="lp.code.model.branchmergeproposaljob.GenerateIncrementalDiffJob">
10+ <allow interface="lp.code.interfaces.branchmergeproposal.IGenerateIncrementalDiffJob"/>
11+ <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />
12+ </class>
13 <class class="lp.code.model.branchmergeproposaljob.UpdatePreviewDiffJob">
14 <allow interface="lp.code.interfaces.branchmergeproposal.IUpdatePreviewDiffJob" />
15 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />
16
17=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
18--- lib/lp/code/interfaces/branchmergeproposal.py 2010-10-25 05:33:20 +0000
19+++ lib/lp/code/interfaces/branchmergeproposal.py 2010-11-05 14:03:38 +0000
20@@ -17,6 +17,8 @@
21 'ICodeReviewCommentEmailJobSource',
22 'ICreateMergeProposalJob',
23 'ICreateMergeProposalJobSource',
24+ 'IGenerateIncrementalDiffJob',
25+ 'IGenerateIncrementalDiffJobSource',
26 'IMergeProposalNeedsReviewEmailJob',
27 'IMergeProposalNeedsReviewEmailJobSource',
28 'IMergeProposalUpdatedEmailJob',
29@@ -556,6 +558,10 @@
30 """A job source that will get all supported merge proposal jobs."""
31
32
33+class IBranchMergeProposalJobSource(IJobSource):
34+ """A job source that will get all supported merge proposal jobs."""
35+
36+
37 class IBranchMergeProposalListingBatchNavigator(ITableBatchNavigator):
38 """A marker interface for registering the appropriate listings."""
39
40@@ -661,6 +667,20 @@
41 """Return the UpdatePreviewDiffJob with this id."""
42
43
44+class IGenerateIncrementalDiffJob(IRunnableJob):
45+ """Interface for the job to update the diff for a merge proposal."""
46+
47+
48+class IGenerateIncrementalDiffJobSource(Interface):
49+ """Create or retrieve jobs that update preview diffs."""
50+
51+ def create(bmp, old_revision_id, new_revision_id):
52+ """Create job to generate incremental diff for this merge proposal."""
53+
54+ def get(id):
55+ """Return the GenerateIncrementalDiffJob with this id."""
56+
57+
58 class ICodeReviewCommentEmailJob(IRunnableJob):
59 """Interface for the job to send code review comment email."""
60
61
62=== modified file 'lib/lp/code/model/branch.py'
63--- lib/lp/code/model/branch.py 2010-11-04 19:59:02 +0000
64+++ lib/lp/code/model/branch.py 2010-11-05 14:03:38 +0000
65@@ -471,10 +471,18 @@
66
67 def scheduleDiffUpdates(self):
68 """See `IBranch`."""
69- from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
70- jobs = [UpdatePreviewDiffJob.create(target)
71- for target in self.active_landing_targets
72- if target.target_branch.last_scanned_id is not None]
73+ from lp.code.model.branchmergeproposaljob import (
74+ GenerateIncrementalDiffJob,
75+ UpdatePreviewDiffJob,
76+ )
77+ jobs = []
78+ for merge_proposal in self.active_landing_targets:
79+ if merge_proposal.target_branch.last_scanned_id is None:
80+ continue
81+ jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
82+ for old, new in merge_proposal.getMissingIncrementalDiffs():
83+ GenerateIncrementalDiffJob.create(
84+ merge_proposal, old.revision_id, new.revision_id)
85 return jobs
86
87 def addToLaunchBag(self, launchbag):
88
89=== modified file 'lib/lp/code/model/branchmergeproposal.py'
90--- lib/lp/code/model/branchmergeproposal.py 2010-11-05 14:03:28 +0000
91+++ lib/lp/code/model/branchmergeproposal.py 2010-11-05 14:03:38 +0000
92@@ -785,6 +785,12 @@
93 Store.of(self).flush()
94 return self.preview_diff
95
96+ def getIncrementalDiffRanges(self):
97+ groups = self.getRevisionsSinceReviewStart()
98+ return [
99+ (group[0].revision.getLefthandParent(), group[-1].revision)
100+ for group in groups]
101+
102 def generateIncrementalDiff(self, old_revision, new_revision, diff=None):
103 """Generate an incremental diff for the merge proposal.
104
105@@ -870,6 +876,11 @@
106 if current_group != []:
107 yield current_group
108
109+ def getMissingIncrementalDiffs(self):
110+ ranges = self.getIncrementalDiffRanges()
111+ diffs = self.getIncrementalDiffs(ranges)
112+ return [range_ for range_, diff in zip(ranges, diffs) if diff is None]
113+
114
115 class BranchMergeProposalGetter:
116 """See `IBranchMergeProposalGetter`."""
117
118=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
119--- lib/lp/code/model/branchmergeproposaljob.py 2010-10-26 15:47:24 +0000
120+++ lib/lp/code/model/branchmergeproposaljob.py 2010-11-05 14:03:38 +0000
121@@ -19,6 +19,8 @@
122 'BranchMergeProposalJobType',
123 'CodeReviewCommentEmailJob',
124 'CreateMergeProposalJob',
125+ 'GenerateIncrementalDiffJob',
126+ 'MergeProposalCreatedJob',
127 'MergeProposalNeedsReviewEmailJob',
128 'MergeProposalUpdatedEmailJob',
129 'ReviewRequestedEmailJob',
130@@ -83,6 +85,8 @@
131 ICodeReviewCommentEmailJobSource,
132 ICreateMergeProposalJob,
133 ICreateMergeProposalJobSource,
134+ IGenerateIncrementalDiffJob,
135+ IGenerateIncrementalDiffJobSource,
136 IMergeProposalNeedsReviewEmailJob,
137 IMergeProposalNeedsReviewEmailJobSource,
138 IMergeProposalUpdatedEmailJob,
139@@ -92,6 +96,7 @@
140 IUpdatePreviewDiffJob,
141 IUpdatePreviewDiffJobSource,
142 )
143+from lp.code.interfaces.revision import IRevisionSet
144 from lp.code.mail.branch import RecipientReason
145 from lp.code.mail.branchmergeproposal import BMPMailer
146 from lp.code.mail.codereviewcomment import CodeReviewCommentMailer
147@@ -144,6 +149,11 @@
148 that have been changed on the merge proposal itself.
149 """)
150
151+ GENERATE_INCREMENTAL_DIFF = DBItem(5, """
152+ Generate incremental diff
153+
154+ This job generates an incremental diff for a merge proposal.""")
155+
156
157 class BranchMergeProposalJob(Storm):
158 """Base class for jobs related to branch merge proposals."""
159@@ -282,7 +292,7 @@
160
161 def getOopsVars(self):
162 """See `IRunnableJob`."""
163- vars = BaseRunnableJob.getOopsVars(self)
164+ vars = BaseRunnableJob.getOopsVars(self)
165 bmp = self.context.branch_merge_proposal
166 vars.extend([
167 ('branchmergeproposal_job_id', self.context.id),
168@@ -490,7 +500,7 @@
169
170 def getOopsVars(self):
171 """See `IRunnableJob`."""
172- vars = BranchMergeProposalJobDerived.getOopsVars(self)
173+ vars = BranchMergeProposalJobDerived.getOopsVars(self)
174 vars.extend([
175 ('code_review_comment', self.metadata['code_review_comment']),
176 ])
177@@ -552,7 +562,7 @@
178
179 def getOopsVars(self):
180 """See `IRunnableJob`."""
181- vars = BranchMergeProposalJobDerived.getOopsVars(self)
182+ vars = BranchMergeProposalJobDerived.getOopsVars(self)
183 vars.extend([
184 ('reviewer', self.metadata['reviewer']),
185 ('requester', self.metadata['requester']),
186@@ -601,7 +611,7 @@
187 def getMetadata(delta_text, editor):
188 metadata = {'delta_text': delta_text}
189 if editor is not None:
190- metadata['editor'] = editor.name;
191+ metadata['editor'] = editor.name
192 return metadata
193
194 @property
195@@ -620,7 +630,7 @@
196
197 def getOopsVars(self):
198 """See `IRunnableJob`."""
199- vars = BranchMergeProposalJobDerived.getOopsVars(self)
200+ vars = BranchMergeProposalJobDerived.getOopsVars(self)
201 vars.extend([
202 ('editor', self.metadata.get('editor', '(not set)')),
203 ('delta_text', self.metadata['delta_text']),
204@@ -638,6 +648,70 @@
205 return 'emailing subscribers about merge proposal changes'
206
207
208+class GenerateIncrementalDiffJob(BranchMergeProposalJobDerived):
209+ """A job to generate an incremental diff for a branch merge proposal.
210+
211+ Provides class methods to create and retrieve such jobs.
212+ """
213+
214+ implements(IGenerateIncrementalDiffJob)
215+
216+ classProvides(IGenerateIncrementalDiffJobSource)
217+
218+ class_job_type = BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF
219+
220+ def acquireLease(self, duration=600):
221+ return self.job.acquireLease(duration)
222+
223+ def run(self):
224+ revision_set = getUtility(IRevisionSet)
225+ old_revision = revision_set.getByRevisionId(self.old_revision_id)
226+ new_revision = revision_set.getByRevisionId(self.new_revision_id)
227+ diff = self.branch_merge_proposal.generateIncrementalDiff(
228+ old_revision, new_revision)
229+
230+ @classmethod
231+ def create(cls, merge_proposal, old_revision_id, new_revision_id):
232+ metadata = cls.getMetadata(old_revision_id, new_revision_id)
233+ job = BranchMergeProposalJob(
234+ merge_proposal, cls.class_job_type, metadata)
235+ return cls(job)
236+
237+ @staticmethod
238+ def getMetadata(old_revision_id, new_revision_id):
239+ return {
240+ 'old_revision_id': old_revision_id,
241+ 'new_revision_id': new_revision_id,
242+ }
243+
244+ @property
245+ def old_revision_id(self):
246+ """The old revision id for the diff."""
247+ return self.metadata['old_revision_id']
248+
249+ @property
250+ def new_revision_id(self):
251+ """The new revision id for the diff."""
252+ return self.metadata['new_revision_id']
253+
254+ def getOopsVars(self):
255+ """See `IRunnableJob`."""
256+ vars = BranchMergeProposalJobDerived.getOopsVars(self)
257+ vars.extend([
258+ ('old_revision_id', self.metadata['old_revision_id']),
259+ ('new_revision_id', self.metadata['new_revision_id']),
260+ ])
261+ return vars
262+
263+ def getOperationDescription(self):
264+ return ('generating an incremental diff for a merge proposal')
265+
266+ def getErrorRecipients(self):
267+ """Return a list of email-ids to notify about user errors."""
268+ registrant = self.branch_merge_proposal.registrant
269+ return format_address_for_person(registrant)
270+
271+
272 class BranchMergeProposalJobFactory:
273 """Construct a derived merge proposal job for a BranchMergeProposalJob."""
274
275@@ -652,6 +726,8 @@
276 ReviewRequestedEmailJob,
277 BranchMergeProposalJobType.MERGE_PROPOSAL_UPDATED:
278 MergeProposalUpdatedEmailJob,
279+ BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF:
280+ GenerateIncrementalDiffJob,
281 }
282
283 @classmethod
284@@ -692,21 +768,24 @@
285 return BranchMergeProposalJobFactory.create(job)
286
287 @staticmethod
288- def iterReady():
289+ def iterReady(job_type=None):
290 from lp.code.model.branch import Branch
291 store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
292 SourceBranch = ClassAlias(Branch)
293 TargetBranch = ClassAlias(Branch)
294+ clauses = [
295+ BranchMergeProposalJob.job == Job.id,
296+ Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]),
297+ BranchMergeProposalJob.branch_merge_proposal ==
298+ BranchMergeProposal.id, BranchMergeProposal.source_branch ==
299+ SourceBranch.id, BranchMergeProposal.target_branch ==
300+ TargetBranch.id,
301+ ]
302+ if job_type is not None:
303+ clauses.append(BranchMergeProposalJob.job_type == job_type)
304 jobs = store.find(
305 (BranchMergeProposalJob, Job, BranchMergeProposal,
306- SourceBranch, TargetBranch),
307- And(BranchMergeProposalJob.job == Job.id,
308- Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]),
309- BranchMergeProposalJob.branch_merge_proposal
310- == BranchMergeProposal.id,
311- BranchMergeProposal.source_branch == SourceBranch.id,
312- BranchMergeProposal.target_branch == TargetBranch.id,
313- ))
314+ SourceBranch, TargetBranch), And(*clauses))
315 # Order by the job status first (to get running before waiting), then
316 # the date_created, then job type. This should give us all creation
317 # jobs before comment jobs.
318
319=== modified file 'lib/lp/code/model/tests/test_branchmergeproposaljobs.py'
320--- lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-10-28 22:31:48 +0000
321+++ lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-11-05 14:03:38 +0000
322@@ -32,6 +32,8 @@
323 IBranchMergeProposalJobSource,
324 ICodeReviewCommentEmailJob,
325 ICodeReviewCommentEmailJobSource,
326+ IGenerateIncrementalDiffJob,
327+ IGenerateIncrementalDiffJobSource,
328 IMergeProposalNeedsReviewEmailJob,
329 IMergeProposalUpdatedEmailJob,
330 IMergeProposalUpdatedEmailJobSource,
331@@ -45,12 +47,13 @@
332 BranchMergeProposalJobDerived,
333 BranchMergeProposalJobType,
334 CodeReviewCommentEmailJob,
335+ GenerateIncrementalDiffJob,
336 MergeProposalNeedsReviewEmailJob,
337 MergeProposalUpdatedEmailJob,
338 ReviewRequestedEmailJob,
339 UpdatePreviewDiffJob,
340 )
341-from lp.code.model.tests.test_diff import DiffTestCase
342+from lp.code.model.tests.test_diff import DiffTestCase, create_example_merge
343 from lp.code.subscribers.branchmergeproposal import merge_proposal_modified
344 from lp.services.job.model.job import Job
345 from lp.services.job.runner import JobRunner
346@@ -192,7 +195,7 @@
347
348 def test_run(self):
349 self.useBzrBranches(direct_database=True)
350- bmp = self.createExampleMerge()[0]
351+ bmp = create_example_merge(self)[0]
352 job = UpdatePreviewDiffJob.create(bmp)
353 self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)
354 bmp.source_branch.next_mirror_time = None
355@@ -226,13 +229,76 @@
356
357 def test_10_minute_lease(self):
358 self.useBzrBranches(direct_database=True)
359- bmp = self.createExampleMerge()[0]
360+ bmp = create_example_merge(self)[0]
361 job = UpdatePreviewDiffJob.create(bmp)
362 job.acquireLease()
363 expiry_delta = job.lease_expires - datetime.now(pytz.UTC)
364 self.assertTrue(500 <= expiry_delta.seconds, expiry_delta)
365
366
367+def make_runnable_incremental_diff_job(test_case):
368+ test_case.useBzrBranches(direct_database=True)
369+ bmp, source_rev_id, target_rev_id = create_example_merge(test_case)
370+ repository = bmp.source_branch.getBzrBranch().repository
371+ parent_id = repository.get_revision(source_rev_id).parent_ids[0]
372+ test_case.factory.makeRevision(rev_id=source_rev_id)
373+ test_case.factory.makeRevision(rev_id=parent_id)
374+ return GenerateIncrementalDiffJob.create(bmp, parent_id, source_rev_id)
375+
376+
377+class TestGenerateIncrementalDiffJob(DiffTestCase):
378+
379+ layer = LaunchpadZopelessLayer
380+
381+ def test_implement_interface(self):
382+ """GenerateIncrementalDiffJob implements its interface."""
383+ verifyObject(
384+ IGenerateIncrementalDiffJobSource, GenerateIncrementalDiffJob)
385+
386+ def test_providesInterface(self):
387+ """MergeProposalCreatedJob provides the expected interfaces."""
388+ bmp = self.factory.makeBranchMergeProposal()
389+ job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
390+ verifyObject(IGenerateIncrementalDiffJob, job)
391+ verifyObject(IBranchMergeProposalJob, job)
392+
393+ def test_getOperationDescription(self):
394+ """The description of the job is sane."""
395+ bmp = self.factory.makeBranchMergeProposal()
396+ job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
397+ self.assertEqual(
398+ 'generating an incremental diff for a merge proposal',
399+ job.getOperationDescription())
400+
401+ def test_run(self):
402+ """The job runs successfully, and its results can be committed."""
403+ job = make_runnable_incremental_diff_job(self)
404+ transaction.commit()
405+ self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
406+ job.run()
407+ transaction.commit()
408+
409+ def test_run_all(self):
410+ """The job can be run under the JobRunner successfully."""
411+ job = make_runnable_incremental_diff_job(self)
412+ transaction.commit()
413+ self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
414+ runner = JobRunner([job])
415+ runner.runAll()
416+ self.assertEqual([job], runner.completed_jobs)
417+
418+ def test_10_minute_lease(self):
419+ """Newly-created jobs have a ten-minute lease."""
420+ self.useBzrBranches(direct_database=True)
421+ bmp = create_example_merge(self)[0]
422+ job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
423+ transaction.commit()
424+ self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
425+ job.acquireLease()
426+ expiry_delta = job.lease_expires - datetime.now(pytz.UTC)
427+ self.assertTrue(500 <= expiry_delta.seconds, expiry_delta)
428+
429+
430 class TestBranchMergeProposalJobSource(TestCaseWithFactory):
431
432 layer = LaunchpadZopelessLayer
433
434=== modified file 'lib/lp/code/model/tests/test_diff.py'
435--- lib/lp/code/model/tests/test_diff.py 2010-11-05 14:03:28 +0000
436+++ lib/lp/code/model/tests/test_diff.py 2010-11-05 14:03:38 +0000
437@@ -54,37 +54,41 @@
438 self.records.append(record)
439
440
441+def commit_file(branch, path, contents, merge_parents=None):
442+ """Create a commit that updates a file to specified contents.
443+
444+ This will create or modify the file, as needed.
445+ """
446+ committer = DirectBranchCommit(
447+ removeSecurityProxy(branch), no_race_check=True,
448+ merge_parents=merge_parents)
449+ committer.writeFile(path, contents)
450+ try:
451+ return committer.commit('committing')
452+ finally:
453+ committer.unlock()
454+
455+
456+def create_example_merge(test_case):
457+ """Create a merge proposal with conflicts and updates."""
458+ bmp = test_case.factory.makeBranchMergeProposal()
459+ # Make the branches of the merge proposal look good as far as the
460+ # model is concerned.
461+ test_case.factory.makeRevisionsForBranch(bmp.source_branch)
462+ test_case.factory.makeRevisionsForBranch(bmp.target_branch)
463+ bzr_target = test_case.createBzrBranch(bmp.target_branch)
464+ commit_file(bmp.target_branch, 'foo', 'a\n')
465+ test_case.createBzrBranch(bmp.source_branch, bzr_target)
466+ source_rev_id = commit_file(bmp.source_branch, 'foo', 'd\na\nb\n')
467+ target_rev_id = commit_file(bmp.target_branch, 'foo', 'c\na\n')
468+ return bmp, source_rev_id, target_rev_id
469+
470+
471 class DiffTestCase(TestCaseWithFactory):
472
473- @staticmethod
474- def commitFile(branch, path, contents, merge_parents=None):
475- """Create a commit that updates a file to specified contents.
476-
477- This will create or modify the file, as needed.
478- """
479- committer = DirectBranchCommit(
480- removeSecurityProxy(branch), no_race_check=True,
481- merge_parents=merge_parents)
482- committer.writeFile(path, contents)
483- try:
484- return committer.commit('committing')
485- finally:
486- committer.unlock()
487-
488 def createExampleMerge(self):
489- """Create a merge proposal with conflicts and updates."""
490 self.useBzrBranches(direct_database=True)
491- bmp = self.factory.makeBranchMergeProposal()
492- # Make the branches of the merge proposal look good as far as the
493- # model is concerned.
494- self.factory.makeRevisionsForBranch(bmp.source_branch)
495- self.factory.makeRevisionsForBranch(bmp.target_branch)
496- bzr_target = self.createBzrBranch(bmp.target_branch)
497- self.commitFile(bmp.target_branch, 'foo', 'a\n')
498- self.createBzrBranch(bmp.source_branch, bzr_target)
499- source_rev_id = self.commitFile(bmp.source_branch, 'foo', 'd\na\nb\n')
500- target_rev_id = self.commitFile(bmp.target_branch, 'foo', 'c\na\n')
501- return bmp, source_rev_id, target_rev_id
502+ return create_example_merge(self)
503
504 def checkExampleMerge(self, diff_text):
505 """Ensure the diff text matches the values for ExampleMerge."""
506@@ -111,12 +115,12 @@
507 source = bmp.source_branch
508 prerequisite = bmp.prerequisite_branch
509 target_bzr = self.createBzrBranch(target)
510- self.commitFile(target, 'file', 'target text\n')
511+ commit_file(target, 'file', 'target text\n')
512 prerequisite_bzr = self.createBzrBranch(prerequisite, target_bzr)
513- self.commitFile(
514+ commit_file(
515 prerequisite, 'file', 'target text\nprerequisite text\n')
516 source_bzr = self.createBzrBranch(source, prerequisite_bzr)
517- source_rev_id = self.commitFile(
518+ source_rev_id = commit_file(
519 source, 'file',
520 'target text\nprerequisite text\nsource text\n')
521 return (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,
522@@ -243,7 +247,7 @@
523 # affect the diff.
524 (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,
525 prerequisite) = self.preparePrerequisiteMerge()
526- self.commitFile(
527+ commit_file(
528 prerequisite, 'file', 'prerequisite text2\n')
529 diff, conflicts = Diff.mergePreviewFromBranches(
530 source_bzr, source_rev_id, target_bzr, prerequisite_bzr)
531@@ -459,10 +463,10 @@
532 self.useBzrBranches(direct_database=True)
533 bmp = self.factory.makeBranchMergeProposal()
534 bzr_target = self.createBzrBranch(bmp.target_branch)
535- self.commitFile(bmp.target_branch, 'foo', 'a\n')
536+ commit_file(bmp.target_branch, 'foo', 'a\n')
537 self.createBzrBranch(bmp.source_branch, bzr_target)
538- source_rev_id = self.commitFile(bmp.source_branch, 'foo', 'a\nb\n')
539- target_rev_id = self.commitFile(bmp.target_branch, 'foo', 'c\na\n')
540+ source_rev_id = commit_file(bmp.source_branch, 'foo', 'a\nb\n')
541+ target_rev_id = commit_file(bmp.target_branch, 'foo', 'c\na\n')
542 diff = PreviewDiff.fromBranchMergeProposal(bmp)
543 self.assertEqual('', diff.conflicts)
544 self.assertFalse(diff.has_conflicts)
545@@ -575,25 +579,22 @@
546 bmp = self.factory.makeBranchMergeProposal(
547 prerequisite_branch=prerequisite_branch)
548 target_branch = self.createBzrBranch(bmp.target_branch)
549- old_revision_id = self.commitFile(
550- bmp.target_branch, 'foo', 'a\nb\ne\n')
551+ old_revision_id = commit_file(bmp.target_branch, 'foo', 'a\nb\ne\n')
552 old_revision = self.factory.makeRevision(rev_id=old_revision_id)
553 source_branch = self.createBzrBranch(
554 bmp.source_branch, target_branch)
555- self.commitFile(bmp.source_branch, 'foo', 'a\nc\ne\n')
556+ commit_file(bmp.source_branch, 'foo', 'a\nc\ne\n')
557 prerequisite = self.createBzrBranch(
558 bmp.prerequisite_branch, target_branch)
559- prerequisite_revision = self.commitFile(bmp.prerequisite_branch,
560- 'foo', 'd\na\nb\ne\n')
561- merge_parent = self.commitFile(bmp.target_branch, 'foo',
562- 'a\nb\ne\nf\n')
563+ prerequisite_revision = commit_file(
564+ bmp.prerequisite_branch, 'foo', 'd\na\nb\ne\n')
565+ merge_parent = commit_file(bmp.target_branch, 'foo', 'a\nb\ne\nf\n')
566 source_branch.repository.fetch(target_branch.repository,
567 revision_id=merge_parent)
568- self.commitFile(
569- bmp.source_branch, 'foo', 'a\nc\ne\nf\n', [merge_parent])
570+ commit_file(bmp.source_branch, 'foo', 'a\nc\ne\nf\n', [merge_parent])
571 source_branch.repository.fetch(prerequisite.repository,
572 revision_id=prerequisite_revision)
573- new_revision_id = self.commitFile(
574+ new_revision_id = commit_file(
575 bmp.source_branch, 'foo', 'd\na\nc\ne\nf\n',
576 [prerequisite_revision])
577 new_revision = self.factory.makeRevision(rev_id=new_revision_id)
578
579=== modified file 'lib/lp/code/scripts/tests/test_merge_proposal_jobs.py'
580--- lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2010-10-04 19:50:45 +0000
581+++ lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2010-11-05 14:03:38 +0000
582@@ -7,8 +7,14 @@
583
584 import unittest
585
586+import transaction
587+
588 from canonical.launchpad.scripts.tests import run_script
589 from canonical.testing.layers import ZopelessAppServerLayer
590+from lp.code.model.tests.test_branchmergeproposaljobs import (
591+ make_runnable_incremental_diff_job
592+)
593+from lp.services.job.interfaces.job import JobStatus
594 from lp.testing import TestCaseWithFactory
595
596
597@@ -18,6 +24,8 @@
598
599 def test_script_runs(self):
600 """Ensure merge-proposal-jobs script runs."""
601+ job = make_runnable_incremental_diff_job(self)
602+ transaction.commit()
603 retcode, stdout, stderr = run_script(
604 'cronscripts/merge-proposal-jobs.py', [])
605 self.assertEqual(0, retcode)
606@@ -25,7 +33,10 @@
607 self.assertEqual(
608 'INFO Creating lockfile:'
609 ' /var/lock/launchpad-merge-proposal-jobs.lock\n'
610- 'INFO Running through Twisted.\n', stderr)
611+ 'INFO Running through Twisted.\n'
612+ 'INFO Ran 1 GenerateIncrementalDiffJob jobs.\n', stderr)
613+ self.assertEqual(JobStatus.COMPLETED, job.status)
614+
615
616 def test_suite():
617 return unittest.TestLoader().loadTestsFromName(__name__)
618
619=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
620--- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-10-26 15:47:24 +0000
621+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-11-05 14:03:38 +0000
622@@ -31,11 +31,14 @@
623 from lp.code.interfaces.branchlookup import IBranchLookup
624 from lp.code.interfaces.revision import IRevisionSet
625 from lp.code.model.branchrevision import BranchRevision
626+from lp.code.model.branchmergeproposaljob import (
627+ BranchMergeProposalJobSource, BranchMergeProposalJobType)
628 from lp.code.model.revision import (
629 Revision,
630 RevisionAuthor,
631 RevisionParent,
632 )
633+from lp.code.model.tests.test_diff import commit_file
634 from lp.codehosting.bzrutils import write_locked
635 from lp.codehosting.scanner.bzrsync import BzrSync
636 from lp.services.osutils import override_environ
637@@ -654,6 +657,37 @@
638 self.assertIsNot(None, bmp.next_preview_diff_job)
639
640
641+class TestGenerateIncrementalDiffJob(BzrSyncTestCase):
642+ """Test the scheduling of GenerateIncrementalDiffJobs."""
643+
644+ def getPending(self):
645+ return list(
646+ BranchMergeProposalJobSource.iterReady(
647+ BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF
648+ )
649+ )
650+
651+ @run_as_db_user(config.launchpad.dbuser)
652+ def test_create_on_new_revision(self):
653+ """When branch tip changes, a job is created."""
654+ parent_id = commit_file(self.db_branch, 'foo', 'bar')
655+ self.factory.makeBranchRevision(self.db_branch, parent_id,
656+ revision_date=self.factory.getUniqueDate())
657+ self.db_branch.last_scanned_id = parent_id
658+ bmp = self.factory.makeBranchMergeProposal(
659+ source_branch=self.db_branch,
660+ date_created=self.factory.getUniqueDate())
661+ revision_id = commit_file(self.db_branch, 'foo', 'baz')
662+ removeSecurityProxy(bmp).target_branch.last_scanned_id = 'rev'
663+ self.assertEqual([], self.getPending())
664+ transaction.commit()
665+ LaunchpadZopelessLayer.switchDbUser(config.branchscanner.dbuser)
666+ self.makeBzrSync(self.db_branch).syncBranchAndClose()
667+ (job,) = self.getPending()
668+ self.assertEqual(revision_id, job.new_revision_id)
669+ self.assertEqual(parent_id, job.old_revision_id)
670+
671+
672 class TestSetRecipeStale(BzrSyncTestCase):
673 """Test recipes associated with the branch are marked stale."""
674

Subscribers

People subscribed via source and target branches

to status/vote changes: