launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18782
Re: [Merge] lp:~cjwatson/launchpad/git-hosting-utility into lp:launchpad
Review: Approve code
Diff comments:
> === modified file 'lib/lp/code/configure.zcml'
> --- lib/lp/code/configure.zcml 2015-06-06 08:49:54 +0000
> +++ lib/lp/code/configure.zcml 2015-06-12 21:46:06 +0000
> @@ -938,6 +938,13 @@
> <adapter factory="lp.code.model.gitlookup.DistributionGitTraversable" />
> <adapter factory="lp.code.model.gitlookup.DistributionSourcePackageGitTraversable" />
>
> + <!-- Git hosting -->
> + <securedutility
> + class="lp.code.model.githosting.GitHostingClient"
> + provides="lp.code.interfaces.githosting.IGitHostingClient">
> + <allow interface="lp.code.interfaces.githosting.IGitHostingClient" />
> + </securedutility>
> +
> <!-- Git-related jobs -->
> <class class="lp.code.model.gitjob.GitJob">
> <allow interface="lp.code.interfaces.gitjob.IGitJob" />
>
> === added file 'lib/lp/code/interfaces/githosting.py'
> --- lib/lp/code/interfaces/githosting.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/code/interfaces/githosting.py 2015-06-12 21:46:06 +0000
> @@ -0,0 +1,63 @@
> +# Copyright 2015 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Interface for communication with the Git hosting service."""
> +
> +__metaclass__ = type
> +__all__ = [
> + 'IGitHostingClient',
> + ]
> +
> +from zope.interface import Interface
> +
> +
> +class IGitHostingClient(Interface):
> + """Interface for the internal API provided by the Git hosting service."""
> +
> + def create(path, clone_from=None):
> + """Create a Git repository.
> +
> + :param path: Physical path of the new repository on the hosting
> + service.
> + :param clone_from: If not None, clone the new repository from this
> + other physical path.
> + """
> +
> + def getRefs(path):
> + """Get all refs in this repository.
> +
> + :param path: Physical path of the repository on the hosting service.
> + :return: A dict mapping ref paths to dicts representing the objects
> + they point to.
> + """
> +
> + def getCommits(path, commit_oids, logger=None):
> + """Get details of a list of commits.
> +
> + :param path: Physical path of the repository on the hosting service.
> + :param commit_oids: A list of commit OIDs.
> + :param logger: An optional logger.
> + :return: A list of dicts each of which represents one of the
> + requested commits. Non-existent commits will be omitted.
> + """
> +
> + def getMergeDiff(path, base, head, logger=None):
> + """Get the merge preview diff between two commits.
> +
> + :param path: Physical path of the repository on the hosting service.
> + :param base: The OID of the base commit.
> + :param head: The OID of the commit that we want to merge into
> + 'base'.
> + :param logger: An optional logger.
> + :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.
> + """
> +
> + def delete(path, logger=None):
> + """Delete a repository.
> +
> + :param path: Physical path of the repository on the hosting service.
> + :param logger: An optional logger.
> + """
>
> === modified file 'lib/lp/code/interfaces/gitrepository.py'
> --- lib/lp/code/interfaces/gitrepository.py 2015-06-12 08:20:17 +0000
> +++ lib/lp/code/interfaces/gitrepository.py 2015-06-12 21:46:06 +0000
> @@ -266,10 +266,9 @@
> :params paths: An iterable of paths.
> """
>
> - def planRefChanges(hosting_client, hosting_path, logger=None):
> + def planRefChanges(hosting_path, logger=None):
> """Plan ref changes based on information from the hosting service.
>
> - :param hosting_client: A `GitHostingClient`.
> :param hosting_path: A path on the hosting service.
> :param logger: An optional logger.
>
> @@ -278,10 +277,9 @@
> paths to remove.
> """
>
> - def fetchRefCommits(hosting_client, hosting_path, refs, logger=None):
> + def fetchRefCommits(hosting_path, refs, logger=None):
> """Fetch commit information from the hosting service for a set of refs.
>
> - :param hosting_client: A `GitHostingClient`.
> :param hosting_path: A path on the hosting service.
> :param refs: A dict mapping ref paths to dictionaries of their
> fields; the field dictionaries will be updated with any detailed
>
> === modified file 'lib/lp/code/model/diff.py'
> --- lib/lp/code/model/diff.py 2015-04-30 22:06:11 +0000
> +++ lib/lp/code/model/diff.py 2015-06-12 21:46:06 +0000
> @@ -42,12 +42,12 @@
> 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,
> IPreviewDiff,
> )
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.codehosting.bzrutils import read_locked
> from lp.services.config import config
> from lp.services.database.constants import UTC_NOW
> @@ -417,10 +417,6 @@
> 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`.
> @@ -449,7 +445,6 @@
> preview.conflicts = u''.join(
> unicode(conflict) + '\n' for conflict in conflicts)
> else:
> - hosting_client = cls.getGitHostingClient()
> source_repository = bmp.source_git_repository
> target_repository = bmp.target_git_repository
> if source_repository == target_repository:
> @@ -460,7 +455,7 @@
> source_repository.getInternalPath())
> # XXX cjwatson 2015-04-30: Add prerequisite handling once turnip
> # supports it.
> - response = hosting_client.getMergeDiff(
> + response = getUtility(IGitHostingClient).getMergeDiff(
> path, bmp.target_git_ref.commit_sha1,
> bmp.source_git_ref.commit_sha1)
> source_revision_id = bmp.source_git_ref.commit_sha1
>
> === renamed file 'lib/lp/code/githosting.py' => 'lib/lp/code/model/githosting.py'
> --- lib/lp/code/githosting.py 2015-05-19 11:29:24 +0000
> +++ lib/lp/code/model/githosting.py 2015-06-12 21:46:06 +0000
> @@ -12,19 +12,24 @@
> from urlparse import urljoin
>
> import requests
> +from zope.interface import implements
>
> from lp.code.errors import (
> GitRepositoryCreationFault,
> GitRepositoryDeletionFault,
> GitRepositoryScanFault,
> )
> +from lp.code.interfaces.githosting import IGitHostingClient
> +from lp.services.config import config
>
>
> class GitHostingClient:
> """A client for the internal API provided by the Git hosting system."""
>
> - def __init__(self, endpoint):
> - self.endpoint = endpoint
> + implements(IGitHostingClient)
> +
> + def __init__(self):
> + self.endpoint = config.codehosting.internal_git_api_endpoint
>
> def _makeSession(self):
> session = requests.Session()
> @@ -39,6 +44,7 @@
> return 5.0
>
> def create(self, path, clone_from=None):
> + """See `IGitHostingClient`."""
> 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
> @@ -60,6 +66,7 @@
> "Failed to create Git repository: %s" % response.text)
>
> def getRefs(self, path):
> + """See `IGitHostingClient`."""
> try:
> response = self._makeSession().get(
> urljoin(self.endpoint, "/repo/%s/refs" % path),
> @@ -77,6 +84,7 @@
> "Failed to decode ref-scan response: %s" % unicode(e))
>
> def getCommits(self, path, commit_oids, logger=None):
> + """See `IGitHostingClient`."""
> commit_oids = list(commit_oids)
> try:
> # XXX cjwatson 2015-03-01: Once we're on requests >= 2.4.2, we
> @@ -104,13 +112,7 @@
> "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.
> - """
> + """See `IGitHostingClient`."""
> try:
> if logger is not None:
> logger.info(
> @@ -136,7 +138,7 @@
> "Failed to decode merge-diff response: %s" % unicode(e))
>
> def delete(self, path, logger=None):
> - """Delete a repository."""
> + """See `IGitHostingClient`."""
> try:
> if logger is not None:
> logger.info("Deleting repository %s" % path)
>
> === modified file 'lib/lp/code/model/gitjob.py'
> --- lib/lp/code/model/gitjob.py 2015-05-26 13:55:10 +0000
> +++ lib/lp/code/model/gitjob.py 2015-06-12 21:46:06 +0000
> @@ -23,13 +23,14 @@
> SQL,
> Store,
> )
> +from zope.component import getUtility
> from zope.interface import (
> classProvides,
> implements,
> )
>
> from lp.app.errors import NotFoundError
> -from lp.code.githosting import GitHostingClient
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.interfaces.gitjob import (
> IGitJob,
> IGitRefScanJob,
> @@ -198,11 +199,6 @@
> job.celeryRunOnCommit()
> return job
>
> - def __init__(self, git_job):
> - super(GitRefScanJob, self).__init__(git_job)
> - self._hosting_client = GitHostingClient(
> - config.codehosting.internal_git_api_endpoint)
> -
> def run(self):
> """See `IGitRefScanJob`."""
> try:
> @@ -211,11 +207,9 @@
> Store.of(self.repository)):
> hosting_path = self.repository.getInternalPath()
> refs_to_upsert, refs_to_remove = (
> - self.repository.planRefChanges(
> - self._hosting_client, hosting_path, logger=log))
> + self.repository.planRefChanges(hosting_path, logger=log))
> self.repository.fetchRefCommits(
> - self._hosting_client, hosting_path, refs_to_upsert,
> - logger=log)
> + hosting_path, refs_to_upsert, logger=log)
> self.repository.synchroniseRefs(refs_to_upsert, refs_to_remove)
> except LostObjectError:
> log.info(
> @@ -250,14 +244,9 @@
> job.celeryRunOnCommit()
> return job
>
> - def __init__(self, git_job):
> - super(ReclaimGitRepositorySpaceJob, self).__init__(git_job)
> - self._hosting_client = GitHostingClient(
> - config.codehosting.internal_git_api_endpoint)
> -
> @property
> def repository_path(self):
> return self.metadata["repository_path"]
>
> def run(self):
> - self._hosting_client.delete(self.repository_path, logger=log)
> + getUtility(IGitHostingClient).delete(self.repository_path, logger=log)
>
> === modified file 'lib/lp/code/model/gitrepository.py'
> --- lib/lp/code/model/gitrepository.py 2015-06-12 08:20:17 +0000
> +++ lib/lp/code/model/gitrepository.py 2015-06-12 21:46:06 +0000
> @@ -76,6 +76,7 @@
> IAllGitRepositories,
> IGitCollection,
> )
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.interfaces.gitlookup import IGitLookup
> from lp.code.interfaces.gitnamespace import (
> get_git_namespace,
> @@ -519,8 +520,9 @@
> GitRef.repository == self, GitRef.path.is_in(paths)).remove()
> self.date_last_modified = UTC_NOW
>
> - def planRefChanges(self, hosting_client, hosting_path, logger=None):
> + def planRefChanges(self, hosting_path, logger=None):
> """See `IGitRepository`."""
> + hosting_client = getUtility(IGitHostingClient)
> new_refs = {}
> for path, info in hosting_client.getRefs(hosting_path).items():
> try:
> @@ -550,12 +552,12 @@
> return refs_to_upsert, refs_to_remove
>
> @staticmethod
> - def fetchRefCommits(hosting_client, hosting_path, refs, logger=None):
> + def fetchRefCommits(hosting_path, refs, logger=None):
> """See `IGitRepository`."""
> oids = sorted(set(info["sha1"] for info in refs.values()))
> commits = {
> commit.get("sha1"): commit
> - for commit in hosting_client.getCommits(
> + for commit in getUtility(IGitHostingClient).getCommits(
> hosting_path, oids, logger=logger)}
> authors_to_acquire = []
> committers_to_acquire = []
>
> === modified file 'lib/lp/code/model/tests/test_diff.py'
> --- lib/lp/code/model/tests/test_diff.py 2015-05-14 13:57:51 +0000
> +++ lib/lp/code/model/tests/test_diff.py 2015-06-12 21:46:06 +0000
> @@ -16,8 +16,8 @@
> parse_patches,
> RemoveLine,
> )
> -from fixtures import MonkeyPatch
> import transaction
> +from zope.interface import implements
> from zope.security.proxy import removeSecurityProxy
>
> from lp.app.errors import NotFoundError
> @@ -26,6 +26,7 @@
> IIncrementalDiff,
> IPreviewDiff,
> )
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.model.diff import (
> Diff,
> PreviewDiff,
> @@ -43,6 +44,7 @@
> verifyObject,
> )
> from lp.testing.fakemethod import FakeMethod
> +from lp.testing.fixture import ZopeUtilityFixture
> from lp.testing.layers import (
> LaunchpadFunctionalLayer,
> LaunchpadZopelessLayer,
> @@ -90,7 +92,8 @@
>
>
> class FakeGitHostingClient:
> - pass
> +
> + implements(IGitHostingClient)
>
>
> class DiffTestCase(TestCaseWithFactory):
> @@ -139,9 +142,7 @@
> hosting_client = FakeGitHostingClient()
> hosting_client.getMergeDiff = FakeMethod(
> result=result, failure=failure)
> - self.useFixture(MonkeyPatch(
> - "lp.code.model.diff.PreviewDiff.getGitHostingClient",
> - staticmethod(lambda: hosting_client)))
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
>
> def createExampleGitMerge(self):
> """Create an example Git-based merge scenario.
>
> === modified file 'lib/lp/code/model/tests/test_gitjob.py'
> --- lib/lp/code/model/tests/test_gitjob.py 2015-05-26 13:35:50 +0000
> +++ lib/lp/code/model/tests/test_gitjob.py 2015-06-12 21:46:06 +0000
> @@ -16,9 +16,11 @@
> MatchesSetwise,
> MatchesStructure,
> )
> +from zope.interface import implements
> from zope.security.proxy import removeSecurityProxy
>
> from lp.code.enums import GitObjectType
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.interfaces.gitjob import (
> IGitJob,
> IGitRefScanJob,
> @@ -39,12 +41,18 @@
> )
> from lp.testing.dbuser import dbuser
> from lp.testing.fakemethod import FakeMethod
> +from lp.testing.fixture import ZopeUtilityFixture
> from lp.testing.layers import (
> DatabaseFunctionalLayer,
> LaunchpadZopelessLayer,
> )
>
>
> +class FakeGitHostingClient:
> +
> + implements(IGitHostingClient)
Maybe worth defining the FakeMethods here?
> +
> +
> class TestGitJob(TestCaseWithFactory):
> """Tests for `GitJob`."""
>
> @@ -129,26 +137,28 @@
>
> def test_run(self):
> # Ensure the job scans the repository.
> + hosting_client = FakeGitHostingClient()
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> repository = self.factory.makeGitRepository()
> job = GitRefScanJob.create(repository)
> paths = (u"refs/heads/master", u"refs/tags/1.0")
> - job._hosting_client.getRefs = FakeMethod(
> - result=self.makeFakeRefs(paths))
> + hosting_client.getRefs = FakeMethod(result=self.makeFakeRefs(paths))
> author = repository.owner
> author_date_start = datetime(2015, 01, 01, tzinfo=pytz.UTC)
> author_date_gen = time_counter(author_date_start, timedelta(days=1))
> - job._hosting_client.getCommits = FakeMethod(
> + hosting_client.getCommits = FakeMethod(
> result=self.makeFakeCommits(author, author_date_gen, paths))
> with dbuser("branchscanner"):
> JobRunner([job]).runAll()
> self.assertRefsMatch(repository.refs, repository, paths)
>
> def test_logs_bad_ref_info(self):
> + hosting_client = FakeGitHostingClient()
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> repository = self.factory.makeGitRepository()
> job = GitRefScanJob.create(repository)
> - job._hosting_client.getRefs = FakeMethod(
> - result={u"refs/heads/master": {}})
> - job._hosting_client.getCommits = FakeMethod(result=[])
> + hosting_client.getRefs = FakeMethod(result={u"refs/heads/master": {}})
> + hosting_client.getCommits = FakeMethod(result=[])
> expected_message = (
> 'Unconvertible ref refs/heads/master {}: '
> 'ref info does not contain "object" key')
> @@ -206,15 +216,17 @@
> def test_run(self):
> # Running a job to reclaim space sends a request to the hosting
> # service.
> + hosting_client = FakeGitHostingClient()
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> name = "/~owner/+git/gone"
> path = "1"
> job = ReclaimGitRepositorySpaceJob.create(name, path)
> self.makeJobReady(job)
> [job] = list(ReclaimGitRepositorySpaceJob.iterReady())
> with dbuser("branchscanner"):
> - job._hosting_client.delete = FakeMethod()
> + hosting_client.delete = FakeMethod()
> JobRunner([job]).runAll()
> - self.assertEqual([(path,)], job._hosting_client.delete.extract_args())
> + self.assertEqual([(path,)], hosting_client.delete.extract_args())
>
>
> # XXX cjwatson 2015-03-12: We should test that the jobs work via Celery too,
>
> === modified file 'lib/lp/code/model/tests/test_gitrepository.py'
> --- lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 08:20:17 +0000
> +++ lib/lp/code/model/tests/test_gitrepository.py 2015-06-12 21:46:06 +0000
> @@ -14,15 +14,15 @@
> from bzrlib import urlutils
> from lazr.lifecycle.event import ObjectModifiedEvent
> from lazr.lifecycle.snapshot import Snapshot
> +import pytz
> from sqlobject import SQLObjectNotFound
> from storm.store import Store
> -import transaction
> -import pytz
> from testtools.matchers import (
> EndsWith,
> MatchesSetwise,
> MatchesStructure,
> )
> +import transaction
> from zope.component import getUtility
> from zope.event import notify
> from zope.interface import providedBy
> @@ -53,6 +53,7 @@
> BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES,
> )
> from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.interfaces.gitjob import IGitRefScanJobSource
> from lp.code.interfaces.gitlookup import IGitLookup
> from lp.code.interfaces.gitnamespace import (
> @@ -118,6 +119,7 @@
> )
> from lp.testing.dbuser import dbuser
> from lp.testing.fakemethod import FakeMethod
> +from lp.testing.fixture import ZopeUtilityFixture
> from lp.testing.layers import (
> DatabaseFunctionalLayer,
> LaunchpadFunctionalLayer,
> @@ -1052,8 +1054,8 @@
> },
> },
> })
> - refs_to_upsert, refs_to_remove = repository.planRefChanges(
> - hosting_client, "dummy")
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> + refs_to_upsert, refs_to_remove = repository.planRefChanges("dummy")
>
> expected_upsert = {
> u"refs/heads/master": {
> @@ -1094,8 +1096,8 @@
> },
> },
> })
> - self.assertEqual(
> - ({}, set()), repository.planRefChanges(hosting_client, "dummy"))
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> + self.assertEqual(({}, set()), repository.planRefChanges("dummy"))
>
> def test_fetchRefCommits(self):
> # fetchRefCommits fetches detailed tip commit metadata for the
> @@ -1126,6 +1128,7 @@
> u"parents": [],
> u"tree": unicode(hashlib.sha1("").hexdigest()),
> }])
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> refs = {
> u"refs/heads/master": {
> u"sha1": master_sha1,
> @@ -1136,7 +1139,7 @@
> u"type": GitObjectType.COMMIT,
> },
> }
> - GitRepository.fetchRefCommits(hosting_client, "dummy", refs)
> + GitRepository.fetchRefCommits("dummy", refs)
>
> expected_oids = [master_sha1, foo_sha1]
> [(_, observed_oids)] = hosting_client.getCommits.extract_args()
>
> === modified file 'lib/lp/code/xmlrpc/git.py'
> --- lib/lp/code/xmlrpc/git.py 2015-05-26 10:54:35 +0000
> +++ lib/lp/code/xmlrpc/git.py 2015-06-12 21:46:06 +0000
> @@ -22,15 +22,16 @@
> from lp.app.validators import LaunchpadValidationError
> from lp.code.errors import (
> GitRepositoryCreationException,
> + GitRepositoryCreationFault,
> GitRepositoryCreationForbidden,
> - GitRepositoryCreationFault,
> GitRepositoryExists,
> GitTargetError,
> InvalidNamespace,
> )
> -from lp.code.githosting import GitHostingClient
> from lp.code.interfaces.codehosting import LAUNCHPAD_ANONYMOUS
> from lp.code.interfaces.gitapi import IGitAPI
> +from lp.code.interfaces.githosting import IGitHostingClient
> +from lp.code.interfaces.gitjob import IGitRefScanJobSource
> from lp.code.interfaces.gitlookup import (
> IGitLookup,
> IGitTraverser,
> @@ -40,7 +41,6 @@
> split_git_unique_name,
> )
> from lp.code.interfaces.gitrepository import IGitRepositorySet
> -from lp.code.interfaces.gitjob import IGitRefScanJobSource
> from lp.code.xmlrpc.codehosting import run_with_login
> from lp.registry.errors import (
> InvalidName,
> @@ -52,7 +52,6 @@
> NoSuchProduct,
> )
> from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
> -from lp.services.config import config
> from lp.services.webapp import LaunchpadXMLRPCView
> from lp.services.webapp.authorization import check_permission
> from lp.services.webapp.errorlog import ScriptRequest
> @@ -67,8 +66,6 @@
>
> def __init__(self, *args, **kwargs):
> super(GitAPI, self).__init__(*args, **kwargs)
> - self.hosting_client = GitHostingClient(
> - config.codehosting.internal_git_api_endpoint)
> self.repository_set = getUtility(IGitRepositorySet)
>
> def _performLookup(self, path):
> @@ -211,15 +208,16 @@
> else:
> default = self.repository_set.getDefaultRepositoryForOwner(
> repository.owner, repository.target)
> - if default is not None and default.visibleByUser(requester):
> + if (default is not None and
> + default.visibleByUser(requester)):
> target_path = default.getInternalPath()
> except GitTargetError:
> pass # Ignore Personal repositories.
>
> hosting_path = repository.getInternalPath()
> try:
> - self.hosting_client.create(hosting_path,
> - clone_from=target_path)
> + getUtility(IGitHostingClient).create(
> + hosting_path, clone_from=target_path)
> except GitRepositoryCreationFault as e:
> # The hosting service failed. Log an OOPS for investigation.
> self._reportError(path, e, hosting_path=hosting_path)
>
> === modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
> --- lib/lp/code/xmlrpc/tests/test_git.py 2015-05-26 10:54:35 +0000
> +++ lib/lp/code/xmlrpc/tests/test_git.py 2015-06-12 21:46:06 +0000
> @@ -6,6 +6,7 @@
> __metaclass__ = type
>
> from zope.component import getUtility
> +from zope.interface import implements
> from zope.security.proxy import removeSecurityProxy
>
> from lp.app.enums import InformationType
> @@ -15,6 +16,7 @@
> LAUNCHPAD_SERVICES,
> )
> from lp.code.interfaces.gitcollection import IAllGitRepositories
> +from lp.code.interfaces.githosting import IGitHostingClient
> from lp.code.interfaces.gitjob import IGitRefScanJobSource
> from lp.code.interfaces.gitrepository import (
> GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE,
> @@ -30,6 +32,7 @@
> person_logged_in,
> TestCaseWithFactory,
> )
> +from lp.testing.fixture import ZopeUtilityFixture
> from lp.testing.layers import (
> AppServerLayer,
> LaunchpadFunctionalLayer,
> @@ -40,6 +43,8 @@
> class FakeGitHostingClient:
> """A GitHostingClient lookalike that just logs calls."""
>
> + implements(IGitHostingClient)
> +
> def __init__(self):
> self.calls = []
>
> @@ -50,6 +55,8 @@
> class BrokenGitHostingClient:
> """A GitHostingClient lookalike that pretends the remote end is down."""
>
> + implements(IGitHostingClient)
> +
> def create(self, path, clone_from=None):
> raise GitRepositoryCreationFault("nothing here")
>
> @@ -60,7 +67,9 @@
> def setUp(self):
> super(TestGitAPIMixin, self).setUp()
> self.git_api = GitAPI(None, None)
> - self.git_api.hosting_client = FakeGitHostingClient()
> + self.hosting_client = FakeGitHostingClient()
> + self.useFixture(
> + ZopeUtilityFixture(self.hosting_client, IGitHostingClient))
> self.repository_set = getUtility(IGitRepositorySet)
>
> def assertPathTranslationError(self, requester, path, permission="read",
> @@ -167,15 +176,14 @@
> translation)
> self.assertEqual(
> ("create", repository.getInternalPath()),
> - self.git_api.hosting_client.calls[0][0:2])
> + self.hosting_client.calls[0][0:2])
> return repository
>
> def assertCreatesFromClone(self, requester, path, cloned_from,
> can_authenticate=False):
> self.assertCreates(requester, path, can_authenticate)
> self.assertEqual(
> - cloned_from.getInternalPath(),
> - self.git_api.hosting_client.calls[0][2])
> + cloned_from.getInternalPath(), self.hosting_client.calls[0][2])
>
> def test_translatePath_private_repository(self):
> requester = self.factory.makePerson()
> @@ -603,7 +611,8 @@
> def test_translatePath_create_broken_hosting_service(self):
> # If the hosting service is down, trying to create a repository
> # fails and doesn't leave junk around in the Launchpad database.
> - self.git_api.hosting_client = BrokenGitHostingClient()
> + hosting_client = BrokenGitHostingClient()
> + self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
> requester = self.factory.makePerson()
> initial_count = getUtility(IAllGitRepositories).count()
> oops_id = self.assertOopsOccurred(
>
> === modified file 'lib/lp/testing/fixture.py'
> --- lib/lp/testing/fixture.py 2013-06-20 05:50:00 +0000
> +++ lib/lp/testing/fixture.py 2015-06-12 21:46:06 +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).
>
> """Launchpad test fixtures that have no better home."""
> @@ -228,7 +228,7 @@
> class ZopeUtilityFixture(Fixture):
> """A fixture that temporarily registers a different utility."""
>
> - def __init__(self, component, intf, name):
> + def __init__(self, component, intf, name=""):
> """Construct a new fixture.
>
> :param component: An instance of a class that provides this
> @@ -244,10 +244,14 @@
> def setUp(self):
> super(ZopeUtilityFixture, self).setUp()
> gsm = getGlobalSiteManager()
> + original = gsm.queryUtility(self.intf, self.name)
> gsm.registerUtility(self.component, self.intf, self.name)
> self.addCleanup(
> gsm.unregisterUtility,
> self.component, self.intf, self.name)
> + if original is not None:
> + self.addCleanup(
> + gsm.registerUtility, original, self.intf, self.name)
>
>
> class Urllib2Fixture(Fixture):
>
> === modified file 'lib/lp/testing/tests/test_fixture.py'
> --- lib/lp/testing/tests/test_fixture.py 2013-06-20 05:50:00 +0000
> +++ lib/lp/testing/tests/test_fixture.py 2015-06-12 21:46:06 +0000
> @@ -104,16 +104,28 @@
>
> layer = BaseLayer
>
> + def getMailer(self):
> + return getGlobalSiteManager().getUtility(IMailDelivery, 'Mail')
> +
> def test_fixture(self):
> - def get_mailer():
> - return getGlobalSiteManager().getUtility(
> - IMailDelivery, 'Mail')
> fake = DummyMailer()
> # In BaseLayer there should be no mailer by default.
> - self.assertRaises(ComponentLookupError, get_mailer)
> - with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
> - self.assertEquals(get_mailer(), fake)
> - self.assertRaises(ComponentLookupError, get_mailer)
> + self.assertRaises(ComponentLookupError, self.getMailer)
> + with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
> + self.assertEqual(fake, self.getMailer())
> + self.assertRaises(ComponentLookupError, self.getMailer)
> +
> + def test_restores_previous_utility(self):
> + # If there was a previous utility, ZopeUtilityFixture restores it on
> + # cleanup.
> + original_fake = DummyMailer()
> + getGlobalSiteManager().registerUtility(
> + original_fake, IMailDelivery, 'Mail')
> + self.assertEqual(original_fake, self.getMailer())
> + fake = DummyMailer()
> + with ZopeUtilityFixture(fake, IMailDelivery, 'Mail'):
> + self.assertEqual(fake, self.getMailer())
> + self.assertEqual(original_fake, self.getMailer())
>
>
> class TestPGBouncerFixtureWithCA(TestCase):
>
--
https://code.launchpad.net/~cjwatson/launchpad/git-hosting-utility/+merge/261868
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References
