launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18787
Re: [Merge] lp:~cjwatson/launchpad/git-detect-merges into lp:launchpad
Review: Approve code
Diff comments:
> === modified file 'lib/lp/code/event/git.py'
> --- lib/lp/code/event/git.py 2015-06-06 08:49:54 +0000
> +++ lib/lp/code/event/git.py 2015-06-12 17:59:30 +0000
> @@ -19,6 +19,7 @@
>
> implements(IGitRefsUpdatedEvent)
>
> - def __init__(self, repository, paths):
> + def __init__(self, repository, paths, logger):
> super(GitRefsUpdatedEvent, self).__init__(repository)
> self.paths = paths
> + self.logger = logger
>
> === modified file 'lib/lp/code/interfaces/gitrepository.py'
> --- lib/lp/code/interfaces/gitrepository.py 2015-06-12 17:59:30 +0000
> +++ lib/lp/code/interfaces/gitrepository.py 2015-06-12 17:59:30 +0000
> @@ -249,12 +249,13 @@
> :return: An `IGitRef`, or None.
> """
>
> - def createOrUpdateRefs(refs_info, get_objects=False):
> + def createOrUpdateRefs(refs_info, get_objects=False, logger=None):
> """Create or update a set of references in this repository.
>
> :param refs_info: A dict mapping ref paths to
> {"sha1": sha1, "type": `GitObjectType`}.
> :param get_objects: Return the created/updated references.
> + :param logger: An optional logger.
>
> :return: A list of the created/updated references if get_objects,
> otherwise None.
> @@ -287,13 +288,14 @@
> :param logger: An optional logger.
> """
>
> - def synchroniseRefs(refs_to_upsert, refs_to_remove):
> + def synchroniseRefs(refs_to_upsert, refs_to_remove, logger=None):
> """Synchronise references with those from the hosting service.
>
> :param refs_to_upsert: A dictionary mapping ref paths to
> dictionaries of their fields; these refs will be created or
> updated as appropriate.
> :param refs_to_remove: A set of ref paths to remove.
> + :param logger: An optional logger.
> """
>
> def setOwnerDefault(value):
> @@ -482,6 +484,16 @@
> "Whether there are recent changes in this repository that have not "
> "yet been scanned.")
>
> + def updateMergeCommitIDs(paths):
> + """Update commit SHA1s of merge proposals for this repository.
> +
> + The *_git_commit_sha1 columns of merge proposals are stored
> + explicitly in order that merge proposals are still meaningful after
> + associated refs have been deleted. However, active merge proposals
> + where the refs in question still exist should have these columns
> + kept up to date.
> + """
> +
> def scheduleDiffUpdates(paths):
> """Create UpdatePreviewDiffJobs for landing targets.
>
> @@ -490,6 +502,15 @@
> diffs updated.
> """
>
> + def detectMerges(paths, logger=None):
> + """Detect merges of landing candidates.
> +
> + :param paths: A list of reference paths. Any merge proposals whose
> + target is this repository and one of these paths will be
> + checked.
> + :param logger: An optional logger.
> + """
> +
>
> class IGitRepositoryModerateAttributes(Interface):
> """IGitRepository attributes that can be edited by more than one community.
>
> === modified file 'lib/lp/code/model/githosting.py'
> --- lib/lp/code/model/githosting.py 2015-06-12 17:59:30 +0000
> +++ lib/lp/code/model/githosting.py 2015-06-12 17:59:30 +0000
> @@ -43,70 +43,59 @@
> # over, but is there some more robust way to do this?
> return 5.0
>
> + def _request(self, method, path, json_data=None, **kwargs):
> + session = self._makeSession()
> + if json_data is not None:
> + # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
> + # should just pass json through directly and drop the explicit
> + # Content-Type header.
> + kwargs.setdefault("headers", {})["Content-Type"] = (
> + "application/json")
> + kwargs["data"] = json.dumps(json_data)
> + response = getattr(session, method)(
> + urljoin(self.endpoint, path), timeout=self.timeout, **kwargs)
> + if response.status_code != 200:
> + raise Exception(response.text)
Can you make this exception class slightly more specific?
> + return response.json()
> +
> + def _get(self, path, **kwargs):
> + return self._request("get", path, **kwargs)
> +
> + def _post(self, path, **kwargs):
> + return self._request("post", path, **kwargs)
> +
> + def _delete(self, path, **kwargs):
> + return self._request("delete", path, **kwargs)
> +
> def create(self, path, clone_from=None):
> try:
> - # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
> - # should just use post(json=) and drop the explicit Content-Type
> - # header.
> if clone_from:
> request = {"repo_path": path, "clone_from": clone_from}
> else:
> request = {"repo_path": path}
> - response = self._makeSession().post(
> - urljoin(self.endpoint, "/repo"),
> - headers={"Content-Type": "application/json"},
> - data=json.dumps(request),
> - timeout=self.timeout)
> + self._post("/repo", json_data=request)
> except Exception as e:
> raise GitRepositoryCreationFault(
> "Failed to create Git repository: %s" % unicode(e))
> - if response.status_code != 200:
> - raise GitRepositoryCreationFault(
> - "Failed to create Git repository: %s" % response.text)
>
> def getRefs(self, path):
> try:
> - response = self._makeSession().get(
> - urljoin(self.endpoint, "/repo/%s/refs" % path),
> - timeout=self.timeout)
> + return self._get("/repo/%s/refs" % path)
> except Exception as e:
> raise GitRepositoryScanFault(
> "Failed to get refs from Git repository: %s" % unicode(e))
> - if response.status_code != 200:
> - raise GitRepositoryScanFault(
> - "Failed to get refs from Git repository: %s" % response.text)
> - try:
> - return response.json()
> - except ValueError as e:
> - raise GitRepositoryScanFault(
> - "Failed to decode ref-scan response: %s" % unicode(e))
>
> def getCommits(self, path, commit_oids, logger=None):
> commit_oids = list(commit_oids)
> try:
> - # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
> - # should just use post(json=) and drop the explicit Content-Type
> - # header.
> if logger is not None:
> logger.info("Requesting commit details for %s" % commit_oids)
> - response = self._makeSession().post(
> - urljoin(self.endpoint, "/repo/%s/commits" % path),
> - headers={"Content-Type": "application/json"},
> - data=json.dumps({"commits": commit_oids}),
> - timeout=self.timeout)
> + return self._post(
> + "/repo/%s/commits" % path, json_data={"commits": commit_oids})
> except Exception as e:
> raise GitRepositoryScanFault(
> "Failed to get commit details from Git repository: %s" %
> unicode(e))
> - if response.status_code != 200:
> - raise GitRepositoryScanFault(
> - "Failed to get commit details from Git repository: %s" %
> - response.text)
> - try:
> - return response.json()
> - 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.
> @@ -121,35 +110,40 @@
> logger.info(
> "Requesting merge diff for %s from %s to %s" % (
> path, base, head))
> - response = self._makeSession().get(
> - urljoin(
> - self.endpoint,
> - "/repo/%s/compare-merge/%s:%s" % (path, base, head)),
> - timeout=self.timeout)
> + return self._get(
> + "/repo/%s/compare-merge/%s:%s" % (path, base, head))
> 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" %
> - response.text)
> +
> + def detectMerges(self, path, target, sources, logger=None):
> + """Detect merges of any of 'sources' into 'target'.
I'd document that sources and target both refer to commit SHA-1s.
> +
> + :return: A dict mapping merged commit OIDs from 'sources' to the
> + first commit OID in the left-hand (first parent only) history of
> + 'target' that is a descendant of the corresponding source
> + commit. Unmerged commits are omitted.
> + """
> + sources = list(sources)
> try:
> - return response.json()
> - except ValueError as e:
> + if logger is not None:
> + logger.info(
> + "Detecting merges for %s from %s to %s" % (
> + path, sources, target))
> + return self._post(
> + "/repo/%s/detect-merges/%s" % (path, target),
> + json_data={"sources": sources})
> + except Exception as e:
> raise GitRepositoryScanFault(
> - "Failed to decode merge-diff response: %s" % unicode(e))
> + "Failed to detect merges in Git repository: %s" % unicode(e))
>
> def delete(self, path, logger=None):
> """Delete a repository."""
> try:
> if logger is not None:
> logger.info("Deleting repository %s" % path)
> - response = self._makeSession().delete(
> - urljoin(self.endpoint, "/repo/%s" % path))
> + return self._delete("/repo/%s" % path)
> except Exception as e:
> raise GitRepositoryDeletionFault(
> "Failed to delete Git repository: %s" % unicode(e))
> - if response.status_code != 200:
> - raise GitRepositoryDeletionFault(
> - "Failed to delete Git repository: %s" % response.text)
>
> === modified file 'lib/lp/code/model/gitjob.py'
> --- lib/lp/code/model/gitjob.py 2015-06-12 17:59:30 +0000
> +++ lib/lp/code/model/gitjob.py 2015-06-12 17:59:30 +0000
> @@ -210,7 +210,8 @@
> self.repository.planRefChanges(hosting_path, logger=log))
> self.repository.fetchRefCommits(
> hosting_path, refs_to_upsert, logger=log)
> - self.repository.synchroniseRefs(refs_to_upsert, refs_to_remove)
> + self.repository.synchroniseRefs(
> + refs_to_upsert, refs_to_remove, logger=log)
> except LostObjectError:
> log.info(
> "Skipping repository %s because it has been deleted." %
>
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py 2015-06-12 17:59:30 +0000
> +++ lib/lp/code/model/gitrepository.py 2015-06-12 17:59:30 +0000
> @@ -10,7 +10,12 @@
>
> from datetime import datetime
> import email
> -from itertools import chain
> +from functools import partial
> +from itertools import (
> + chain,
> + groupby,
> + )
> +from operator import attrgetter
>
> from bzrlib import urlutils
> import pytz
> @@ -71,6 +76,7 @@
> from lp.code.event.git import GitRefsUpdatedEvent
> from lp.code.interfaces.branchmergeproposal import (
> BRANCH_MERGE_PROPOSAL_FINAL_STATES,
> + notify_modified,
> )
> from lp.code.interfaces.gitcollection import (
> IAllGitRepositories,
> @@ -440,7 +446,7 @@
> sha1 = sha1.decode("US-ASCII")
> return {"sha1": sha1, "type": object_type_map[obj["type"]]}
>
> - def createOrUpdateRefs(self, refs_info, get_objects=False):
> + def createOrUpdateRefs(self, refs_info, get_objects=False, logger=None):
> """See `IGitRepository`."""
> def dbify_values(values):
> return [
> @@ -509,7 +515,8 @@
>
> self.date_last_modified = UTC_NOW
> if updated:
> - notify(GitRefsUpdatedEvent(self, [value[1] for value in updated]))
> + notify(GitRefsUpdatedEvent(
> + self, [value[1] for value in updated], logger))
> if get_objects:
> return bulk.load(GitRef, updated + created)
>
> @@ -597,10 +604,10 @@
> if committer is not None:
> info["committer"] = committer.id
>
> - def synchroniseRefs(self, refs_to_upsert, refs_to_remove):
> + def synchroniseRefs(self, refs_to_upsert, refs_to_remove, logger=None):
> """See `IGitRepository`."""
> if refs_to_upsert:
> - self.createOrUpdateRefs(refs_to_upsert)
> + self.createOrUpdateRefs(refs_to_upsert, logger=logger)
> if refs_to_remove:
> self.removeRefs(refs_to_remove)
>
> @@ -812,6 +819,15 @@
> Not(BranchMergeProposal.queue_status.is_in(
> BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
>
> + def getActiveLandingCandidates(self, paths):
> + """Merge proposals not in final states where these refs are target."""
> + return Store.of(self).find(
> + BranchMergeProposal,
> + BranchMergeProposal.target_git_repository == self,
> + BranchMergeProposal.target_git_path.is_in(paths),
> + Not(BranchMergeProposal.queue_status.is_in(
> + BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
> +
> @property
> def dependent_landings(self):
> """See `IGitRepository`."""
> @@ -844,6 +860,52 @@
> Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]))
> return not jobs.is_empty()
>
> + def updateMergeCommitIDs(self, paths):
> + """See `IGitRepository`."""
> + store = Store.of(self)
> + refs = {
> + path: commit_sha1 for path, commit_sha1 in store.find(
> + (GitRef.path, GitRef.commit_sha1),
> + GitRef.repository_id == self.id,
> + GitRef.path.is_in(paths))}
> + updated = set()
> + for kind in ("source", "target", "prerequisite"):
> + repository_name = "%s_git_repositoryID" % kind
> + path_name = "%s_git_path" % kind
> + commit_sha1_name = "%s_git_commit_sha1" % kind
> + old_column = partial(getattr, BranchMergeProposal)
> + db_kind = "dependent" if kind == "prerequisite" else kind
> + column_types = [
> + ("%s_git_path" % db_kind, "text"),
> + ("%s_git_commit_sha1" % db_kind, "character(40)"),
> + ]
> + db_values = [(
> + bulk.dbify_value(old_column(path_name), path),
> + bulk.dbify_value(old_column(commit_sha1_name), commit_sha1)
> + ) for path, commit_sha1 in refs.items()]
> + new_proposals_expr = Values(
> + "new_proposals", column_types, db_values)
> + new_proposals = ClassAlias(BranchMergeProposal, "new_proposals")
> + new_column = partial(getattr, new_proposals)
> + updated_columns = {
> + old_column(commit_sha1_name): new_column(commit_sha1_name)}
> + update_filter = And(
> + old_column(repository_name) == self.id,
> + old_column(path_name) == new_column(path_name),
> + Not(BranchMergeProposal.queue_status.is_in(
> + BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
> + result = store.execute(Returning(BulkUpdate(
> + updated_columns, table=BranchMergeProposal,
> + values=new_proposals_expr, where=update_filter,
> + primary_columns=BranchMergeProposal.id)))
> + updated.update(item[0] for item in result)
> + if updated:
> + # Some existing BranchMergeProposal objects may no longer be
> + # valid. Without knowing which ones we already have, it's
> + # safest to just invalidate everything.
> + store.invalidate()
Storm really should provide a public API for this, but it doesn't. We're not calling this frequently, so invalidating everything is fine.
If you want a good laugh, search for _iter_alive in ResultSet.set. ResultSet.cached is also some of the best comedy I've seen in years.
> + return updated
> +
> def scheduleDiffUpdates(self, paths):
> """See `IGitRepository`."""
> from lp.code.model.branchmergeproposaljob import UpdatePreviewDiffJob
> @@ -852,6 +914,33 @@
> jobs.append(UpdatePreviewDiffJob.create(merge_proposal))
> return jobs
>
> + def _markProposalMerged(self, proposal, merged_revision_id, logger=None):
> + if logger is not None:
> + logger.info(
> + "Merge detected: %s => %s",
> + proposal.source_git_ref.identity,
> + proposal.target_git_ref.identity)
> + notify_modified(
> + proposal, proposal.markAsMerged,
> + merged_revision_id=merged_revision_id)
> +
> + def detectMerges(self, paths, logger=None):
> + """See `IGitRepository`."""
> + hosting_client = getUtility(IGitHostingClient)
> + all_proposals = self.getActiveLandingCandidates(paths).order_by(
> + BranchMergeProposal.target_git_path)
> + for _, group in groupby(all_proposals, attrgetter("target_git_path")):
> + proposals = list(group)
> + merges = hosting_client.detectMerges(
> + self.getInternalPath(), proposals[0].target_git_commit_sha1,
> + set(proposal.source_git_commit_sha1 for proposal in proposals))
> + for proposal in proposals:
> + merged_revision_id = merges.get(
> + proposal.source_git_commit_sha1)
> + if merged_revision_id is not None:
> + self._markProposalMerged(
> + proposal, merged_revision_id, logger=logger)
> +
> def canBeDeleted(self):
> """See `IGitRepository`."""
> # Can't delete if the repository is associated with anything.
>
> === modified file 'lib/lp/code/model/tests/test_gitrepository.py'
> --- lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 17:59:30 +0000
> +++ lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 17:59:30 +0000
> @@ -37,6 +37,7 @@
> )
> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
> from lp.code.enums import (
> + BranchMergeProposalStatus,
> BranchSubscriptionDiffSize,
> BranchSubscriptionNotificationLevel,
> CodeReviewNotificationLevel,
> @@ -101,6 +102,7 @@
> from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
> from lp.services.config import config
> from lp.services.database.constants import UTC_NOW
> +from lp.services.database.interfaces import IStore
> from lp.services.job.interfaces.job import JobStatus
> from lp.services.job.model.job import Job
> from lp.services.job.runner import JobRunner
> @@ -125,6 +127,7 @@
> LaunchpadFunctionalLayer,
> LaunchpadZopelessLayer,
> )
> +from lp.testing.mail_helpers import pop_notifications
> from lp.testing.pages import webservice_for_person
>
>
> @@ -1016,6 +1019,9 @@
> jobs = self._getWaitingUpdatePreviewDiffJobs(repository)
> self.assertEqual(
> [bmp, bmp], [job.branch_merge_proposal for job in jobs])
> + self.assertEqual(
> + u"0000000000000000000000000000000000000000",
> + bmp.source_git_commit_sha1)
>
> def test_getRefByPath_without_leading_refs_heads(self):
> [ref] = self.factory.makeGitRefs(paths=[u"refs/heads/master"])
> @@ -1618,6 +1624,66 @@
> target=commercial_project, user=owner)
>
>
> +class TestGitRepositoryUpdateMergeCommitIDs(TestCaseWithFactory):
> +
> + layer = DatabaseFunctionalLayer
> +
> + def test_updates_proposals(self):
> + # `updateMergeCommitIDs` updates proposals for specified refs.
> + repository = self.factory.makeGitRepository()
> + paths = [u"refs/heads/1", u"refs/heads/2", u"refs/heads/3"]
> + [ref1, ref2, ref3] = self.factory.makeGitRefs(
> + repository=repository, paths=paths)
> + bmp1 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=ref1, source_ref=ref2)
> + bmp2 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=ref1, source_ref=ref3, prerequisite_ref=ref2)
> + removeSecurityProxy(ref1).commit_sha1 = u"0" * 40
> + removeSecurityProxy(ref2).commit_sha1 = u"1" * 40
> + removeSecurityProxy(ref3).commit_sha1 = u"2" * 40
> + self.assertNotEqual(ref1, bmp1.target_git_ref)
> + self.assertNotEqual(ref2, bmp1.source_git_ref)
> + self.assertNotEqual(ref1, bmp2.target_git_ref)
> + self.assertNotEqual(ref3, bmp2.source_git_ref)
> + self.assertNotEqual(ref2, bmp2.prerequisite_git_ref)
> + self.assertContentEqual(
> + [bmp1.id, bmp2.id], repository.updateMergeCommitIDs(paths))
> + self.assertEqual(ref1, bmp1.target_git_ref)
> + self.assertEqual(ref2, bmp1.source_git_ref)
> + self.assertEqual(ref1, bmp2.target_git_ref)
> + self.assertIsNone(bmp1.prerequisite_git_ref)
> + self.assertEqual(ref3, bmp2.source_git_ref)
> + self.assertEqual(ref2, bmp2.prerequisite_git_ref)
> +
> + def test_skips_unspecified_refs(self):
> + # `updateMergeCommitIDs` skips unspecified refs.
> + repository1 = self.factory.makeGitRepository()
> + paths = [u"refs/heads/1", u"refs/heads/2"]
> + [ref1_1, ref1_2] = self.factory.makeGitRefs(
> + repository=repository1, paths=paths)
> + bmp1 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=ref1_1, source_ref=ref1_2)
> + repository2 = self.factory.makeGitRepository(target=repository1.target)
> + [ref2_1, ref2_2] = self.factory.makeGitRefs(
> + repository=repository2, paths=paths)
> + bmp2 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=ref2_1, source_ref=ref2_2)
> + removeSecurityProxy(ref1_1).commit_sha1 = u"0" * 40
> + removeSecurityProxy(ref1_2).commit_sha1 = u"1" * 40
> + removeSecurityProxy(ref2_1).commit_sha1 = u"2" * 40
> + removeSecurityProxy(ref2_2).commit_sha1 = u"3" * 40
> + self.assertNotEqual(ref1_1, bmp1.target_git_ref)
> + self.assertNotEqual(ref1_2, bmp1.source_git_ref)
> + self.assertNotEqual(ref2_1, bmp2.target_git_ref)
> + self.assertNotEqual(ref2_2, bmp2.source_git_ref)
> + self.assertContentEqual(
> + [bmp1.id], repository1.updateMergeCommitIDs([paths[0]]))
> + self.assertEqual(ref1_1, bmp1.target_git_ref)
> + self.assertNotEqual(ref1_2, bmp1.source_git_ref)
> + self.assertNotEqual(ref2_1, bmp2.target_git_ref)
> + self.assertNotEqual(ref2_2, bmp2.source_git_ref)
> +
> +
> class TestGitRepositoryScheduleDiffUpdates(TestCaseWithFactory):
>
> layer = DatabaseFunctionalLayer
> @@ -1649,6 +1715,85 @@
> self.assertEqual(0, len(jobs))
>
>
> +class TestGitRepositoryDetectMerges(TestCaseWithFactory):
> +
> + layer = LaunchpadZopelessLayer
> +
> + def test_markProposalMerged(self):
> + # A merge proposal that is merged is marked as such.
> + proposal = self.factory.makeBranchMergeProposalForGit()
> + self.assertNotEqual(
> + BranchMergeProposalStatus.MERGED, proposal.queue_status)
> + proposal.target_git_repository._markProposalMerged(
> + proposal, proposal.target_git_commit_sha1)
> + self.assertEqual(
> + BranchMergeProposalStatus.MERGED, proposal.queue_status)
> + job = IStore(proposal).find(
> + BranchMergeProposalJob,
> + BranchMergeProposalJob.branch_merge_proposal == proposal,
> + BranchMergeProposalJob.job_type ==
> + BranchMergeProposalJobType.MERGE_PROPOSAL_UPDATED).one()
> + derived_job = job.makeDerived()
> + derived_job.run()
> + notifications = pop_notifications()
> + self.assertIn(
> + "Work in progress => Merged",
> + notifications[0].get_payload(decode=True))
> + self.assertEqual(
> + config.canonical.noreply_from_address, notifications[0]["From"])
> + recipients = set(msg["x-envelope-to"] for msg in notifications)
> + expected = set(
> + [proposal.source_git_repository.registrant.preferredemail.email,
> + proposal.target_git_repository.registrant.preferredemail.email])
> + self.assertEqual(expected, recipients)
> +
> + def test_update_detects_merges(self):
> + # Pushing changes to a branch causes a check whether any active
> + # merge proposals with that branch as the target have been merged.
> + repository = self.factory.makeGitRepository()
> + [target_1, target_2, source_1, source_2] = self.factory.makeGitRefs(
> + repository, paths=[
> + u"refs/heads/target-1",
> + u"refs/heads/target-2",
> + u"refs/heads/source-1",
> + u"refs/heads/source-2",
> + ])
> + bmp1 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=target_1, source_ref=source_1)
> + bmp2 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=target_1, source_ref=source_2)
> + bmp3 = self.factory.makeBranchMergeProposalForGit(
> + target_ref=target_2, source_ref=source_1)
> + hosting_client = FakeMethod()
> + hosting_client.detectMerges = FakeMethod(
> + result={source_1.commit_sha1: u"0" * 40})
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> + repository.createOrUpdateRefs({
> + u"refs/heads/target-1": {
> + u"sha1": u"0" * 40,
> + u"type": GitObjectType.COMMIT,
> + },
> + u"refs/heads/target-2": {
> + u"sha1": u"1" * 40,
> + u"type": GitObjectType.COMMIT,
> + }
> + })
> + expected_args = [
> + (repository.getInternalPath(), target_1.commit_sha1,
> + set([source_1.commit_sha1, source_2.commit_sha1])),
> + (repository.getInternalPath(), target_2.commit_sha1,
> + set([source_1.commit_sha1])),
> + ]
> + self.assertContentEqual(
> + expected_args, hosting_client.detectMerges.extract_args())
> + self.assertEqual(BranchMergeProposalStatus.MERGED, bmp1.queue_status)
> + self.assertEqual(u"0" * 40, bmp1.merged_revision_id)
> + self.assertEqual(
> + BranchMergeProposalStatus.WORK_IN_PROGRESS, bmp2.queue_status)
> + self.assertEqual(BranchMergeProposalStatus.MERGED, bmp3.queue_status)
> + self.assertEqual(u"0" * 40, bmp3.merged_revision_id)
> +
> +
> class TestGitRepositorySet(TestCaseWithFactory):
>
> layer = DatabaseFunctionalLayer
>
> === modified file 'lib/lp/code/subscribers/git.py'
> --- lib/lp/code/subscribers/git.py 2015-06-06 08:49:54 +0000
> +++ lib/lp/code/subscribers/git.py 2015-06-12 17:59:30 +0000
> @@ -8,4 +8,6 @@
>
> def refs_updated(repository, event):
> """Some references in a Git repository have been updated."""
> + repository.updateMergeCommitIDs(event.paths)
> repository.scheduleDiffUpdates(event.paths)
> + repository.detectMerges(event.paths, logger=event.logger)
>
> === modified file 'lib/lp/testing/factory.py'
> --- lib/lp/testing/factory.py 2015-06-11 08:06:28 +0000
> +++ lib/lp/testing/factory.py 2015-06-12 17:59:30 +0000
> @@ -1766,8 +1766,11 @@
> u"type": GitObjectType.COMMIT,
> }
> for path in paths}
> - return removeSecurityProxy(repository).createOrUpdateRefs(
> - refs_info, get_objects=True)
> + refs_by_path = {
> + ref.path: ref
> + for ref in removeSecurityProxy(repository).createOrUpdateRefs(
> + refs_info, get_objects=True)}
> + return [refs_by_path[path] for path in paths]
>
> def makeBug(self, target=None, owner=None, bug_watch_url=None,
> information_type=None, date_closed=None, title=None,
>
--
https://code.launchpad.net/~cjwatson/launchpad/git-detect-merges/+merge/261877
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References