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
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2010-11-05 14:03:28 +0000
+++ lib/lp/code/configure.zcml 2010-11-05 14:03:38 +0000
@@ -307,6 +307,11 @@
307 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJobSource"/>307 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJobSource"/>
308 </securedutility>308 </securedutility>
309309
310 <class
311 class="lp.code.model.branchmergeproposaljob.GenerateIncrementalDiffJob">
312 <allow interface="lp.code.interfaces.branchmergeproposal.IGenerateIncrementalDiffJob"/>
313 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />
314 </class>
310 <class class="lp.code.model.branchmergeproposaljob.UpdatePreviewDiffJob">315 <class class="lp.code.model.branchmergeproposaljob.UpdatePreviewDiffJob">
311 <allow interface="lp.code.interfaces.branchmergeproposal.IUpdatePreviewDiffJob" />316 <allow interface="lp.code.interfaces.branchmergeproposal.IUpdatePreviewDiffJob" />
312 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />317 <allow interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalJob" />
313318
=== modified file 'lib/lp/code/interfaces/branchmergeproposal.py'
--- lib/lp/code/interfaces/branchmergeproposal.py 2010-10-25 05:33:20 +0000
+++ lib/lp/code/interfaces/branchmergeproposal.py 2010-11-05 14:03:38 +0000
@@ -17,6 +17,8 @@
17 'ICodeReviewCommentEmailJobSource',17 'ICodeReviewCommentEmailJobSource',
18 'ICreateMergeProposalJob',18 'ICreateMergeProposalJob',
19 'ICreateMergeProposalJobSource',19 'ICreateMergeProposalJobSource',
20 'IGenerateIncrementalDiffJob',
21 'IGenerateIncrementalDiffJobSource',
20 'IMergeProposalNeedsReviewEmailJob',22 'IMergeProposalNeedsReviewEmailJob',
21 'IMergeProposalNeedsReviewEmailJobSource',23 'IMergeProposalNeedsReviewEmailJobSource',
22 'IMergeProposalUpdatedEmailJob',24 'IMergeProposalUpdatedEmailJob',
@@ -556,6 +558,10 @@
556 """A job source that will get all supported merge proposal jobs."""558 """A job source that will get all supported merge proposal jobs."""
557559
558560
561class IBranchMergeProposalJobSource(IJobSource):
562 """A job source that will get all supported merge proposal jobs."""
563
564
559class IBranchMergeProposalListingBatchNavigator(ITableBatchNavigator):565class IBranchMergeProposalListingBatchNavigator(ITableBatchNavigator):
560 """A marker interface for registering the appropriate listings."""566 """A marker interface for registering the appropriate listings."""
561567
@@ -661,6 +667,20 @@
661 """Return the UpdatePreviewDiffJob with this id."""667 """Return the UpdatePreviewDiffJob with this id."""
662668
663669
670class IGenerateIncrementalDiffJob(IRunnableJob):
671 """Interface for the job to update the diff for a merge proposal."""
672
673
674class IGenerateIncrementalDiffJobSource(Interface):
675 """Create or retrieve jobs that update preview diffs."""
676
677 def create(bmp, old_revision_id, new_revision_id):
678 """Create job to generate incremental diff for this merge proposal."""
679
680 def get(id):
681 """Return the GenerateIncrementalDiffJob with this id."""
682
683
664class ICodeReviewCommentEmailJob(IRunnableJob):684class ICodeReviewCommentEmailJob(IRunnableJob):
665 """Interface for the job to send code review comment email."""685 """Interface for the job to send code review comment email."""
666686
667687
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2010-11-04 19:59:02 +0000
+++ lib/lp/code/model/branch.py 2010-11-05 14:03:38 +0000
@@ -471,10 +471,18 @@
471471
472 def scheduleDiffUpdates(self):472 def scheduleDiffUpdates(self):
473 """See `IBranch`."""473 """See `IBranch`."""
474 from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob474 from lp.code.model.branchmergeproposaljob import (
475 jobs = [UpdatePreviewDiffJob.create(target)475 GenerateIncrementalDiffJob,
476 for target in self.active_landing_targets476 UpdatePreviewDiffJob,
477 if target.target_branch.last_scanned_id is not None]477 )
478 jobs = []
479 for merge_proposal in self.active_landing_targets:
480 if merge_proposal.target_branch.last_scanned_id is None:
481 continue
482 jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
483 for old, new in merge_proposal.getMissingIncrementalDiffs():
484 GenerateIncrementalDiffJob.create(
485 merge_proposal, old.revision_id, new.revision_id)
478 return jobs486 return jobs
479487
480 def addToLaunchBag(self, launchbag):488 def addToLaunchBag(self, launchbag):
481489
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2010-11-05 14:03:28 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2010-11-05 14:03:38 +0000
@@ -785,6 +785,12 @@
785 Store.of(self).flush()785 Store.of(self).flush()
786 return self.preview_diff786 return self.preview_diff
787787
788 def getIncrementalDiffRanges(self):
789 groups = self.getRevisionsSinceReviewStart()
790 return [
791 (group[0].revision.getLefthandParent(), group[-1].revision)
792 for group in groups]
793
788 def generateIncrementalDiff(self, old_revision, new_revision, diff=None):794 def generateIncrementalDiff(self, old_revision, new_revision, diff=None):
789 """Generate an incremental diff for the merge proposal.795 """Generate an incremental diff for the merge proposal.
790796
@@ -870,6 +876,11 @@
870 if current_group != []:876 if current_group != []:
871 yield current_group877 yield current_group
872878
879 def getMissingIncrementalDiffs(self):
880 ranges = self.getIncrementalDiffRanges()
881 diffs = self.getIncrementalDiffs(ranges)
882 return [range_ for range_, diff in zip(ranges, diffs) if diff is None]
883
873884
874class BranchMergeProposalGetter:885class BranchMergeProposalGetter:
875 """See `IBranchMergeProposalGetter`."""886 """See `IBranchMergeProposalGetter`."""
876887
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2010-10-26 15:47:24 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2010-11-05 14:03:38 +0000
@@ -19,6 +19,8 @@
19 'BranchMergeProposalJobType',19 'BranchMergeProposalJobType',
20 'CodeReviewCommentEmailJob',20 'CodeReviewCommentEmailJob',
21 'CreateMergeProposalJob',21 'CreateMergeProposalJob',
22 'GenerateIncrementalDiffJob',
23 'MergeProposalCreatedJob',
22 'MergeProposalNeedsReviewEmailJob',24 'MergeProposalNeedsReviewEmailJob',
23 'MergeProposalUpdatedEmailJob',25 'MergeProposalUpdatedEmailJob',
24 'ReviewRequestedEmailJob',26 'ReviewRequestedEmailJob',
@@ -83,6 +85,8 @@
83 ICodeReviewCommentEmailJobSource,85 ICodeReviewCommentEmailJobSource,
84 ICreateMergeProposalJob,86 ICreateMergeProposalJob,
85 ICreateMergeProposalJobSource,87 ICreateMergeProposalJobSource,
88 IGenerateIncrementalDiffJob,
89 IGenerateIncrementalDiffJobSource,
86 IMergeProposalNeedsReviewEmailJob,90 IMergeProposalNeedsReviewEmailJob,
87 IMergeProposalNeedsReviewEmailJobSource,91 IMergeProposalNeedsReviewEmailJobSource,
88 IMergeProposalUpdatedEmailJob,92 IMergeProposalUpdatedEmailJob,
@@ -92,6 +96,7 @@
92 IUpdatePreviewDiffJob,96 IUpdatePreviewDiffJob,
93 IUpdatePreviewDiffJobSource,97 IUpdatePreviewDiffJobSource,
94 )98 )
99from lp.code.interfaces.revision import IRevisionSet
95from lp.code.mail.branch import RecipientReason100from lp.code.mail.branch import RecipientReason
96from lp.code.mail.branchmergeproposal import BMPMailer101from lp.code.mail.branchmergeproposal import BMPMailer
97from lp.code.mail.codereviewcomment import CodeReviewCommentMailer102from lp.code.mail.codereviewcomment import CodeReviewCommentMailer
@@ -144,6 +149,11 @@
144 that have been changed on the merge proposal itself.149 that have been changed on the merge proposal itself.
145 """)150 """)
146151
152 GENERATE_INCREMENTAL_DIFF = DBItem(5, """
153 Generate incremental diff
154
155 This job generates an incremental diff for a merge proposal.""")
156
147157
148class BranchMergeProposalJob(Storm):158class BranchMergeProposalJob(Storm):
149 """Base class for jobs related to branch merge proposals."""159 """Base class for jobs related to branch merge proposals."""
@@ -282,7 +292,7 @@
282292
283 def getOopsVars(self):293 def getOopsVars(self):
284 """See `IRunnableJob`."""294 """See `IRunnableJob`."""
285 vars = BaseRunnableJob.getOopsVars(self)295 vars = BaseRunnableJob.getOopsVars(self)
286 bmp = self.context.branch_merge_proposal296 bmp = self.context.branch_merge_proposal
287 vars.extend([297 vars.extend([
288 ('branchmergeproposal_job_id', self.context.id),298 ('branchmergeproposal_job_id', self.context.id),
@@ -490,7 +500,7 @@
490500
491 def getOopsVars(self):501 def getOopsVars(self):
492 """See `IRunnableJob`."""502 """See `IRunnableJob`."""
493 vars = BranchMergeProposalJobDerived.getOopsVars(self)503 vars = BranchMergeProposalJobDerived.getOopsVars(self)
494 vars.extend([504 vars.extend([
495 ('code_review_comment', self.metadata['code_review_comment']),505 ('code_review_comment', self.metadata['code_review_comment']),
496 ])506 ])
@@ -552,7 +562,7 @@
552562
553 def getOopsVars(self):563 def getOopsVars(self):
554 """See `IRunnableJob`."""564 """See `IRunnableJob`."""
555 vars = BranchMergeProposalJobDerived.getOopsVars(self)565 vars = BranchMergeProposalJobDerived.getOopsVars(self)
556 vars.extend([566 vars.extend([
557 ('reviewer', self.metadata['reviewer']),567 ('reviewer', self.metadata['reviewer']),
558 ('requester', self.metadata['requester']),568 ('requester', self.metadata['requester']),
@@ -601,7 +611,7 @@
601 def getMetadata(delta_text, editor):611 def getMetadata(delta_text, editor):
602 metadata = {'delta_text': delta_text}612 metadata = {'delta_text': delta_text}
603 if editor is not None:613 if editor is not None:
604 metadata['editor'] = editor.name;614 metadata['editor'] = editor.name
605 return metadata615 return metadata
606616
607 @property617 @property
@@ -620,7 +630,7 @@
620630
621 def getOopsVars(self):631 def getOopsVars(self):
622 """See `IRunnableJob`."""632 """See `IRunnableJob`."""
623 vars = BranchMergeProposalJobDerived.getOopsVars(self)633 vars = BranchMergeProposalJobDerived.getOopsVars(self)
624 vars.extend([634 vars.extend([
625 ('editor', self.metadata.get('editor', '(not set)')),635 ('editor', self.metadata.get('editor', '(not set)')),
626 ('delta_text', self.metadata['delta_text']),636 ('delta_text', self.metadata['delta_text']),
@@ -638,6 +648,70 @@
638 return 'emailing subscribers about merge proposal changes'648 return 'emailing subscribers about merge proposal changes'
639649
640650
651class GenerateIncrementalDiffJob(BranchMergeProposalJobDerived):
652 """A job to generate an incremental diff for a branch merge proposal.
653
654 Provides class methods to create and retrieve such jobs.
655 """
656
657 implements(IGenerateIncrementalDiffJob)
658
659 classProvides(IGenerateIncrementalDiffJobSource)
660
661 class_job_type = BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF
662
663 def acquireLease(self, duration=600):
664 return self.job.acquireLease(duration)
665
666 def run(self):
667 revision_set = getUtility(IRevisionSet)
668 old_revision = revision_set.getByRevisionId(self.old_revision_id)
669 new_revision = revision_set.getByRevisionId(self.new_revision_id)
670 diff = self.branch_merge_proposal.generateIncrementalDiff(
671 old_revision, new_revision)
672
673 @classmethod
674 def create(cls, merge_proposal, old_revision_id, new_revision_id):
675 metadata = cls.getMetadata(old_revision_id, new_revision_id)
676 job = BranchMergeProposalJob(
677 merge_proposal, cls.class_job_type, metadata)
678 return cls(job)
679
680 @staticmethod
681 def getMetadata(old_revision_id, new_revision_id):
682 return {
683 'old_revision_id': old_revision_id,
684 'new_revision_id': new_revision_id,
685 }
686
687 @property
688 def old_revision_id(self):
689 """The old revision id for the diff."""
690 return self.metadata['old_revision_id']
691
692 @property
693 def new_revision_id(self):
694 """The new revision id for the diff."""
695 return self.metadata['new_revision_id']
696
697 def getOopsVars(self):
698 """See `IRunnableJob`."""
699 vars = BranchMergeProposalJobDerived.getOopsVars(self)
700 vars.extend([
701 ('old_revision_id', self.metadata['old_revision_id']),
702 ('new_revision_id', self.metadata['new_revision_id']),
703 ])
704 return vars
705
706 def getOperationDescription(self):
707 return ('generating an incremental diff for a merge proposal')
708
709 def getErrorRecipients(self):
710 """Return a list of email-ids to notify about user errors."""
711 registrant = self.branch_merge_proposal.registrant
712 return format_address_for_person(registrant)
713
714
641class BranchMergeProposalJobFactory:715class BranchMergeProposalJobFactory:
642 """Construct a derived merge proposal job for a BranchMergeProposalJob."""716 """Construct a derived merge proposal job for a BranchMergeProposalJob."""
643717
@@ -652,6 +726,8 @@
652 ReviewRequestedEmailJob,726 ReviewRequestedEmailJob,
653 BranchMergeProposalJobType.MERGE_PROPOSAL_UPDATED:727 BranchMergeProposalJobType.MERGE_PROPOSAL_UPDATED:
654 MergeProposalUpdatedEmailJob,728 MergeProposalUpdatedEmailJob,
729 BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF:
730 GenerateIncrementalDiffJob,
655 }731 }
656732
657 @classmethod733 @classmethod
@@ -692,21 +768,24 @@
692 return BranchMergeProposalJobFactory.create(job)768 return BranchMergeProposalJobFactory.create(job)
693769
694 @staticmethod770 @staticmethod
695 def iterReady():771 def iterReady(job_type=None):
696 from lp.code.model.branch import Branch772 from lp.code.model.branch import Branch
697 store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)773 store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
698 SourceBranch = ClassAlias(Branch)774 SourceBranch = ClassAlias(Branch)
699 TargetBranch = ClassAlias(Branch)775 TargetBranch = ClassAlias(Branch)
776 clauses = [
777 BranchMergeProposalJob.job == Job.id,
778 Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]),
779 BranchMergeProposalJob.branch_merge_proposal ==
780 BranchMergeProposal.id, BranchMergeProposal.source_branch ==
781 SourceBranch.id, BranchMergeProposal.target_branch ==
782 TargetBranch.id,
783 ]
784 if job_type is not None:
785 clauses.append(BranchMergeProposalJob.job_type == job_type)
700 jobs = store.find(786 jobs = store.find(
701 (BranchMergeProposalJob, Job, BranchMergeProposal,787 (BranchMergeProposalJob, Job, BranchMergeProposal,
702 SourceBranch, TargetBranch),788 SourceBranch, TargetBranch), And(*clauses))
703 And(BranchMergeProposalJob.job == Job.id,
704 Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]),
705 BranchMergeProposalJob.branch_merge_proposal
706 == BranchMergeProposal.id,
707 BranchMergeProposal.source_branch == SourceBranch.id,
708 BranchMergeProposal.target_branch == TargetBranch.id,
709 ))
710 # Order by the job status first (to get running before waiting), then789 # Order by the job status first (to get running before waiting), then
711 # the date_created, then job type. This should give us all creation790 # the date_created, then job type. This should give us all creation
712 # jobs before comment jobs.791 # jobs before comment jobs.
713792
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposaljobs.py'
--- lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-10-28 22:31:48 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2010-11-05 14:03:38 +0000
@@ -32,6 +32,8 @@
32 IBranchMergeProposalJobSource,32 IBranchMergeProposalJobSource,
33 ICodeReviewCommentEmailJob,33 ICodeReviewCommentEmailJob,
34 ICodeReviewCommentEmailJobSource,34 ICodeReviewCommentEmailJobSource,
35 IGenerateIncrementalDiffJob,
36 IGenerateIncrementalDiffJobSource,
35 IMergeProposalNeedsReviewEmailJob,37 IMergeProposalNeedsReviewEmailJob,
36 IMergeProposalUpdatedEmailJob,38 IMergeProposalUpdatedEmailJob,
37 IMergeProposalUpdatedEmailJobSource,39 IMergeProposalUpdatedEmailJobSource,
@@ -45,12 +47,13 @@
45 BranchMergeProposalJobDerived,47 BranchMergeProposalJobDerived,
46 BranchMergeProposalJobType,48 BranchMergeProposalJobType,
47 CodeReviewCommentEmailJob,49 CodeReviewCommentEmailJob,
50 GenerateIncrementalDiffJob,
48 MergeProposalNeedsReviewEmailJob,51 MergeProposalNeedsReviewEmailJob,
49 MergeProposalUpdatedEmailJob,52 MergeProposalUpdatedEmailJob,
50 ReviewRequestedEmailJob,53 ReviewRequestedEmailJob,
51 UpdatePreviewDiffJob,54 UpdatePreviewDiffJob,
52 )55 )
53from lp.code.model.tests.test_diff import DiffTestCase56from lp.code.model.tests.test_diff import DiffTestCase, create_example_merge
54from lp.code.subscribers.branchmergeproposal import merge_proposal_modified57from lp.code.subscribers.branchmergeproposal import merge_proposal_modified
55from lp.services.job.model.job import Job58from lp.services.job.model.job import Job
56from lp.services.job.runner import JobRunner59from lp.services.job.runner import JobRunner
@@ -192,7 +195,7 @@
192195
193 def test_run(self):196 def test_run(self):
194 self.useBzrBranches(direct_database=True)197 self.useBzrBranches(direct_database=True)
195 bmp = self.createExampleMerge()[0]198 bmp = create_example_merge(self)[0]
196 job = UpdatePreviewDiffJob.create(bmp)199 job = UpdatePreviewDiffJob.create(bmp)
197 self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)200 self.factory.makeRevisionsForBranch(bmp.source_branch, count=1)
198 bmp.source_branch.next_mirror_time = None201 bmp.source_branch.next_mirror_time = None
@@ -226,13 +229,76 @@
226229
227 def test_10_minute_lease(self):230 def test_10_minute_lease(self):
228 self.useBzrBranches(direct_database=True)231 self.useBzrBranches(direct_database=True)
229 bmp = self.createExampleMerge()[0]232 bmp = create_example_merge(self)[0]
230 job = UpdatePreviewDiffJob.create(bmp)233 job = UpdatePreviewDiffJob.create(bmp)
231 job.acquireLease()234 job.acquireLease()
232 expiry_delta = job.lease_expires - datetime.now(pytz.UTC)235 expiry_delta = job.lease_expires - datetime.now(pytz.UTC)
233 self.assertTrue(500 <= expiry_delta.seconds, expiry_delta)236 self.assertTrue(500 <= expiry_delta.seconds, expiry_delta)
234237
235238
239def make_runnable_incremental_diff_job(test_case):
240 test_case.useBzrBranches(direct_database=True)
241 bmp, source_rev_id, target_rev_id = create_example_merge(test_case)
242 repository = bmp.source_branch.getBzrBranch().repository
243 parent_id = repository.get_revision(source_rev_id).parent_ids[0]
244 test_case.factory.makeRevision(rev_id=source_rev_id)
245 test_case.factory.makeRevision(rev_id=parent_id)
246 return GenerateIncrementalDiffJob.create(bmp, parent_id, source_rev_id)
247
248
249class TestGenerateIncrementalDiffJob(DiffTestCase):
250
251 layer = LaunchpadZopelessLayer
252
253 def test_implement_interface(self):
254 """GenerateIncrementalDiffJob implements its interface."""
255 verifyObject(
256 IGenerateIncrementalDiffJobSource, GenerateIncrementalDiffJob)
257
258 def test_providesInterface(self):
259 """MergeProposalCreatedJob provides the expected interfaces."""
260 bmp = self.factory.makeBranchMergeProposal()
261 job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
262 verifyObject(IGenerateIncrementalDiffJob, job)
263 verifyObject(IBranchMergeProposalJob, job)
264
265 def test_getOperationDescription(self):
266 """The description of the job is sane."""
267 bmp = self.factory.makeBranchMergeProposal()
268 job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
269 self.assertEqual(
270 'generating an incremental diff for a merge proposal',
271 job.getOperationDescription())
272
273 def test_run(self):
274 """The job runs successfully, and its results can be committed."""
275 job = make_runnable_incremental_diff_job(self)
276 transaction.commit()
277 self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
278 job.run()
279 transaction.commit()
280
281 def test_run_all(self):
282 """The job can be run under the JobRunner successfully."""
283 job = make_runnable_incremental_diff_job(self)
284 transaction.commit()
285 self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
286 runner = JobRunner([job])
287 runner.runAll()
288 self.assertEqual([job], runner.completed_jobs)
289
290 def test_10_minute_lease(self):
291 """Newly-created jobs have a ten-minute lease."""
292 self.useBzrBranches(direct_database=True)
293 bmp = create_example_merge(self)[0]
294 job = GenerateIncrementalDiffJob.create(bmp, 'old', 'new')
295 transaction.commit()
296 self.layer.switchDbUser(config.merge_proposal_jobs.dbuser)
297 job.acquireLease()
298 expiry_delta = job.lease_expires - datetime.now(pytz.UTC)
299 self.assertTrue(500 <= expiry_delta.seconds, expiry_delta)
300
301
236class TestBranchMergeProposalJobSource(TestCaseWithFactory):302class TestBranchMergeProposalJobSource(TestCaseWithFactory):
237303
238 layer = LaunchpadZopelessLayer304 layer = LaunchpadZopelessLayer
239305
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py 2010-11-05 14:03:28 +0000
+++ lib/lp/code/model/tests/test_diff.py 2010-11-05 14:03:38 +0000
@@ -54,37 +54,41 @@
54 self.records.append(record)54 self.records.append(record)
5555
5656
57def commit_file(branch, path, contents, merge_parents=None):
58 """Create a commit that updates a file to specified contents.
59
60 This will create or modify the file, as needed.
61 """
62 committer = DirectBranchCommit(
63 removeSecurityProxy(branch), no_race_check=True,
64 merge_parents=merge_parents)
65 committer.writeFile(path, contents)
66 try:
67 return committer.commit('committing')
68 finally:
69 committer.unlock()
70
71
72def create_example_merge(test_case):
73 """Create a merge proposal with conflicts and updates."""
74 bmp = test_case.factory.makeBranchMergeProposal()
75 # Make the branches of the merge proposal look good as far as the
76 # model is concerned.
77 test_case.factory.makeRevisionsForBranch(bmp.source_branch)
78 test_case.factory.makeRevisionsForBranch(bmp.target_branch)
79 bzr_target = test_case.createBzrBranch(bmp.target_branch)
80 commit_file(bmp.target_branch, 'foo', 'a\n')
81 test_case.createBzrBranch(bmp.source_branch, bzr_target)
82 source_rev_id = commit_file(bmp.source_branch, 'foo', 'd\na\nb\n')
83 target_rev_id = commit_file(bmp.target_branch, 'foo', 'c\na\n')
84 return bmp, source_rev_id, target_rev_id
85
86
57class DiffTestCase(TestCaseWithFactory):87class DiffTestCase(TestCaseWithFactory):
5888
59 @staticmethod
60 def commitFile(branch, path, contents, merge_parents=None):
61 """Create a commit that updates a file to specified contents.
62
63 This will create or modify the file, as needed.
64 """
65 committer = DirectBranchCommit(
66 removeSecurityProxy(branch), no_race_check=True,
67 merge_parents=merge_parents)
68 committer.writeFile(path, contents)
69 try:
70 return committer.commit('committing')
71 finally:
72 committer.unlock()
73
74 def createExampleMerge(self):89 def createExampleMerge(self):
75 """Create a merge proposal with conflicts and updates."""
76 self.useBzrBranches(direct_database=True)90 self.useBzrBranches(direct_database=True)
77 bmp = self.factory.makeBranchMergeProposal()91 return create_example_merge(self)
78 # Make the branches of the merge proposal look good as far as the
79 # model is concerned.
80 self.factory.makeRevisionsForBranch(bmp.source_branch)
81 self.factory.makeRevisionsForBranch(bmp.target_branch)
82 bzr_target = self.createBzrBranch(bmp.target_branch)
83 self.commitFile(bmp.target_branch, 'foo', 'a\n')
84 self.createBzrBranch(bmp.source_branch, bzr_target)
85 source_rev_id = self.commitFile(bmp.source_branch, 'foo', 'd\na\nb\n')
86 target_rev_id = self.commitFile(bmp.target_branch, 'foo', 'c\na\n')
87 return bmp, source_rev_id, target_rev_id
8892
89 def checkExampleMerge(self, diff_text):93 def checkExampleMerge(self, diff_text):
90 """Ensure the diff text matches the values for ExampleMerge."""94 """Ensure the diff text matches the values for ExampleMerge."""
@@ -111,12 +115,12 @@
111 source = bmp.source_branch115 source = bmp.source_branch
112 prerequisite = bmp.prerequisite_branch116 prerequisite = bmp.prerequisite_branch
113 target_bzr = self.createBzrBranch(target)117 target_bzr = self.createBzrBranch(target)
114 self.commitFile(target, 'file', 'target text\n')118 commit_file(target, 'file', 'target text\n')
115 prerequisite_bzr = self.createBzrBranch(prerequisite, target_bzr)119 prerequisite_bzr = self.createBzrBranch(prerequisite, target_bzr)
116 self.commitFile(120 commit_file(
117 prerequisite, 'file', 'target text\nprerequisite text\n')121 prerequisite, 'file', 'target text\nprerequisite text\n')
118 source_bzr = self.createBzrBranch(source, prerequisite_bzr)122 source_bzr = self.createBzrBranch(source, prerequisite_bzr)
119 source_rev_id = self.commitFile(123 source_rev_id = commit_file(
120 source, 'file',124 source, 'file',
121 'target text\nprerequisite text\nsource text\n')125 'target text\nprerequisite text\nsource text\n')
122 return (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,126 return (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,
@@ -243,7 +247,7 @@
243 # affect the diff.247 # affect the diff.
244 (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,248 (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,
245 prerequisite) = self.preparePrerequisiteMerge()249 prerequisite) = self.preparePrerequisiteMerge()
246 self.commitFile(250 commit_file(
247 prerequisite, 'file', 'prerequisite text2\n')251 prerequisite, 'file', 'prerequisite text2\n')
248 diff, conflicts = Diff.mergePreviewFromBranches(252 diff, conflicts = Diff.mergePreviewFromBranches(
249 source_bzr, source_rev_id, target_bzr, prerequisite_bzr)253 source_bzr, source_rev_id, target_bzr, prerequisite_bzr)
@@ -459,10 +463,10 @@
459 self.useBzrBranches(direct_database=True)463 self.useBzrBranches(direct_database=True)
460 bmp = self.factory.makeBranchMergeProposal()464 bmp = self.factory.makeBranchMergeProposal()
461 bzr_target = self.createBzrBranch(bmp.target_branch)465 bzr_target = self.createBzrBranch(bmp.target_branch)
462 self.commitFile(bmp.target_branch, 'foo', 'a\n')466 commit_file(bmp.target_branch, 'foo', 'a\n')
463 self.createBzrBranch(bmp.source_branch, bzr_target)467 self.createBzrBranch(bmp.source_branch, bzr_target)
464 source_rev_id = self.commitFile(bmp.source_branch, 'foo', 'a\nb\n')468 source_rev_id = commit_file(bmp.source_branch, 'foo', 'a\nb\n')
465 target_rev_id = self.commitFile(bmp.target_branch, 'foo', 'c\na\n')469 target_rev_id = commit_file(bmp.target_branch, 'foo', 'c\na\n')
466 diff = PreviewDiff.fromBranchMergeProposal(bmp)470 diff = PreviewDiff.fromBranchMergeProposal(bmp)
467 self.assertEqual('', diff.conflicts)471 self.assertEqual('', diff.conflicts)
468 self.assertFalse(diff.has_conflicts)472 self.assertFalse(diff.has_conflicts)
@@ -575,25 +579,22 @@
575 bmp = self.factory.makeBranchMergeProposal(579 bmp = self.factory.makeBranchMergeProposal(
576 prerequisite_branch=prerequisite_branch)580 prerequisite_branch=prerequisite_branch)
577 target_branch = self.createBzrBranch(bmp.target_branch)581 target_branch = self.createBzrBranch(bmp.target_branch)
578 old_revision_id = self.commitFile(582 old_revision_id = commit_file(bmp.target_branch, 'foo', 'a\nb\ne\n')
579 bmp.target_branch, 'foo', 'a\nb\ne\n')
580 old_revision = self.factory.makeRevision(rev_id=old_revision_id)583 old_revision = self.factory.makeRevision(rev_id=old_revision_id)
581 source_branch = self.createBzrBranch(584 source_branch = self.createBzrBranch(
582 bmp.source_branch, target_branch)585 bmp.source_branch, target_branch)
583 self.commitFile(bmp.source_branch, 'foo', 'a\nc\ne\n')586 commit_file(bmp.source_branch, 'foo', 'a\nc\ne\n')
584 prerequisite = self.createBzrBranch(587 prerequisite = self.createBzrBranch(
585 bmp.prerequisite_branch, target_branch)588 bmp.prerequisite_branch, target_branch)
586 prerequisite_revision = self.commitFile(bmp.prerequisite_branch,589 prerequisite_revision = commit_file(
587 'foo', 'd\na\nb\ne\n')590 bmp.prerequisite_branch, 'foo', 'd\na\nb\ne\n')
588 merge_parent = self.commitFile(bmp.target_branch, 'foo',591 merge_parent = commit_file(bmp.target_branch, 'foo', 'a\nb\ne\nf\n')
589 'a\nb\ne\nf\n')
590 source_branch.repository.fetch(target_branch.repository,592 source_branch.repository.fetch(target_branch.repository,
591 revision_id=merge_parent)593 revision_id=merge_parent)
592 self.commitFile(594 commit_file(bmp.source_branch, 'foo', 'a\nc\ne\nf\n', [merge_parent])
593 bmp.source_branch, 'foo', 'a\nc\ne\nf\n', [merge_parent])
594 source_branch.repository.fetch(prerequisite.repository,595 source_branch.repository.fetch(prerequisite.repository,
595 revision_id=prerequisite_revision)596 revision_id=prerequisite_revision)
596 new_revision_id = self.commitFile(597 new_revision_id = commit_file(
597 bmp.source_branch, 'foo', 'd\na\nc\ne\nf\n',598 bmp.source_branch, 'foo', 'd\na\nc\ne\nf\n',
598 [prerequisite_revision])599 [prerequisite_revision])
599 new_revision = self.factory.makeRevision(rev_id=new_revision_id)600 new_revision = self.factory.makeRevision(rev_id=new_revision_id)
600601
=== modified file 'lib/lp/code/scripts/tests/test_merge_proposal_jobs.py'
--- lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2010-10-04 19:50:45 +0000
+++ lib/lp/code/scripts/tests/test_merge_proposal_jobs.py 2010-11-05 14:03:38 +0000
@@ -7,8 +7,14 @@
77
8import unittest8import unittest
99
10import transaction
11
10from canonical.launchpad.scripts.tests import run_script12from canonical.launchpad.scripts.tests import run_script
11from canonical.testing.layers import ZopelessAppServerLayer13from canonical.testing.layers import ZopelessAppServerLayer
14from lp.code.model.tests.test_branchmergeproposaljobs import (
15 make_runnable_incremental_diff_job
16)
17from lp.services.job.interfaces.job import JobStatus
12from lp.testing import TestCaseWithFactory18from lp.testing import TestCaseWithFactory
1319
1420
@@ -18,6 +24,8 @@
1824
19 def test_script_runs(self):25 def test_script_runs(self):
20 """Ensure merge-proposal-jobs script runs."""26 """Ensure merge-proposal-jobs script runs."""
27 job = make_runnable_incremental_diff_job(self)
28 transaction.commit()
21 retcode, stdout, stderr = run_script(29 retcode, stdout, stderr = run_script(
22 'cronscripts/merge-proposal-jobs.py', [])30 'cronscripts/merge-proposal-jobs.py', [])
23 self.assertEqual(0, retcode)31 self.assertEqual(0, retcode)
@@ -25,7 +33,10 @@
25 self.assertEqual(33 self.assertEqual(
26 'INFO Creating lockfile:'34 'INFO Creating lockfile:'
27 ' /var/lock/launchpad-merge-proposal-jobs.lock\n'35 ' /var/lock/launchpad-merge-proposal-jobs.lock\n'
28 'INFO Running through Twisted.\n', stderr)36 'INFO Running through Twisted.\n'
37 'INFO Ran 1 GenerateIncrementalDiffJob jobs.\n', stderr)
38 self.assertEqual(JobStatus.COMPLETED, job.status)
39
2940
30def test_suite():41def test_suite():
31 return unittest.TestLoader().loadTestsFromName(__name__)42 return unittest.TestLoader().loadTestsFromName(__name__)
3243
=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-10-26 15:47:24 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2010-11-05 14:03:38 +0000
@@ -31,11 +31,14 @@
31from lp.code.interfaces.branchlookup import IBranchLookup31from lp.code.interfaces.branchlookup import IBranchLookup
32from lp.code.interfaces.revision import IRevisionSet32from lp.code.interfaces.revision import IRevisionSet
33from lp.code.model.branchrevision import BranchRevision33from lp.code.model.branchrevision import BranchRevision
34from lp.code.model.branchmergeproposaljob import (
35 BranchMergeProposalJobSource, BranchMergeProposalJobType)
34from lp.code.model.revision import (36from lp.code.model.revision import (
35 Revision,37 Revision,
36 RevisionAuthor,38 RevisionAuthor,
37 RevisionParent,39 RevisionParent,
38 )40 )
41from lp.code.model.tests.test_diff import commit_file
39from lp.codehosting.bzrutils import write_locked42from lp.codehosting.bzrutils import write_locked
40from lp.codehosting.scanner.bzrsync import BzrSync43from lp.codehosting.scanner.bzrsync import BzrSync
41from lp.services.osutils import override_environ44from lp.services.osutils import override_environ
@@ -654,6 +657,37 @@
654 self.assertIsNot(None, bmp.next_preview_diff_job)657 self.assertIsNot(None, bmp.next_preview_diff_job)
655658
656659
660class TestGenerateIncrementalDiffJob(BzrSyncTestCase):
661 """Test the scheduling of GenerateIncrementalDiffJobs."""
662
663 def getPending(self):
664 return list(
665 BranchMergeProposalJobSource.iterReady(
666 BranchMergeProposalJobType.GENERATE_INCREMENTAL_DIFF
667 )
668 )
669
670 @run_as_db_user(config.launchpad.dbuser)
671 def test_create_on_new_revision(self):
672 """When branch tip changes, a job is created."""
673 parent_id = commit_file(self.db_branch, 'foo', 'bar')
674 self.factory.makeBranchRevision(self.db_branch, parent_id,
675 revision_date=self.factory.getUniqueDate())
676 self.db_branch.last_scanned_id = parent_id
677 bmp = self.factory.makeBranchMergeProposal(
678 source_branch=self.db_branch,
679 date_created=self.factory.getUniqueDate())
680 revision_id = commit_file(self.db_branch, 'foo', 'baz')
681 removeSecurityProxy(bmp).target_branch.last_scanned_id = 'rev'
682 self.assertEqual([], self.getPending())
683 transaction.commit()
684 LaunchpadZopelessLayer.switchDbUser(config.branchscanner.dbuser)
685 self.makeBzrSync(self.db_branch).syncBranchAndClose()
686 (job,) = self.getPending()
687 self.assertEqual(revision_id, job.new_revision_id)
688 self.assertEqual(parent_id, job.old_revision_id)
689
690
657class TestSetRecipeStale(BzrSyncTestCase):691class TestSetRecipeStale(BzrSyncTestCase):
658 """Test recipes associated with the branch are marked stale."""692 """Test recipes associated with the branch are marked stale."""
659693

Subscribers

People subscribed via source and target branches

to status/vote changes: