Removing the mutation in the factories for getting dep repositories

appengine/findit/crash/crash_pipeline.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import copy
 import json
 import logging
 
 from crash import monitoring
 
 from common import appengine_util
 from common import pubsub_util
 from common.http_client_appengine import HttpClientAppengine
 from common.pipeline_wrapper import BasePipeline
 from common.pipeline_wrapper import pipeline
 from crash import findit_for_chromecrash
 from crash import findit_for_clusterfuzz
 from crash.type_enums import CrashClient
 from libs import time_util
 from gae_libs.gitiles import cached_gitiles_repository
 from model import analysis_status
 
 
 # TODO(http://crbug.com/659346): write complete coverage tests for this.
-def FinditForClientID(client_id, repository): # pragma: no cover
+def FinditForClientID(client_id, get_repository): # pragma: no cover
   """Construct a Findit object from a client id string specifying the class.
 
   We cannot pass Findit objects to the various methods in
   ``crash.crash_pipeline``, because they are not JSON serializable. For
   now, we just serialize Findit objects as their ``client_id``, and then
   use this function to reconstruct them. Alas, this means we will lose
   various other information stored in the Findit object (i.e., stuff that
   comes from CrashConfig); which could lead to some hard-to-diagnose
   coherency bugs, since the new Findit object will be based on the
   CrashConfig at the time it's constructed, which may be different
   than the CrashConfig at the time the previous Findit object was
   constructed. In the future we should fix all this to serialize Findit
   objects in a more robust way.
   """
   assert isinstance(client_id, (str, unicode)), (
       'FinditForClientID: expected string or unicode, but got %s'
       % client_id.__class__.__name__)
   # TODO(wrengr): it'd be nice to replace this with a single lookup in
   # a dict; but that's buggy for some unknown reason.
   if client_id == CrashClient.FRACAS:
     cls = findit_for_chromecrash.FinditForFracas
   elif client_id == CrashClient.CRACAS: # pragma: no cover
     cls = findit_for_chromecrash.FinditForCracas
   elif client_id == CrashClient.CLUSTERFUZZ: # pragma: no cover
     cls = findit_for_clusterfuzz.FinditForClusterfuzz
   else: # pragma: no cover
     raise ValueError('FinditForClientID: '
         'unknown or unsupported client %s' % client_id)
 
-  return cls(repository)
+  return cls(get_repository)
 
 
 # Some notes about the classes below, for people who are not familiar
 # with AppEngine pipelines:
 #
 # The pipeline library is designed in a strange way which requires that
 # all the ``__init__`` and ``run`` methods in this file take the exact
 # same arguments. This arises from the interaction between a few
 # different constraints. First, for any given pipeline, its ``__init__``
 # and ``run`` must take the same arguments. Second, all the objects that
@@ skipping 14 matching lines @@
 # serialization. Thus, we cannot pass a ``Findit`` object directly to
 # any of the methods here, but rather must instead pass the ``client_id``
 # (or whatever JSON dict), and then reconstruct the ``Findit`` object
 # from that data.
 #
 # Moreover, the ``run`` and ``finalized`` methods are executed in separate
 # processes, so we'll actually end up reconstructing the ``Findit`` object
 # twice. This also means ``run`` can't store anything in the pipeline
 # object and expect it to still be available in the ``finalized`` method.
 
+def _CachedGitilesRepositoryFactory(repo_url): # pragma: no cover

[Sharu Jiang, 2016/12/28 23:05:12]

what's the reason for ``pragma: no cover`` here?

[wrengr, 2016/12/29 20:57:31]

Apparently it's never called by the unit tests ¯\_(ツ)_/¯

+  return cached_gitiles_repository.CachedGitilesRepository(
+      HttpClientAppengine(), repo_url)
+
+
 class CrashBasePipeline(BasePipeline):
   def __init__(self, client_id, crash_identifiers):
     super(CrashBasePipeline, self).__init__(client_id, crash_identifiers)
     self._crash_identifiers = crash_identifiers
-    self._findit = FinditForClientID(
-        client_id,
-        cached_gitiles_repository.CachedGitilesRepository(
-            HttpClientAppengine()))
+    self._findit = FinditForClientID(client_id, _CachedGitilesRepositoryFactory)
 
   @property
   def client_id(self): # pragma: no cover
     return self._findit.client_id
 
   def run(self, *args, **kwargs):
     raise NotImplementedError()
 
 
 class CrashAnalysisPipeline(CrashBasePipeline):
@@ skipping 122 matching lines @@
     N.B., due to the structure of AppEngine pipelines, this method must
     accept the same arguments as are passed to ``__init__``; however,
     because they were already passed to ``__init__`` there's no use in
     recieving them here. Thus, we discard all the arguments to this method
     (except for ``self``, naturally).
     """
     run_analysis = yield CrashAnalysisPipeline(
         self._client_id, self._crash_identifiers)
     with pipeline.After(run_analysis):
       yield PublishResultPipeline(self._client_id, self._crash_identifiers)