validation_pool: pool.changes -> pool.candidates, pool.applied.

Validation pool had a list of changes, and one would know if they were
applied or not by knowing if ApplyPoolIntoRepo had been called.

This change breaks pool.changes into a pool.candidates (the list of
changes being considered for application), and pool.applied (the list of
changes that have been appled to the current repo).

The point is that it's now safe to call ApplyPoolIntoRepo() more than
once. In particular, that function now accepts a filter argument. This
means we can apply changes which match a filter, do some work, then come
back and apply the rest of the changes.

A follow on change will start applying manifest changes first, doing
work to include those changes in the manifest used by the CQ, and then
to apply the rest of the changes on top of the patched manifest.

BUG=chromium:549844
TEST=cbuildbot/run_tests

Change-Id: I2275248d19d6ba64e255258eeeea771d827a8022
Reviewed-on: https://chromium-review.googlesource.com/311393
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
diff --git a/cbuildbot/lkgm_manager.py b/cbuildbot/lkgm_manager.py
index 947b7b2..a2303f0 100644
--- a/cbuildbot/lkgm_manager.py
+++ b/cbuildbot/lkgm_manager.py
@@ -269,7 +269,7 @@
           not config_lib.IsPFQType(self.build_type)):
         return None
 
-      self._AddPatchesToManifest(new_manifest, validation_pool.changes)
+      self._AddPatchesToManifest(new_manifest, validation_pool.applied)
 
       # Add info about the last known good version to the manifest. This will
       # be used by slaves to calculate what artifacts from old builds are safe
@@ -288,7 +288,7 @@
 
         # If we don't have any valid changes to test, make sure the checkout
         # is at least different.
