Differentiate between tests that needs to be run or need to wait for completion.

verification/try_job_on_rietveld.py

Index: verification/try_job_on_rietveld.py
diff --git a/verification/try_job_on_rietveld.py b/verification/try_job_on_rietveld.py
index 704d443c7a15fb7b727af31d01749b59e5cf5f57..b6c66e6fcb8d6c5d1b1f81662ae9d6865ac4dafd 100644
--- a/verification/try_job_on_rietveld.py
+++ b/verification/try_job_on_rietveld.py
@@ -30,13 +30,28 @@ def is_job_expired(revision, timestamp, checkout):
   return False
 
 
+class RietveldTryJobPending(model.PersistentMixIn):
+  """Represents a pending try job for a pending commit that we care about.
+  """
+  persistent = [
+    'builder', 'revision', 'requested_steps',
+    'clobber', 'tries',
+  ]
+  def __init__(self, builder, revision, requested_steps, clobber, tries):
+    super(RietveldTryJobPending, self).__init__()
+    self.builder = builder
+    self.revision = revision
+    self.requested_steps = requested_steps
+    self.clobber = clobber
+    # Number of retries for this configuration. Initial try is 1.
+    self.tries = tries
+
+
 class RietveldTryJob(model.PersistentMixIn):
   """Represents a try job for a pending commit that we care about.
 
   This data can be regenerated by parsing all the try job names but it is a bit
   hard on the try server.
-
-  TODO(maruel): Should use __getstate__(), __setstate__() and __reduce__().
   """
   persistent = [
     'builder', 'build', 'revision', 'requested_steps', 'started',
@@ -51,6 +66,7 @@ class RietveldTryJob(model.PersistentMixIn):
     self.build = build
     self.revision = revision
     self.requested_steps = requested_steps
+    # The timestamp when the build started.
     self.started = started
     self.steps_passed = passed
     self.steps_failed = failed
@@ -102,11 +118,12 @@ class RietveldTryJobs(base.IVerifierStatus):
     self.skipped = False
     self.builders_and_tests = {}
     # Jobs that have been sent but are not found yet. Likely a builder is fully
-    # utilized or the try server hasn't polled Rietveld yet.
+    # utilized or the try server hasn't polled Rietveld yet. list of
+    # RietveldTryJobPending() instances.
     self.pendings = []
 
   def get_state(self):
-    if self.skipped or (not self.remaining() and not self.pendings):
+    if self.skipped or not self.tests_waiting_for_result():
       return base.SUCCEEDED
     if (self.pendings or
         not all(t.completed for t in self.try_jobs.itervalues())):
@@ -114,33 +131,50 @@ class RietveldTryJobs(base.IVerifierStatus):
     logging.debug('Not pending, all %d jobs completed' % len(self.try_jobs))
     return base.FAILED
 
-  def remaining(self):
-    """Returns what remains to be tested.
+  def tests_need_to_be_run(self):
+    """Returns which tests need to be run.
 
-    This excludes tests from any pending build so they are not retried
-    unnecessarily.
+    These are the tests that are not pending on any try job, either running or
+    in the pending list.
     """
     # What needs to be run.
     all_tests = dict(
         (builder, set(tests))
         for builder, tests in self.builders_and_tests.iteritems())
 
-    def clean(job):
-      if job.builder in all_tests:
-        all_tests[job.builder] -= set(job.steps_passed)
+    # Removes what was run and almost to be run, e.g. the build is running.
+    for try_job in self.try_jobs.itervalues():
+      if try_job.builder in all_tests:
+        all_tests[try_job.builder] -= set(try_job.steps_passed)
         # If it was requested but still not run, do not add it either. Only do
         # that if the job hasn't completed yet to catch issues like: a test was
         # requested on a builder but the test was not run because it is not in
         # the BuildFactory for the Builder.
-        if not job.completed:
-          to_be_run = set(job.requested_steps) - set(job.steps_failed)
-          all_tests[job.builder] -= to_be_run
+        if not try_job.completed:
+          to_be_run = set(try_job.requested_steps) - set(try_job.steps_failed)
+          all_tests[try_job.builder] -= to_be_run
+
+    # Removes what is queued to be run but hasn't started yet.
+    for try_job in self.pendings:
+      if try_job.builder in all_tests:
+        all_tests[try_job.builder] -= set(try_job.requested_steps)
+
+    return dict(
+        (builder, list(tests)) for builder, tests in all_tests.iteritems()
+        if tests)
+
+  def tests_waiting_for_result(self):
+    """Returns the tests that we are waiting for results on pending or running
+    builds.
+    """
+    all_tests = dict(
+        (builder, set(tests))
+        for builder, tests in self.builders_and_tests.iteritems())
 
     # Removes what was run.
     for try_job in self.try_jobs.itervalues():
-      clean(try_job)
-    for try_job in self.pendings:
-      clean(try_job)
+      if try_job.builder in all_tests:
+        all_tests[try_job.builder] -= set(try_job.steps_passed)
 
     return dict(
         (builder, list(tests)) for builder, tests in all_tests.iteritems()
@@ -215,6 +249,7 @@ class RietveldTryJobs(base.IVerifierStatus):
 
       passed = [s.name for s in build.steps if s.simplified_result]
       failed = [s.name for s in build.steps if s.simplified_result is False]
+      # The steps in neither passed or failed were skipped.
       logging.info(
           'Found Job success: %s/%d: %s',
           builder, buildnumber, ','.join(passed))
@@ -253,8 +288,9 @@ class TryRunnerRietveld(base.VerifierCheckout):
     1. Fetch result from try server.
     2. If try job was generated from rietveld;
       1. If not is_job_expired();
-        1. Strips from jobs to run for each succeeded steps
-        2. If no step remaining, mark job as succeeded.
+        1. Skip any scheduled test that succeeded on this builder.
+  2. For each builder with tests scheduled;
+    1. If no step waiting to be triggered, skip this builder completely.
   2. For each non succeeded job;
     1. Send try jobs to rietveld.
 
@@ -330,7 +366,7 @@ class TryRunnerRietveld(base.VerifierCheckout):
     if jobs.error_message:
       # Too late.
       return
-    remaining = jobs.remaining()
+    remaining = jobs.tests_need_to_be_run()
     if not remaining:
       return
     # Send them in order to simplify testing.
@@ -367,9 +403,7 @@ class TryRunnerRietveld(base.VerifierCheckout):
           pending.issue, pending.patchset, 'CQ', clobber, None,
           {builder: tests})
       jobs.pendings.append(
-          RietveldTryJob(
-            builder, None, None, tests, time.time(), [], [],
-            clobber, False, tries + 1))
+          RietveldTryJobPending(builder, None, tests, clobber, tries + 1))
       # Update the status on the AppEngine status to signal a new try job was
       # sent.
       info = {
@@ -405,7 +439,7 @@ class TryRunnerRietveld(base.VerifierCheckout):
     }
     self.send_status(pending, info)
 
-    remaining = jobs.remaining()
+    remaining = jobs.tests_need_to_be_run()
     retry = [
       step for step in job.steps_failed
       if step in remaining.get(job.builder, [])