Changed FindSuspects to take a Repository factory, rather than mutating it

appengine/findit/crash/loglinear/changelist_classifier.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 logging
 import math
 
-from common import chrome_dependency_fetcher
+from common.chrome_dependency_fetcher import ChromeDependencyFetcher
 from crash import changelist_classifier
 from crash.loglinear.changelist_features import min_distance
 from crash.loglinear.changelist_features import top_frame_index
 from crash.loglinear.model import ToFeatureFunction
 from crash.loglinear.model import UnnormalizedLogLinearModel
 from crash.stacktrace import CallStack
 from crash.stacktrace import Stacktrace
 
 
 class LogLinearChangelistClassifier(object):
   """A ``LogLinearModel``-based implementation of CL classification."""
 
-  def __init__(self, repository, weights, top_n_frames=7, top_n_suspects=3):
+  def __init__(self, repository, get_repository, weights, top_n_frames=7,
+               top_n_suspects=3):
     """Args:
       repository (Repository): the Git repository for getting CLs to classify.
+      get_repository (callable): a function from DEP urls to ``Repository``
+        objects, so we can get changelogs and blame for each dep. Notably,
+        to keep the code here generic, we make no assumptions about
+        which subclass of ``Repository`` this function returns. Thus,
+        it is up to the caller to decide what class to return and handle
+        any other arguments that class may require (e.g., an http client
+        for ``GitilesRepository``).
       weights (dict of float): the weights for the features. The keys of
         the dictionary are the names of the feature that weight is
         for. We take this argument as a dict rather than as a list so that
         callers needn't worry about what order to provide the weights in.
       top_n_frames (int): how many frames of each callstack to look at.
       top_n_suspects (int): maximum number of suspects to return.
     """
     self._repository = repository
+    self._dependency_fetcher = ChromeDependencyFetcher(self._repository)
+    self._get_repository = get_repository
     self._top_n_frames = top_n_frames
     self._top_n_suspects = top_n_suspects
 
     feature_function = ToFeatureFunction([
         top_frame_index.TopFrameIndexFeature(top_n_frames),
         min_distance.MinDistanceFeature(),
     ])
 
     weight_list = [
         weights['TopFrameIndex'],
@@ skipping 54 matching lines @@
     logging.info('ChangelistClassifier.__call__: Regression range %s:%s',
         last_good_version, first_bad_version)
 
     # We are only interested in the deps in crash stack (the callstack that
     # caused the crash).
     # TODO(wrengr): we may want to receive the crash deps as an argument,
     # so that when this method is called via Findit.FindCulprit, we avoid
     # doing redundant work creating it.
     stack_deps = changelist_classifier.GetDepsInCrashStack(
         report.stacktrace.crash_stack,
-        chrome_dependency_fetcher.ChromeDependencyFetcher(
-            self._repository).GetDependency(report.crashed_version,
-                                            report.platform))
+        self._dependency_fetcher.GetDependency(
+            report.crashed_version, report.platform))
 
     # Get dep and file to changelogs, stack_info and blame dicts.
-    dep_rolls = chrome_dependency_fetcher.ChromeDependencyFetcher(
-        self._repository).GetDependencyRollsDict(
-            last_good_version, first_bad_version, report.platform)
+    dep_rolls = self._dependency_fetcher.GetDependencyRollsDict(
+        last_good_version, first_bad_version, report.platform)
 
     # Regression of a dep added/deleted (old_revision/new_revision is None) can
     # not be known for sure and this case rarely happens, so just filter them
     # out.
     regression_deps_rolls = {}
     for dep_path, dep_roll in dep_rolls.iteritems():
       if not dep_roll.old_revision or not dep_roll.new_revision:
         logging.info('Skip %s denpendency %s',
                      'added' if dep_roll.new_revision else 'deleted', dep_path)
         continue
       regression_deps_rolls[dep_path] = dep_roll
 
     dep_to_file_to_changelogs, ignore_cls = (
         changelist_classifier.GetChangeLogsForFilesGroupedByDeps(
             regression_deps_rolls, stack_deps, self._repository))
     dep_to_file_to_stack_infos = (
         changelist_classifier.GetStackInfosForFilesGroupedByDeps(
             report.stacktrace, stack_deps))
 
     # Get the possible suspects.
     suspects = changelist_classifier.FindSuspects(
         dep_to_file_to_changelogs,
         dep_to_file_to_stack_infos,
         stack_deps,
-        self._repository,
+        self._get_repository,
         ignore_cls)
     if suspects is None:
       return []
 
     # Score the suspects and organize them for outputting/returning.
     features_given_report = self._model.Features(report)
     score_given_report = self._model.Score(report)
     scored_suspects = []
     for suspect in suspects:
       score = score_given_report(suspect)
@@ skipping 73 matching lines @@
           all_changed_files[changed_file.name] = changed_file
           continue
 
         assert accumulated_changed_file.blame_url == changed_file.blame_url, (
             ValueError('Blame URLs do not match: %s != %s'
                 % (accumulated_changed_file.blame_url, changed_file.blame_url)))
         accumulated_changed_file.reasons.extend(changed_file.reasons or [])
 
     return sorted(all_changed_files.values(),
         key=lambda changed_file: changed_file.name)