-        if ((not validation_pool or not validation_pool.changes) and
+        if ((not validation_pool or not validation_pool.applied) and
             not self.force and self.HasCheckoutBeenBuilt()):
           return None
 
diff --git a/cbuildbot/stages/completion_stages.py b/cbuildbot/stages/completion_stages.py
index e1467d3..e18eaf6 100644
--- a/cbuildbot/stages/completion_stages.py
+++ b/cbuildbot/stages/completion_stages.py
@@ -640,7 +640,7 @@
     messages = self._GetFailedMessages(failing)
     self.SendInfraAlertIfNeeded(failing, inflight, no_stat)
 
-    changes = self.sync_stage.pool.changes
+    changes = self.sync_stage.pool.applied
 
     do_partial_submission = self._ShouldSubmitPartialPool()
 
diff --git a/cbuildbot/stages/completion_stages_unittest.py b/cbuildbot/stages/completion_stages_unittest.py
index 7e900be..af5ac87 100644
--- a/cbuildbot/stages/completion_stages_unittest.py
+++ b/cbuildbot/stages/completion_stages_unittest.py
@@ -352,7 +352,7 @@
     """
     sync_stage = sync_stages.CommitQueueSyncStage(self._run)
     sync_stage.pool = mock.MagicMock()
-    sync_stage.pool.changes = self.changes
+    sync_stage.pool.applied = self.changes
     sync_stage.pool.tree_was_open = tree_was_open
 
     sync_stage.pool.handle_failure_mock = self.PatchObject(
diff --git a/cbuildbot/stages/sync_stages.py b/cbuildbot/stages/sync_stages.py
index 8eb6ce7..e682f42 100644
--- a/cbuildbot/stages/sync_stages.py
+++ b/cbuildbot/stages/sync_stages.py
@@ -1058,7 +1058,7 @@
     else:
       ManifestVersionedSyncStage.PerformStage(self)
 
-    self.WriteChangesToMetadata(self.pool.changes)
+    self.WriteChangesToMetadata(self.pool.applied)
 
 
 class PreCQSyncStage(SyncStage):
@@ -1091,14 +1091,14 @@
     self.pool = validation_pool.ValidationPool.AcquirePreCQPool(
         self._run.config.overlays, self._build_root,
         self._run.buildnumber, self._run.config.name,
-        dryrun=self._run.options.debug_forced, changes=self.patches,
+        dryrun=self._run.options.debug_forced, candidates=self.patches,
         builder_run=self._run)
     self.pool.ApplyPoolIntoRepo()
 
-    if len(self.pool.changes) == 0 and self.patches:
+    if len(self.pool.applied) == 0 and self.patches:
       cros_build_lib.Die('No changes have been applied.')
 
-    changes = self.pool.changes or self.patches
+    changes = self.pool.applied or self.patches
     self.WriteChangesToMetadata(changes)
 
 class PreCQLauncherStage(SyncStage):
diff --git a/cbuildbot/stages/sync_stages_unittest.py b/cbuildbot/stages/sync_stages_unittest.py
index e2dc829..f79a906 100644
--- a/cbuildbot/stages/sync_stages_unittest.py
+++ b/cbuildbot/stages/sync_stages_unittest.py
@@ -526,38 +526,38 @@
   def testCommitNonManifestChange(self):
     """See MasterCQSyncTestCase"""
     changes = self._testCommitNonManifestChange()
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testFailedCommitOfNonManifestChange(self):
     """See MasterCQSyncTestCase"""
     changes = self._testFailedCommitOfNonManifestChange()
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testCommitManifestChange(self):
     """See MasterCQSyncTestCase"""
     changes = self._testCommitManifestChange()
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testCommitManifestChangeWithoutPreCQ(self):
     """Changes get ignored if they aren't approved by pre-cq."""
     self._testCommitManifestChange(pre_cq_status=None)
-    self.assertItemsEqual(self.sync_stage.pool.changes, [])
+    self.assertItemsEqual(self.sync_stage.pool.candidates, [])
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testCommitManifestChangeWithoutPreCQAndOldPatches(self):
     """Changes get tested without pre-cq if the approval_timestamp is old."""
     changes = self._testCommitManifestChange(pre_cq_status=None,
                                              approval_timestamp=0)
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testDefaultSync(self):
     """See MasterCQSyncTestCase"""
     changes = self._testDefaultSync()
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testReload(self):
@@ -565,7 +565,7 @@
     # Use zero patches because mock patches can't be pickled.
     changes = self.PerformSync(num_patches=0, runs=0)
     self.ReloadPool()
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
   def testTreeClosureBlocksCommit(self):
@@ -579,7 +579,7 @@
     gerrit.GerritHelper.Query.assert_called_with(
         mock.ANY, constants.THROTTLED_CQ_READY_QUERY[0],
         sort='lastUpdated')
-    self.assertItemsEqual(self.sync_stage.pool.changes, changes)
+    self.assertItemsEqual(self.sync_stage.pool.candidates, changes)
     self.assertItemsEqual(self.sync_stage.pool.non_manifest_changes, [])
 
 
diff --git a/cbuildbot/validation_pool.py b/cbuildbot/validation_pool.py
index 15f18e9..4c0bde0 100644
--- a/cbuildbot/validation_pool.py
+++ b/cbuildbot/validation_pool.py
@@ -1120,9 +1120,9 @@
 
 
   def __init__(self, overlays, build_root, build_number, builder_name,
-               is_master, dryrun, changes=None, non_os_changes=None,
+               is_master, dryrun, candidates=None, non_os_changes=None,
                conflicting_changes=None, pre_cq_trybot=False,
-               tree_was_open=True, _applied=None, builder_run=None):
+               tree_was_open=True, applied=None, builder_run=None):
     """Initializes an instance by setting default variables to instance vars.
 
     Generally use AcquirePool as an entry pool to a pool rather than this
@@ -1136,7 +1136,7 @@
       is_master: True if this is the master builder for the Commit Queue.
       dryrun: If set to True, do not submit anything to Gerrit.
     Optional Args:
-      changes: List of changes for this validation pool.
+      candidates: List of changes to consider validating.
       non_os_changes: List of changes that are part of this validation
         pool but aren't part of the cros checkout.
       conflicting_changes: Changes that failed to apply but we're keeping around
@@ -1144,8 +1144,7 @@
       pre_cq_trybot: If set to True, this is a Pre-CQ trybot. (Note: The Pre-CQ
         launcher is NOT considered a Pre-CQ trybot.)
       tree_was_open: Whether the tree was open when the pool was created.
-      applied: List of CLs that have been applied to the current repo. Not
-        yet used, but needs to be here for pickling compatibility.
+      applied: List of CLs that have been applied to the current repo.
       builder_run: BuilderRun instance used to fetch cidb handle and metadata
         instance. Please note due to the pickling logic, this MUST be the last
         kwarg listed.
@@ -1167,7 +1166,9 @@
       raise ValueError("Invalid builder_name: %r" % (builder_name,))
 
     for changes_name, changes_value in (
-        ('changes', changes), ('non_os_changes', non_os_changes)):
+        ('candidates', candidates),
+        ('non_os_changes', non_os_changes),
+        ('applied', applied)):
       if not changes_value:
         continue
       if not all(isinstance(x, cros_patch.GitRepoPatch) for x in changes_value):
@@ -1195,8 +1196,17 @@
       self.queue = 'The Commit Queue'
 
     # See optional args for types of changes.
-    self.changes = changes or []
+    self.candidates = candidates or []
     self.non_manifest_changes = non_os_changes or []
+
+    # TODO(dgarrett): Remove this if block after pickle changes settle down.
+    if applied is None:
+      # If we received a pickle from an older version, it will use the
+      # default applied value. All candidates were already applied.
+      applied = self.candidates[:]
+
+    self.applied = applied or []
+
     # Note, we hold onto these CLs since they conflict against our current CLs
     # being tested; if our current ones succeed, we notify the user to deal
     # w/ the conflict.  If the CLs we're testing fail, then there is no
@@ -1241,11 +1251,12 @@
         (
             self._overlays,
             self.build_root, self._build_number, self._builder_name,
-            self.is_master, self.dryrun, self.changes,
+            self.is_master, self.dryrun, self.candidates,
             self.non_manifest_changes,
             self.changes_that_failed_to_apply_earlier,
             self.pre_cq_trybot,
-            self.tree_was_open))
+            self.tree_was_open,
+            self.applied))
 
   @classmethod
   @failures_lib.SetFailureType(failures_lib.BuilderFailure)
@@ -1298,14 +1309,14 @@
 
       changes, non_manifest_changes = ValidationPool._FilterNonCrosProjects(
           changes, git.ManifestCheckout.Cached(self.build_root))
-      self.changes.extend(changes)
+      self.candidates.extend(changes)
       self.non_manifest_changes.extend(non_manifest_changes)
 
     # Filter out unwanted changes.
-    self.changes, self.non_manifest_changes = change_filter(
-        self, self.changes, self.non_manifest_changes)
+    self.candidates, self.non_manifest_changes = change_filter(
+        self, self.candidates, self.non_manifest_changes)
 
-    return self.changes or self.non_manifest_changes
+    return self.candidates or self.non_manifest_changes
 
   @classmethod
   def AcquirePool(cls, overlays, repo, build_number, builder_name, query,
@@ -1454,7 +1465,7 @@
         attr_dict[name] = pc.getAttribute(name)
       patch = cros_patch.GerritFetchOnlyPatch.FromAttrDict(attr_dict)
 
-      self.changes.append(patch)
+      self.candidates.append(patch)
 
   @classmethod
   def AcquirePoolFromManifest(cls, manifest, overlays, repo, build_number,
@@ -1619,6 +1630,8 @@
   def FilterChangesForThrottledTree(self):
     """Apply Throttled Tree logic to select patch candidates.
 
