Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1339)

Unified Diff: appengine/findit/crash/loglinear/test/changelist_classifier_test.py

Issue 2605943002: Removing the mutation in the factories for getting dep repositories (Closed)
Patch Set: Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: appengine/findit/crash/loglinear/test/changelist_classifier_test.py
diff --git a/appengine/findit/crash/loglinear/test/changelist_classifier_test.py b/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
index c80db81d3890d503e9f3e3523e06b9caa705459e..3914e7de094c5b3ca17b283972c26d3af84d0c89 100644
--- a/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
+++ b/appengine/findit/crash/loglinear/test/changelist_classifier_test.py
@@ -117,23 +117,12 @@ class LogLinearChangelistClassifierTest(CrashTestSuite):
'TopFrameIndex': 1.,
}
- repository = GitilesRepository(self.GetMockHttpClient())
-
- # TODO(crbug.com/677224): should replace this with an actual factory.
- def MutateTheRepo(dep_url): # pragma: no cover
- """A factory function for returning ``Repository`` objects.
-
- The current definition captures the functionality of before
- we factored out this factory method. That is, it's not really a
- "factory" but rather mutates the main repo object in place. In
- the future this should be changed to do the right thing instead.
- """
- repository.repo_url = dep_url
- return repository
+ def GitilesRepositoryFactory(repo_url): # pragma: no cover
+ return GitilesRepository(self.GetMockHttpClient(), repo_url)
self.changelist_classifier = (
loglinear_changelist_classifier.LogLinearChangelistClassifier(
- repository, MutateTheRepo, weights))
+ GitilesRepositoryFactory, weights))
def testAggregateChangedFilesAggreegates(self):
"""Test that ``AggregateChangedFiles`` does aggregate reasons per file.

Powered by Google App Engine
This is Rietveld 408576698