launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18418
[Merge] lp:~cjwatson/launchpad/git-mp-diffs into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-mp-diffs into lp:launchpad with lp:~cjwatson/launchpad/git-mp-register as a prerequisite.
Commit message:
Generate preview diffs for Git-based merge proposals.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1445017 in Launchpad itself: "Support for Launchpad Git merge proposals "
https://bugs.launchpad.net/launchpad/+bug/1445017
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-mp-diffs/+merge/257768
Generate preview diffs for Git-based merge proposals.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-mp-diffs into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2015-04-21 10:34:24 +0000
+++ database/schema/security.cfg 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
#
# Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -1937,6 +1937,8 @@
public.distribution = SELECT
public.distroseries = SELECT
public.emailaddress = SELECT
+public.gitref = SELECT
+public.gitrepository = SELECT
public.incrementaldiff = SELECT, INSERT
public.job = SELECT, INSERT, UPDATE
public.karma = SELECT, INSERT
=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py 2013-12-12 05:06:11 +0000
+++ lib/lp/app/browser/stringformatter.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""TALES formatter for strings."""
@@ -884,7 +884,9 @@
for row, line in enumerate(text.splitlines()[:max_format_lines]):
result.append('<tr id="diff-line-%s">' % (row + 1))
result.append('<td class="line-no">%s</td>' % (row + 1))
- if line.startswith('==='):
+ if (line.startswith('===') or
+ line.startswith('diff') or
+ line.startswith('index')):
css_class = 'diff-file text'
header_next = True
elif (header_next and
=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py 2013-12-13 02:09:14 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for the string TALES formatter."""
@@ -165,7 +165,7 @@
class TestLinkifyingProtocols(TestCaseWithFactory):
-
+
layer = DatabaseFunctionalLayer
def test_normal_set(self):
@@ -357,6 +357,40 @@
'diff-comment text'],
[str(tag['class']) for tag in text])
+ def test_cssClasses_git(self):
+ # Git diffs look slightly different, so check that they also end up
+ # with the correct CSS classes.
+ diff = dedent('''\
+ diff --git a/tales.py b/tales.py
+ index aaaaaaa..bbbbbbb 100644
+ --- a/tales.py
+ +++ b/tales.py
+ @@ -2435,6 +2435,8 @@
+ def format_diff(self):
+ - removed this line
+ + added this line
+ -------- a sql style comment
+ ++++++++ a line of pluses
+ ''')
+ html = FormattersAPI(diff).format_diff()
+ line_numbers = find_tags_by_class(html, 'line-no')
+ self.assertEqual(
+ ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10'],
+ [tag.renderContents() for tag in line_numbers])
+ text = find_tags_by_class(html, 'text')
+ self.assertEqual(
+ ['diff-file text',
+ 'diff-file text',
+ 'diff-header text',
+ 'diff-header text',
+ 'diff-chunk text',
+ 'text',
+ 'diff-removed text',
+ 'diff-added text',
+ 'diff-removed text',
+ 'diff-added text'],
+ [str(tag['class']) for tag in text])
+
def test_config_value_limits_line_count(self):
# The config.diff.max_line_format contains the maximum number of lines
# to format.
=== modified file 'lib/lp/code/githosting.py'
--- lib/lp/code/githosting.py 2015-03-31 00:59:44 +0000
+++ lib/lp/code/githosting.py 2015-04-29 14:40:47 +0000
@@ -101,3 +101,35 @@
except ValueError as e:
raise GitRepositoryScanFault(
"Failed to decode commit-scan response: %s" % unicode(e))
+
+ def getMergeDiff(self, path, base, head, logger=None):
+ """Get the merge preview diff between two commits.
+
+ :return: A dict mapping 'commits' to a list of commits between
+ 'base' and 'head' (formatted as with `getCommits`), 'patch' to
+ the text of the diff between 'base' and 'head', and 'conflicts'
+ to a list of conflicted paths.
+ """
+ try:
+ if logger is not None:
+ logger.info(
+ "Requesting merge diff for %s from %s to %s" % (
+ path, base, head))
+ response = self._makeSession().get(
+ urlutils.join(
+ self.endpoint, "repo", path, "compare-merge",
+ "%s:%s" % (base, head)),
+ timeout=self.timeout)
+ except Exception as e:
+ raise GitRepositoryScanFault(
+ "Failed to get merge diff from Git repository: %s" %
+ unicode(e))
+ if response.status_code != 200:
+ raise GitRepositoryScanFault(
+ "Failed to get merge diff from Git repository: %s" %
+ unicode(e))
+ try:
+ return response.json()
+ except ValueError as e:
+ raise GitRepositoryScanFault(
+ "Failed to decode merge-diff response: %s" % unicode(e))
=== modified file 'lib/lp/code/model/branchmergeproposaljob.py'
--- lib/lp/code/model/branchmergeproposaljob.py 2014-05-28 12:23:10 +0000
+++ lib/lp/code/model/branchmergeproposaljob.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Job classes related to BranchMergeProposals are in here.
@@ -21,6 +21,7 @@
'UpdatePreviewDiffJob',
]
+from contextlib import contextmanager
from datetime import (
datetime,
timedelta,
@@ -39,7 +40,6 @@
Desc,
Or,
)
-from storm.info import ClassAlias
from storm.locals import (
Int,
Reference,
@@ -102,6 +102,7 @@
from lp.services.mail.sendmail import format_address_for_person
from lp.services.webapp import canonical_url
+
class BranchMergeProposalJobType(DBEnumeratedType):
"""Values that ICodeImportJob.state can take."""
@@ -235,7 +236,7 @@
return '<%(job_type)s job for merge %(merge_id)s on %(branch)s>' % {
'job_type': self.context.job_type.name,
'merge_id': bmp.id,
- 'branch': bmp.source_branch.unique_name,
+ 'branch': bmp.merge_source.unique_name,
}
@classmethod
@@ -278,12 +279,13 @@
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.
- Branch.revision_count > 0,
- Or(Branch.next_mirror_time == None,
- Branch.branch_type != BranchType.HOSTED)))
+ Or(And(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.
+ Branch.revision_count > 0,
+ Or(Branch.next_mirror_time == None,
+ Branch.branch_type != BranchType.HOSTED)),
+ BranchMergeProposal.source_git_repository != None)))
return (klass(job) for job in jobs)
def getOopsVars(self):
@@ -293,8 +295,8 @@
vars.extend([
('branchmergeproposal_job_id', self.context.id),
('branchmergeproposal_job_type', self.context.job_type.title),
- ('source_branch', bmp.source_branch.unique_name),
- ('target_branch', bmp.target_branch.unique_name)])
+ ('source_branch', bmp.merge_source.unique_name),
+ ('target_branch', bmp.merge_target.unique_name)])
return vars
@@ -320,8 +322,8 @@
def getOperationDescription(self):
return ('notifying people about the proposal to merge %s into %s' %
- (self.branch_merge_proposal.source_branch.bzr_identity,
- self.branch_merge_proposal.target_branch.bzr_identity))
+ (self.branch_merge_proposal.merge_source.identity,
+ self.branch_merge_proposal.merge_target.identity))
class UpdatePreviewDiffJob(BranchMergeProposalJobDerived):
@@ -350,15 +352,23 @@
"""Is this job ready to run?"""
bmp = self.branch_merge_proposal
url = canonical_url(bmp)
- if bmp.source_branch.last_scanned_id is None:
- raise UpdatePreviewDiffNotReady(
- 'The source branch of %s has no revisions.' % url)
- if bmp.target_branch.last_scanned_id is None:
- raise UpdatePreviewDiffNotReady(
- 'The target branch of %s has no revisions.' % url)
- if bmp.source_branch.pending_writes:
- raise BranchHasPendingWrites(
- 'The source branch of %s has pending writes.' % url)
+ if bmp.source_branch is not None:
+ if bmp.source_branch.last_scanned_id is None:
+ raise UpdatePreviewDiffNotReady(
+ 'The source branch of %s has no revisions.' % url)
+ if bmp.target_branch.last_scanned_id is None:
+ raise UpdatePreviewDiffNotReady(
+ 'The target branch of %s has no revisions.' % url)
+ if bmp.source_branch.pending_writes:
+ raise BranchHasPendingWrites(
+ 'The source branch of %s has pending writes.' % url)
+ else:
+ if bmp.source_git_ref.commit_sha1 is None:
+ raise UpdatePreviewDiffNotReady(
+ 'The source branch of %s has not been scanned.' % url)
+ if bmp.target_git_ref.commit_sha1 is None:
+ raise UpdatePreviewDiffNotReady(
+ 'The source branch of %s has not been scanned.' % url)
def acquireLease(self, duration=600):
return self.job.acquireLease(duration)
@@ -366,7 +376,13 @@
def run(self):
"""See `IRunnableJob`."""
self.checkReady()
- with server(get_ro_server(), no_replace=True):
+ if self.branch_merge_proposal.source_branch is not None:
+ server_context = server(get_ro_server(), no_replace=True)
+ else:
+ # A no-op context manager. (This could be simplified with
+ # contextlib.ExitStack from Python 3.3.)
+ server_context = contextmanager(lambda: (None for _ in [None]))()
+ with server_context:
with BranchMergeProposalDelta.monitor(self.branch_merge_proposal):
PreviewDiff.fromBranchMergeProposal(self.branch_merge_proposal)
@@ -655,22 +671,16 @@
@staticmethod
def iterReady(job_type=None):
- from lp.code.model.branch import Branch
- SourceBranch = ClassAlias(Branch)
- TargetBranch = ClassAlias(Branch)
clauses = [
BranchMergeProposalJob.job == Job.id,
Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]),
BranchMergeProposalJob.branch_merge_proposal ==
- BranchMergeProposal.id, BranchMergeProposal.source_branch ==
- SourceBranch.id, BranchMergeProposal.target_branch ==
- TargetBranch.id,
+ BranchMergeProposal.id,
]
if job_type is not None:
clauses.append(BranchMergeProposalJob.job_type == job_type)
- jobs = IMasterStore(Branch).find(
- (BranchMergeProposalJob, Job, BranchMergeProposal,
- SourceBranch, TargetBranch), And(*clauses))
+ jobs = IMasterStore(BranchMergeProposalJob).find(
+ (BranchMergeProposalJob, Job, BranchMergeProposal), And(*clauses))
# Order by the job status first (to get running before waiting), then
# the date_created, then job type. This should give us all creation
# jobs before comment jobs.
@@ -680,7 +690,7 @@
# Now only return one job for any given merge proposal.
ready_jobs = []
seen_merge_proposals = set()
- for bmp_job, job, bmp, source, target in jobs:
+ for bmp_job, job, bmp in jobs:
# If we've seen this merge proposal already, skip this job.
if bmp.id in seen_merge_proposals:
continue
=== modified file 'lib/lp/code/model/diff.py'
--- lib/lp/code/model/diff.py 2014-02-26 13:38:39 +0000
+++ lib/lp/code/model/diff.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementation classes for IDiff, etc."""
@@ -41,6 +41,7 @@
from zope.interface import implements
from lp.app.errors import NotFoundError
+from lp.code.githosting import GitHostingClient
from lp.code.interfaces.diff import (
IDiff,
IIncrementalDiff,
@@ -233,7 +234,7 @@
return cls.fromFile(diff_content, size, filename)
@classmethod
- def fromFile(cls, diff_content, size, filename=None):
+ def fromFile(cls, diff_content, size, filename=None, strip_slashes=0):
"""Create a Diff from a textual diff.
:diff_content: The diff text
@@ -254,7 +255,8 @@
diff_content_bytes = diff_content.read(size)
diff_lines_count = len(diff_content_bytes.strip().split('\n'))
try:
- diffstat = cls.generateDiffstat(diff_content_bytes)
+ diffstat = cls.generateDiffstat(
+ diff_content_bytes, strip_slashes=strip_slashes)
except Exception:
getUtility(IErrorReportingUtility).raising(sys.exc_info())
# Set the diffstat to be empty.
@@ -272,10 +274,13 @@
removed_lines_count=removed_lines_count)
@staticmethod
- def generateDiffstat(diff_bytes):
+ def generateDiffstat(diff_bytes, strip_slashes=0):
"""Generate statistics about the provided diff.
:param diff_bytes: A unified diff, as bytes.
+ :param strip_slashes: Strip the smallest prefix containing this many
+ leading slashes from each file name found in the patch file, as
+ with "patch -p".
:return: A map of {filename: (added_line_count, removed_line_count)}
"""
file_stats = {}
@@ -285,6 +290,8 @@
if not isinstance(patch, Patch):
continue
path = patch.newname.split('\t')[0]
+ if strip_slashes:
+ path = path.split('/', strip_slashes)[-1]
file_stats[path] = tuple(patch.stats_values()[:2])
return file_stats
@@ -378,27 +385,39 @@
# diff was generated for absent branch revisions (e.g. the source
# or target branch was overwritten). Which means some entries for
# the same BMP may have much wider titles depending on the
- # branch history. It is particulary bad for rendering the diff
+ # branch history. It is particularly bad for rendering the diff
# navigator 'select' widget in the UI.
- source_rev = bmp.source_branch.getBranchRevision(
- revision_id=self.source_revision_id)
- if source_rev and source_rev.sequence:
- source_revno = u'r{0}'.format(source_rev.sequence)
- else:
- source_revno = self.source_revision_id
- target_rev = bmp.target_branch.getBranchRevision(
- revision_id=self.target_revision_id)
- if target_rev and target_rev.sequence:
- target_revno = u'r{0}'.format(target_rev.sequence)
- else:
- target_revno = self.target_revision_id
+ if bmp.source_branch is not None:
+ source_revision = bmp.source_branch.getBranchRevision(
+ revision_id=self.source_revision_id)
+ if source_revision and source_revision.sequence:
+ source_rev = u'r{0}'.format(source_revision.sequence)
+ else:
+ source_rev = self.source_revision_id
+ target_revision = bmp.target_branch.getBranchRevision(
+ revision_id=self.target_revision_id)
+ if target_revision and target_revision.sequence:
+ target_rev = u'r{0}'.format(target_revision.sequence)
+ else:
+ target_rev = self.target_revision_id
+ else:
+ # For Git, we shorten to seven characters since that's usual.
+ # We should perhaps shorten only as far as preserves uniqueness,
+ # but that requires talking to the hosting service and it's
+ # unlikely to be a problem in practice.
+ source_rev = self.source_revision_id[:7]
+ target_rev = self.target_revision_id[:7]
- return u'{0} into {1}'.format(source_revno, target_revno)
+ return u'{0} into {1}'.format(source_rev, target_rev)
@property
def has_conflicts(self):
return self.conflicts is not None and self.conflicts != ''
+ @staticmethod
+ def getGitHostingClient():
+ return GitHostingClient(config.codehosting.internal_git_api_endpoint)
+
@classmethod
def fromBranchMergeProposal(cls, bmp):
"""Create a `PreviewDiff` from a `BranchMergeProposal`.
@@ -407,34 +426,60 @@
:param bmp: The `BranchMergeProposal` to generate a `PreviewDiff` for.
:return: A `PreviewDiff`.
"""
- source_branch = bmp.source_branch.getBzrBranch()
- source_revision = source_branch.last_revision()
- target_branch = bmp.target_branch.getBzrBranch()
- target_revision = target_branch.last_revision()
- if bmp.prerequisite_branch is not None:
- prerequisite_branch = bmp.prerequisite_branch.getBzrBranch()
+ if bmp.source_branch is not None:
+ source_branch = bmp.source_branch.getBzrBranch()
+ source_revision = source_branch.last_revision()
+ target_branch = bmp.target_branch.getBzrBranch()
+ target_revision = target_branch.last_revision()
+ if bmp.prerequisite_branch is not None:
+ prerequisite_branch = bmp.prerequisite_branch.getBzrBranch()
+ else:
+ prerequisite_branch = None
+ diff, conflicts = Diff.mergePreviewFromBranches(
+ source_branch, source_revision, target_branch,
+ prerequisite_branch)
+ preview = cls()
+ preview.source_revision_id = source_revision.decode('utf-8')
+ preview.target_revision_id = target_revision.decode('utf-8')
+ preview.branch_merge_proposal = bmp
+ preview.diff = diff
+ preview.conflicts = u''.join(
+ unicode(conflict) + '\n' for conflict in conflicts)
else:
- prerequisite_branch = None
- diff, conflicts = Diff.mergePreviewFromBranches(
- source_branch, source_revision, target_branch, prerequisite_branch)
- preview = cls()
- preview.source_revision_id = source_revision.decode('utf-8')
- preview.target_revision_id = target_revision.decode('utf-8')
- preview.branch_merge_proposal = bmp
- preview.diff = diff
- preview.conflicts = u''.join(
- unicode(conflict) + '\n' for conflict in conflicts)
+ hosting_client = cls.getGitHostingClient()
+ source_repository = bmp.source_git_repository
+ target_repository = bmp.target_git_repository
+ if source_repository == target_repository:
+ path = source_repository.getInternalPath()
+ else:
+ path = "%s:%s" % (
+ target_repository.getInternalPath(),
+ source_repository.getInternalPath())
+ response = hosting_client.getMergeDiff(
+ path, bmp.source_git_ref.commit_sha1,
+ bmp.target_git_ref.commit_sha1)
+ source_revision_id = bmp.source_git_ref.commit_sha1
+ target_revision_id = bmp.target_git_ref.commit_sha1
+ if bmp.prerequisite_git_ref is not None:
+ prerequisite_revision_id = bmp.prerequisite_git_ref.commit_sha1
+ else:
+ prerequisite_revision_id = None
+ conflicts = u"".join(
+ u"Conflict in %s\n" % path for path in response['conflicts'])
+ preview = cls.create(
+ bmp, response['patch'], source_revision_id, target_revision_id,
+ prerequisite_revision_id, conflicts, strip_slashes=1)
del get_property_cache(bmp).preview_diffs
del get_property_cache(bmp).preview_diff
return preview
@classmethod
def create(cls, bmp, diff_content, source_revision_id, target_revision_id,
- prerequisite_revision_id, conflicts):
+ prerequisite_revision_id, conflicts, strip_slashes=0):
"""Create a PreviewDiff with specified values.
:param bmp: The `BranchMergeProposal` this diff references.
- :param diff_content: The text of the dift, as bytes.
+ :param diff_content: The text of the diff, as bytes.
:param source_revision_id: The revision_id of the source branch.
:param target_revision_id: The revision_id of the target branch.
:param prerequisite_revision_id: The revision_id of the prerequisite
@@ -444,7 +489,9 @@
"""
filename = str(uuid1()) + '.txt'
size = len(diff_content)
- diff = Diff.fromFile(StringIO(diff_content), size, filename)
+ diff = Diff.fromFile(
+ StringIO(diff_content), size, filename,
+ strip_slashes=strip_slashes)
preview = cls()
preview.branch_merge_proposal = bmp
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposaljobs.py'
--- lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2014-05-29 07:08:17 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposaljobs.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for branch merge proposal jobs."""
@@ -38,6 +38,7 @@
IUpdatePreviewDiffJob,
IUpdatePreviewDiffJobSource,
)
+from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
from lp.code.model.branchmergeproposaljob import (
BranchMergeProposalJob,
BranchMergeProposalJobDerived,
@@ -219,6 +220,14 @@
JobRunner([job]).runAll()
self.checkExampleMerge(bmp.preview_diff.text)
+ def test_run_git(self):
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+ bmp, _, _, patch = self.createExampleGitMerge()
+ job = UpdatePreviewDiffJob.create(bmp)
+ with dbuser("merge-proposal-jobs"):
+ JobRunner([job]).runAll()
+ self.assertEqual(patch, bmp.preview_diff.text)
+
def test_run_object_events(self):
# While the job runs a single IObjectModifiedEvent is issued when the
# preview diff has been calculated.
@@ -514,6 +523,16 @@
self.assertEqual(job.branch_merge_proposal, bmp)
self.assertIsInstance(job, MergeProposalUpdatedEmailJob)
+ def test_iterReady_supports_git(self):
+ # iterReady supports merge proposals based on Git. (These are
+ # currently considered ready regardless of scanning, since the hard
+ # work is done by the backend.)
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ [job] = self.job_source.iterReady()
+ self.assertEqual(bmp, job.branch_merge_proposal)
+ self.assertIsInstance(job, UpdatePreviewDiffJob)
+
class TestCodeReviewCommentEmailJob(TestCaseWithFactory):
=== modified file 'lib/lp/code/model/tests/test_diff.py'
--- lib/lp/code/model/tests/test_diff.py 2014-02-26 13:38:39 +0000
+++ lib/lp/code/model/tests/test_diff.py 2015-04-29 14:40:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Diff, etc."""
@@ -8,6 +8,7 @@
from cStringIO import StringIO
from difflib import unified_diff
import logging
+from textwrap import dedent
from bzrlib import trace
from bzrlib.patches import (
@@ -15,6 +16,7 @@
parse_patches,
RemoveLine,
)
+from fixtures import MonkeyPatch
import transaction
from zope.security.proxy import removeSecurityProxy
@@ -24,11 +26,13 @@
IIncrementalDiff,
IPreviewDiff,
)
+from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
from lp.code.model.diff import (
Diff,
PreviewDiff,
)
from lp.code.model.directbranchcommit import DirectBranchCommit
+from lp.services.features.testing import FeatureFixture
from lp.services.librarian.interfaces.client import (
LIBRARIAN_SERVER_DEFAULT_TIMEOUT,
)
@@ -40,6 +44,7 @@
TestCaseWithFactory,
verifyObject,
)
+from lp.testing.fakemethod import FakeMethod
from lp.testing.layers import (
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
@@ -86,6 +91,10 @@
return bmp, source_rev_id, target_rev_id
+class FakeGitHostingClient:
+ pass
+
+
class DiffTestCase(TestCaseWithFactory):
def createExampleMerge(self):
@@ -128,6 +137,44 @@
return (source_bzr, source_rev_id, target_bzr, prerequisite_bzr,
prerequisite)
+ def installFakeGitMergeDiff(self, result=None, failure=None):
+ hosting_client = FakeGitHostingClient()
+ hosting_client.getMergeDiff = FakeMethod(
+ result=result, failure=failure)
+ self.useFixture(MonkeyPatch(
+ "lp.code.model.diff.PreviewDiff.getGitHostingClient",
+ staticmethod(lambda: hosting_client)))
+
+ def createExampleGitMerge(self):
+ """Create an example Git-based merge scenario.
+
+ The actual diff work is done in turnip, and we have no turnip
+ fixture as yet, so for the moment we just install a fake hosting
+ client that will return a plausible-looking diff.
+ """
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ source_sha1 = bmp.source_git_ref.commit_sha1
+ target_sha1 = bmp.target_git_ref.commit_sha1
+ patch = dedent("""\
+ diff --git a/foo b/foo
+ index %s..%s 100644
+ --- a/foo
+ +++ b/foo
+ @@ -1,2 +1,7 @@
+ +<<<<<<< foo
+ c
+ +=======
+ +d
+ +>>>>>>> foo
+ a
+ +b
+ """) % (target_sha1[:7], source_sha1[:7])
+ self.installFakeGitMergeDiff(result={
+ "patch": patch,
+ "conflicts": ["foo"],
+ })
+ return bmp, source_sha1, target_sha1, patch
+
class TestDiff(DiffTestCase):
@@ -329,6 +376,10 @@
layer = LaunchpadFunctionalLayer
+ def setUp(self):
+ super(TestPreviewDiff, self).setUp()
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+
def _createProposalWithPreviewDiff(self, prerequisite_branch=None,
content=None):
# Create and return a preview diff.
@@ -348,6 +399,24 @@
transaction.commit()
return mp
+ def _createGitProposalWithPreviewDiff(self, merge_prerequisite=None,
+ content=None):
+ mp = self.factory.makeBranchMergeProposalForGit(
+ prerequisite_ref=merge_prerequisite)
+ login_person(mp.registrant)
+ if merge_prerequisite is None:
+ prerequisite_revision_id = None
+ else:
+ prerequisite_revision_id = u"c" * 40
+ if content is None:
+ content = "".join(unified_diff("", "content"))
+ mp.updatePreviewDiff(
+ content, u"a" * 40, u"b" * 40,
+ prerequisite_revision_id=prerequisite_revision_id)
+ # Make sure the librarian file is written.
+ transaction.commit()
+ return mp
+
def test_providesInterface(self):
# In order to test the interface provision, we need to make sure that
# the associated diff object that is delegated to is also created.
@@ -365,7 +434,7 @@
def test_title(self):
# PreviewDiff has title and it copes with absent
- # BranchRevision{.sequence} when branches get overridden/rebased.
+ # BranchRevision{.sequence} when branches get overridden/rebased.
mp = self._createProposalWithPreviewDiff(content='')
preview = mp.preview_diff
# Both, source and target, branch revisions are absent,
@@ -386,6 +455,12 @@
target_rev.sequence = 1
self.assertEqual('r10 into r1', preview.title)
+ def test_title_git(self):
+ # Git commit IDs are shortened to seven characters.
+ mp = self._createGitProposalWithPreviewDiff(content='')
+ preview = mp.preview_diff
+ self.assertEqual('aaaaaaa into bbbbbbb', preview.title)
+
def test_empty_diff(self):
# Once the source is merged into the target, the diff between the
# branches will be empty.
@@ -495,6 +570,24 @@
finally:
logger.removeHandler(handler)
+ def test_fromBranchMergeProposalForGit(self):
+ # Correctly generates a PreviewDiff from a Git-based
+ # BranchMergeProposal.
+ bmp, source_rev_id, target_rev_id, patch = self.createExampleGitMerge()
+ preview = PreviewDiff.fromBranchMergeProposal(bmp)
+ self.assertEqual(source_rev_id, preview.source_revision_id)
+ self.assertEqual(target_rev_id, preview.target_revision_id)
+ transaction.commit()
+ self.assertEqual(patch, preview.text)
+ self.assertEqual({'foo': (5, 0)}, preview.diffstat)
+
+ def test_fromBranchMergeProposalForGit_sets_conflicts(self):
+ # Conflicts are set on the PreviewDiff.
+ bmp, source_rev_id, target_rev_id, _ = self.createExampleGitMerge()
+ preview = PreviewDiff.fromBranchMergeProposal(bmp)
+ self.assertEqual('Conflict in foo\n', preview.conflicts)
+ self.assertTrue(preview.has_conflicts)
+
def test_getFileByName(self):
diff = self._createProposalWithPreviewDiff().preview_diff
self.assertEqual(diff.diff_text, diff.getFileByName('preview.diff'))
Follow ups