+    This should be called before any CLs are applied.
+
     If the tree is throttled, we only test a random subset of our candidate
     changes. Call this to select that subset, and throw away unrelated changes.
 
@@ -1627,12 +1640,14 @@
     if self.tree_was_open:
       return
 
+    # By filtering the candidates before any calls to Apply, we can make sure
+    # that repeated calls to Apply always consider the same list of candidates.
     fail_streak = self._GetFailStreak()
-    test_pool_size = max(1, len(self.changes) / (2**fail_streak))
-    random.shuffle(self.changes)
-    self.changes = self.changes[:test_pool_size]
+    test_pool_size = max(1, len(self.candidates) / (2**fail_streak))
+    random.shuffle(self.candidates)
+    self.candidates = self.candidates[:test_pool_size]
 
-  def ApplyPoolIntoRepo(self, manifest=None):
+  def ApplyPoolIntoRepo(self, manifest=None, filter_fn=lambda p: True):
     """Applies changes from pool into the directory specified by the buildroot.
 
     This method applies changes in the order specified. If the build
@@ -1640,6 +1655,13 @@
     order. Otherwise, the changes should already be listed in an order
     that will not break the dependency order.
 
+    It is safe to call this method more than once, probably with different
+    filter functions. A given patch will never be applied more than  once.
+
+    Args:
+      manifest: A manifest object to use for mapping projects to repositories.
+      filter_fn: Takes a patch argument, returns bool for 'should apply'.
+
     Returns:
       True if we managed to apply any changes.
     """
@@ -1652,9 +1674,12 @@
 
     if self.is_master:
       try:
+        candidates = [c for c in self.candidates if
+                      c not in self.applied and filter_fn(c)]
+
         # pylint: disable=E1123
         applied, failed_tot, failed_inflight = patch_series.Apply(
-            self.changes, manifest=manifest)
+            candidates, manifest=manifest)
       except (KeyboardInterrupt, RuntimeError, SystemExit):
         raise
       except Exception as e:
@@ -1667,9 +1692,9 @@
             'commit queue does not go into an infinite loop retrying '
             'patches.' % (e,)
         )
-        links = cros_patch.GetChangesAsString(self.changes)
+        links = cros_patch.GetChangesAsString(self.candidates)
         logging.error('%s\nAffected Patches are: %s', msg, links)
-        errors = [InternalCQError(patch, msg) for patch in self.changes]
+        errors = [InternalCQError(patch, msg) for patch in self.candidates]
         self._HandleApplyFailure(errors)
         raise
 
@@ -1691,7 +1716,7 @@
       # Slaves do not need to create transactions and should simply
       # apply the changes serially, based on the order that the
       # changes were listed on the manifest.
-      for change in self.changes:
+      for change in self.candidates:
         try:
           # pylint: disable=E1123
           patch_series.ApplyChange(change, manifest=manifest)
@@ -1733,9 +1758,9 @@
         self._HandleFailedToApplyDueToInflightConflict(x.patch)
 
     self.changes_that_failed_to_apply_earlier.extend(failed_inflight)
-    self.changes = applied
+    self.applied.extend(applied)
 
-    return bool(self.changes)
+    return bool(applied)
 
   @staticmethod
   def Load(filename, builder_run=None):
@@ -2323,7 +2348,7 @@
     # a CQ run (since the submit state has changed, we have no way of
     # knowing).  They *likely* will still fail, but this approach tries
     # to minimize wasting the developers time.
-    submitted, errors = self.SubmitChanges(self.changes,
+    submitted, errors = self.SubmitChanges(self.applied,
                                            check_tree_open=check_tree_open,
                                            throttled_ok=throttled_ok,
                                            reason=reason)
@@ -2453,7 +2478,7 @@
         not sane, none of the changes will have their CommitReady bit modified.
     """
     if changes is None:
-      changes = self.changes
+      changes = self.applied
 
     logging.info('Validation timed out for all changes.')
     base_msg = ('%(queue)s timed out while verifying your change in '
@@ -2518,7 +2543,7 @@
         self._InsertCLActionToDatabase(change, constants.CL_ACTION_VERIFIED)
 
     # Process the changes in parallel.
-    inputs = [[change] for change in self.changes]
+    inputs = [[change] for change in self.applied]
     parallel.RunTasksInProcessPool(ProcessChange, inputs)
 
   def _HandleCouldNotSubmit(self, change, error=''):
@@ -2675,7 +2700,7 @@
         status. If not None, this implies there were infrastructure issues.
     """
     if changes is None:
-      changes = self.changes
+      changes = self.applied
 
     candidates = []
 
diff --git a/cbuildbot/validation_pool_unittest.py b/cbuildbot/validation_pool_unittest.py
index 3fab2d1..fbf47e1 100644
--- a/cbuildbot/validation_pool_unittest.py
+++ b/cbuildbot/validation_pool_unittest.py
@@ -652,7 +652,7 @@
              builder_name='foon', is_master=True, dryrun=True,
              fake_db=None, **kwargs):
   """Helper for creating ValidationPool objects for tests."""
-  kwargs.setdefault('changes', [])
+  kwargs.setdefault('candidates', [])
   build_root = kwargs.pop('build_root', '/fake_root')
 
   builder_run = FakeBuilderRun(fake_db)
@@ -785,7 +785,7 @@
 
   def setUp(self):
     self._patches = self.GetPatches(3)
-    self._pool = MakePool(changes=self._patches, fake_db=self.fake_db)
+    self._pool = MakePool(applied=self._patches, fake_db=self.fake_db)
 
     self.PatchObject(
         triage_lib.CalculateSuspects, 'FindSuspects',
@@ -878,8 +878,8 @@
     return cros_patch.ApplyPatchException(patch, inflight=inflight)
 
   def GetPool(self, changes, applied=(), tot=(), inflight=(), **kwargs):
-
-    pool = self.MakePool(changes=changes, fake_db=self.fake_db, **kwargs)
+    pool = self.MakePool(
+        candidates=changes, applied=[], fake_db=self.fake_db, **kwargs)
     applied = list(applied)
     tot = [self.MakeFailure(x, inflight=False) for x in tot]
     inflight = [self.MakeFailure(x, inflight=True) for x in inflight]
@@ -909,7 +909,7 @@
     """Verifies that slave calls ApplyChange() directly for each patch."""
     slave_pool = self.MakePool(is_master=False)
     patches = self.GetPatches(3)
-    slave_pool.changes = patches
+    slave_pool.candidates = patches
     for patch in patches:
       # pylint: disable=E1120, E1123
       validation_pool.PatchSeries.ApplyChange(patch, manifest=mox.IgnoreArg())
@@ -920,7 +920,7 @@
 
   def runApply(self, pool, result):
     self.assertEqual(result, pool.ApplyPoolIntoRepo())
-    self.assertEqual(pool.changes, pool._test_data[1])
+    self.assertEqual(pool.applied, pool._test_data[1])
     failed_inflight = pool.changes_that_failed_to_apply_earlier
     expected_inflight = set(pool._test_data[3])
     # Intersect the results, since it's possible there were results failed
@@ -928,8 +928,6 @@
     self.assertEqual(set(failed_inflight).intersection(expected_inflight),
                      expected_inflight)
 
-    self.assertEqual(pool.changes, pool._test_data[1])
-
   def testPatchSeriesInteraction(self):
     """Verify the interaction between PatchSeries and ValidationPool.
 
@@ -991,7 +989,7 @@
     pool = self.MakePool(dryrun=False, handlers=True)
     patches = self.GetPatches(3)
     failed = self.GetPatches(3)
-    pool.changes = patches[:]
+    pool.applied = patches[:]
     # While we don't do anything w/ these patches, that's
     # intentional; we're verifying that it isn't submitted
     # if there is a failure.
@@ -1124,6 +1122,9 @@
     """Test that CQ doesn't loop due to unhandled Exceptions."""
     pool, patches, _failed = self._setUpSubmit()
 
+    pool.candidates = pool.applied
+    pool.applied = []
+
     class MyException(Exception):
       """"Unique Exception used for testing."""
 
@@ -1371,11 +1372,11 @@
     self.mox.ReplayAll()
 
     # Perform test.
-    slave_pool = self.MakePool(changes=patches, tree_was_open=True)
+    slave_pool = self.MakePool(candidates=patches, tree_was_open=True)
     slave_pool.FilterChangesForThrottledTree()
 
     # Validate results.
-    self.assertEqual(len(slave_pool.changes), 4)
+    self.assertEqual(len(slave_pool.candidates), 4)
     self.mox.VerifyAll()
     self.mox.ResetAll()
 
@@ -1388,11 +1389,11 @@
     self.mox.ReplayAll()
 
     # Perform test.
-    slave_pool = self.MakePool(changes=patches, tree_was_open=False)
+    slave_pool = self.MakePool(candidates=patches, tree_was_open=False)
     slave_pool.FilterChangesForThrottledTree()
 
     # Validate results.
-    self.assertEqual(len(slave_pool.changes), 2)
+    self.assertEqual(len(slave_pool.candidates), 2)
     self.mox.VerifyAll()
     self.mox.ResetAll()
 
@@ -1405,11 +1406,11 @@
     self.mox.ReplayAll()
 
     # Perform test.
-    slave_pool = self.MakePool(changes=patches, tree_was_open=False)
+    slave_pool = self.MakePool(candidates=patches, tree_was_open=False)
     slave_pool.FilterChangesForThrottledTree()
 
     # Validate results.
-    self.assertEqual(len(slave_pool.changes), 1)
+    self.assertEqual(len(slave_pool.candidates), 1)
     self.mox.VerifyAll()
     self.mox.ResetAll()
 
@@ -1422,11 +1423,11 @@
     self.mox.ReplayAll()
 
     # Perform test.
-    slave_pool = self.MakePool(changes=patches, tree_was_open=False)
+    slave_pool = self.MakePool(candidates=patches, tree_was_open=False)
     slave_pool.FilterChangesForThrottledTree()
 
     # Validate results.
-    self.assertEqual(len(slave_pool.changes), 1)
+    self.assertEqual(len(slave_pool.candidates), 1)
     self.mox.VerifyAll()
 
 
@@ -1495,7 +1496,7 @@
         constants.PUBLIC_OVERLAYS,
         '/fake/pathway', 1,
         'testing', True, True,
-        changes=changes, non_os_changes=non_os,
+        candidates=changes, non_os_changes=non_os,
         conflicting_changes=conflicting)
     return pickle.dumps([pool, changes, non_os, conflicting])
 
@@ -1510,7 +1511,7 @@
       for s_item, v_item in zip(source, value):
         assert getter(s_item).id == getter(v_item).id
         assert getter(s_item).remote == getter(v_item).remote
-    _f(pool.changes, changes)
+    _f(pool.candidates, changes)
     _f(pool.non_manifest_changes, non_os)
     _f(pool.changes_that_failed_to_apply_earlier, conflicting,
        getter=lambda s: getattr(s, 'patch', s))
@@ -1640,8 +1641,8 @@
       # the specified length, ignoring any remaining patches.
       expected_plans = [txn[:max_txn_length] for txn in txns]
 
-    pool = MakePool(changes=patches)
-    plans = pool.CreateDisjointTransactions(None, pool.changes,
+    pool = MakePool(candidates=patches)
+    plans = pool.CreateDisjointTransactions(None, pool.candidates,
                                             max_txn_length=max_txn_length)
 
     # If the dependencies are circular, the order of the patches is not
@@ -1665,7 +1666,7 @@
     notify = self.PatchObject(validation_pool.ValidationPool,
                               'SendNotification')
     remove = self.PatchObject(gerrit.GerritHelper, 'RemoveReady')
-    pool = MakePool(changes=changes)
+    pool = MakePool(candidates=changes)
     plans = pool.CreateDisjointTransactions(None, changes,
                                             max_txn_length=max_txn_length)
     self.assertEqual(plans, [])
@@ -1764,7 +1765,7 @@
     self.patches = self.GetPatches(2)
 
   def SetUpPatchPool(self, failed_to_apply=False):
-    pool = MakePool(changes=self.patches, dryrun=False)
+    pool = MakePool(candidates=self.patches, dryrun=False)
     if failed_to_apply:
       # Create some phony errors and add them to the pool.
       errors = []
@@ -1788,7 +1789,7 @@
     with mock.patch.object(git.ManifestCheckout, 'Cached', new=mock_manifest):
       if not self.ALL_BUILDS_PASSED:
         actually_rejected = sorted(pool.SubmitPartialPool(
-            pool.changes, mock.ANY, dict(), [], [], [], reason=reason))
+            pool.candidates, mock.ANY, dict(), [], [], [], reason=reason))
       else:
         _, actually_rejected = pool.SubmitChanges(self.patches, reason=reason)
 
@@ -2027,17 +2028,17 @@
       f.flush()
       self.pool.AddPendingCommitsIntoPool(f.name)
 
-    self.assertEqual(self.pool.changes[0].owner_email, 'foo@chromium.org')
-    self.assertEqual(self.pool.changes[0].tracking_branch, 'master')
-    self.assertEqual(self.pool.changes[0].remote, 'cros')
-    self.assertEqual(self.pool.changes[0].gerrit_number, '17000')
-    self.assertEqual(self.pool.changes[0].project, 'chromiumos/taco/bar')
-    self.assertEqual(self.pool.changes[0].project_url,
+    self.assertEqual(self.pool.candidates[0].owner_email, 'foo@chromium.org')
+    self.assertEqual(self.pool.candidates[0].tracking_branch, 'master')
+    self.assertEqual(self.pool.candidates[0].remote, 'cros')
+    self.assertEqual(self.pool.candidates[0].gerrit_number, '17000')
+    self.assertEqual(self.pool.candidates[0].project, 'chromiumos/taco/bar')
+    self.assertEqual(self.pool.candidates[0].project_url,
                      'https://base_url/chromiumos/taco/bar')
-    self.assertEqual(self.pool.changes[0].change_id,
+    self.assertEqual(self.pool.candidates[0].change_id,
                      'Ieeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee1')
-    self.assertEqual(self.pool.changes[0].commit,
+    self.assertEqual(self.pool.candidates[0].commit,
                      '1ddddddddddddddddddddddddddddddddddddddd')
-    self.assertEqual(self.pool.changes[0].fail_count, 2)
-    self.assertEqual(self.pool.changes[0].pass_count, 0)
-    self.assertEqual(self.pool.changes[0].total_fail_count, 3)
+    self.assertEqual(self.pool.candidates[0].fail_count, 2)
+    self.assertEqual(self.pool.candidates[0].pass_count, 0)
+    self.assertEqual(self.pool.candidates[0].total_fail_count, 3)