Add automatic retry mechanism and LKGR support.

tests/try_server_test.py

Index: tests/try_server_test.py
diff --git a/tests/try_server_test.py b/tests/try_server_test.py
index e7e974934328f12a8b3fd1bda7ec2d08098ac9d6..64cd19f18aca55f8aa36edc3aca6bbc96fe06a2b 100755
--- a/tests/try_server_test.py
+++ b/tests/try_server_test.py
@@ -194,6 +194,14 @@ class FakeTryServer(object):
     data['results'] = [result]
     self._builds[builder].append(data)
 
+  def step_result(self, builder, name, result):

[Dirk Pranke, 2011/06/08 22:49:49]

maybe rename this to 'set_step_result' if you renamed the other to
'set_build_result' in the last change? The names should be similar, at least.

+    """Sets step result on builder's last build."""
+    for step in self._builds[builder][-1]['steps']:
+      if name == step['name']:
+        step['results'] = result
+        return
+    self.fail()
+
   def build_result(self, builder, result):
     for step in self._builds[builder][-1]['steps']:
       step['results'] = [result]
@@ -206,6 +214,8 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
     self.email = 'user1@example.com'
     self.user = 'user1'
     self.timestamp = [1]
+    # Mocks http://chromium-status.appspot.com/lkgr
+    self.lkgr = 123
     self.try_server = FakeTryServer(self)
     self.mock(time, 'time', lambda: self.timestamp[-1])
 
@@ -215,7 +225,8 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
         self.email,
         ('linux', 'mac'),
         ('test1', 'test2'),
-        ['extra_flags',])
+        ['extra_flags',],
+        lambda: self.lkgr)
     self.pending = pending_manager.PendingCommit(
         42, 'owner@example.com', [], 23, None, 'lame desc', [],
         self.try_server.server_url)
@@ -259,12 +270,108 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
   def testVoidUpdate(self):
     self.try_runner.update_status([])
 
+  def test_steps_quality(self):
+    self.assertEquals(None, try_server.steps_quality([]))
+    self.assertEquals(True, try_server.steps_quality([True, None]))
+    self.assertEquals(False, try_server.steps_quality([True, None, False]))
+
+  def testStepQualityNone(self):
+    self.try_runner.status.builders['linux'].builds.cache()
+    self.assertEquals(
+        (None, 0),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+
+  def testStepQualityGood(self):
+    self.try_server.add_build(
+        'linux', 123, None, [SUCCESS, None, None, None])
+    self.try_runner.status.builders['linux'].builds.cache()
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+    self.assertEquals(
+        ([True, None, None, None], 1),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+    self.try_server.build_result('linux', SUCCESS)
+    self.try_runner.status.builders['linux'].builds.refresh()
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+
+  def testStepQualityBad(self):
+    self.try_server.add_build(
+        'linux', 123, None, [SUCCESS, SUCCESS, FAILURE, SUCCESS])
+    self.try_runner.status.builders['linux'].builds.cache()
+    # Also test that FakeTryServer.add_build() is implemented correctly.
+    self.assertEquals(
+        ([True, True, False, True], 1),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+
+  def testStepQualityBadIncomplete(self):
+    self.try_server.add_build(
+        'linux', 123, None, [SUCCESS, SUCCESS, FAILURE, None])
+    self.try_runner.status.builders['linux'].builds.cache()
+    # Also test that FakeTryServer.add_build() is implemented correctly.
+    self.assertEquals(
+        ([True, True, False, None], 1),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+
+  def testStepQualityGoodAndBad(self):
+    self.try_server.add_build(
+        'linux', 123, None, [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.try_server.add_build('linux', 123, None, [FAILURE, None, None, None])
+    self.try_runner.status.builders['linux'].builds.cache()
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+    self.assertEquals(
+        ([True] * 4, 2),
+        self.try_runner.step_db.revision_quality_builder_steps('linux', 123))
+
+  def testQualityAutomatic(self):
+    self.try_runner.verify(self.pending, '123')
+    self.assertEquals(
+        (None, 0),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'linux', 123))
+    self.try_server.add_build(
+        'linux', 123, 'georges tried stuff',
+        [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.try_runner.update_status([self.pending])
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'linux', 123))
+    self.try_server.check_calls(
+        ['trychange b=linux,mac c=False r=123 t=test1,test2'] +
+        ['builders/?select=linux&select=mac'] +
+        ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
+
+  def testQualityManual(self):
+    self.try_server.add_build(
+        'linux', 123, 'georges tried stuff',
+        [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.try_runner.status.builders['linux'].builds.cache()
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'linux', 123))
+    self.try_server.check_calls(['builders/linux/builds/_all'])
+
   def _simple(self, status_linux, status_mac=None, error_msg=None):
     """status_linux affects test1, status_mac affects test2."""
     def is_failure(status):
       return status in (FAILURE, EXCEPTION)
+
+    self.assertEquals(
+        bool(is_failure(status_linux) or is_failure(status_mac)),
+        bool(error_msg))
     if status_mac is None:
       status_mac = status_linux
+    self.lkgr = 12
+    self.try_server.add_build(
+        'linux', 123, None, [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.try_server.add_build(
+        'mac', 123, None, [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
 
     self.try_runner.verify(self.pending, '123')
     self.try_server.check_calls(
@@ -273,32 +380,50 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
         'linux', 123, None, [SUCCESS, SUCCESS, status_linux, SUCCESS])
     self.try_server.add_build(
         'mac', 123, None, [SUCCESS, SUCCESS, SUCCESS, status_mac])
+
     self.try_runner.update_status([self.pending])
+    if is_failure(status_linux):
+      self.assertEquals(123, self.try_runner.get_lkgr('linux'))
+    else:
+      self.assertEquals(123, self.try_runner.get_lkgr('linux'))
+    if is_failure(status_mac):
+      self.assertEquals(123, self.try_runner.get_lkgr('mac'))
+    else:
+      self.assertEquals(123, self.try_runner.get_lkgr('mac'))
 
     if error_msg:
+      # Can't test failure without testing automatic retry mechanism.
+      self.assertPending(base.PROCESSING, 2, None)
+      expected = (
+          ['builders/?select=linux&select=mac'] +
+          ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
+      if is_failure(status_linux):
+        expected.append('trychange b=linux c=False r=123 t=test1')
+      if is_failure(status_mac):
+        expected.append('trychange b=mac c=False r=123 t=test2')
+      self.try_server.check_calls(expected)
+      self.try_server.add_build(
+          'linux', 123, None, [SUCCESS, SUCCESS, status_linux, SUCCESS])
+      self.try_server.add_build(
+          'mac', 123, None, [SUCCESS, SUCCESS, SUCCESS, status_mac])
+
+      self.try_runner.update_status([self.pending])
       self.assertPending(base.FAILED, 2, error_msg)
+      if is_failure(status_linux) and is_failure(status_mac):
+        self.try_server.check_calls(
+            ['builders/?select=linux&select=mac'] +
+            ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
+      elif is_failure(status_linux):
+        self.try_server.check_calls(
+            ['builders/?select=linux', 'builders/linux/builds/_all'])
+      else:
+        self.try_server.check_calls(
+            ['builders/?select=mac', 'builders/mac/builds/_all'])
     else:
       self.assertPending(base.SUCCEEDED, 2, None)
-
-    # Verify good_revisions() and bad_revisions()
-    if is_failure(status_linux) and is_failure(status_mac):
-      self.assertEquals([], list(self.try_runner.step_db.good_revisions()))
-      self.assertEquals([123], list(self.try_runner.step_db.bad_revisions()))
-    else:
-      # It passed on at least one builder.
-      self.assertEquals([123], list(self.try_runner.step_db.good_revisions()))
-      self.assertEquals([], list(self.try_runner.step_db.bad_revisions()))
-
-    # Verify revision_quality_builder.
-    self.assertEquals(
-        not is_failure(status_linux),
-        self.try_runner.step_db.revision_quality_builder('linux', 123))
-    self.assertEquals(
-        not is_failure(status_mac),
-        self.try_runner.step_db.revision_quality_builder('mac', 123))
-    self.try_server.check_calls(
-        ['builders/?select=linux&select=mac'] +
-        ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
+      self.try_server.check_calls(
+          ['builders/?select=linux&select=mac'] +
+          ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
 
   def testImmediateSuccess(self):
     self._simple(SUCCESS)
@@ -325,31 +450,29 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
   def testImmediateSkipped(self):
     self._simple(SKIPPED)
 
-  def testImmediateFailureLinux(self):
-    self._simple(
-        FAILURE, SUCCESS,
-        'Try job failure for 42-23 on linux for step test1:\n'
-        '%s/buildstatus?builder=linux&number=0' % self.try_server.server_url)
-
-  def testImmediateFailureMac(self):
+  def testImmediateFailure(self):
     self._simple(
         SUCCESS, FAILURE,
-        'Try job failure for 42-23 on mac for step test2:\n'
-        '%s/buildstatus?builder=mac&number=0' % self.try_server.server_url)
+        ('Try job failure for 42-23 on mac for step "test2".\n'
+        'It\'s a second try, previously, step "test2" failed.\n'
+        '%s/buildstatus?builder=mac&number=2') % self.try_server.server_url)
 
   def testImmediateDoubleFailure(self):
     self._simple(
         FAILURE, FAILURE,
-        'Try job failure for 42-23 on mac for step test2:\n'
-        '%s/buildstatus?builder=mac&number=0' % self.try_server.server_url)
+        'Try job failure for 42-23 on mac for step "test2".\n'
+        'It\'s a second try, previously, step "test2" failed.\n'
+        '%s/buildstatus?builder=mac&number=2' % self.try_server.server_url)
 
   def testImmediateException(self):
     self._simple(
         SUCCESS, EXCEPTION,
-        'Try job failure for 42-23 on mac for step test2:\n'
-        '%s/buildstatus?builder=mac&number=0' % self.try_server.server_url)
+        ('Try job failure for 42-23 on mac for step "test2".\n'
+        'It\'s a second try, previously, step "test2" failed.\n'
+        '%s/buildstatus?builder=mac&number=2') % self.try_server.server_url)
 
   def testSuccess(self):
+    self.lkgr = 2
     # Normal workflow with incremental success.
     self.try_runner.verify(self.pending, '123')
     self.try_server.check_calls(
@@ -387,8 +510,14 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
     self.try_server.build_result('linux', SUCCESS)
     self.try_runner.update_status([self.pending])
     self.assertPending(base.SUCCEEDED, 2, None)
-    self.assertEquals([123], list(self.try_runner.step_db.good_revisions()))
-    self.assertEquals([], list(self.try_runner.step_db.bad_revisions()))
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'linux', 123))
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'mac', 123))
     self.try_server.check_calls(['builders/linux/builds/?select=0'])
 
   def testIgnorePreviousJobs(self):
@@ -413,8 +542,14 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
     self.try_server.add_build('mac', 123, None, [SUCCESS] * 4)
     self.try_runner.update_status([self.pending])
     self.assertPending(base.SUCCEEDED, 2, None)
-    self.assertEquals([12, 123], list(self.try_runner.step_db.good_revisions()))
-    self.assertEquals([], list(self.try_runner.step_db.bad_revisions()))
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'linux', 123))
+    self.assertEquals(
+        ([True] * 4, 1),
+        self.try_runner.step_db.revision_quality_builder_steps(
+          'mac', 123))
     self.try_server.check_calls(
         ['builders/?select=linux&select=mac',
           'builders/linux/builds/_all', 'builders/mac/builds/_all'])
@@ -525,24 +660,34 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
 
     self.try_runner.update_status([self.pending])
 
-  def testFailedSteps(self):
-    # Verifies failed steps are listed correctly. It will be necessary for bad
-    # revision and flaky test detections.
+  def testFailedStepRetryLkgr(self):
     self.try_runner.verify(self.pending, '123')
     self.try_server.check_calls(
         ['trychange b=linux,mac c=False r=123 t=test1,test2'])
     self.try_server.add_build(
-        'linux', 123, None, [SUCCESS, FAILURE, SUCCESS, FAILURE])
+        'linux', 123, None, [SUCCESS, SUCCESS, FAILURE, SUCCESS])
+
+    self.lkgr = 122
     self.try_runner.update_status([self.pending])
     self.try_server.check_calls(
         ['builders/?select=linux&select=mac'] +
-        ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
-    self.assertEquals(
-        ['compile', 'test2'],
-        self.get_verif().try_jobs[0].failed_steps)
+        ['builders/linux/builds/_all', 'builders/mac/builds/_all'] +
+        # Only the failed test is retried, on lkgr revision.
+        ['trychange b=linux c=False r=122 t=test1'])
+    self.assertEquals(['test1'], self.get_verif().try_jobs[0].failed_steps)
 
   def testFailedUpdate(self):
     # It must not retry a failed update.
+
+    # Add succeededing builds, this sets quality to True, which disable retry
+    # mechanism.
+    self.try_server.add_build(
+        'linux', 123, 'georges tried stuff',
+        [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.try_server.add_build(
+        'mac', 123, 'georges tried stuff', [SUCCESS, SUCCESS, SUCCESS, SUCCESS])
+    self.lkgr = 123
+
     self.try_runner.verify(self.pending, '123')
     self.try_server.check_calls(
         ['trychange b=linux,mac c=False r=123 t=test1,test2'])
@@ -552,11 +697,15 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
     self.try_server.check_calls(
         ['builders/?select=linux&select=mac'] +
         ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
+    self.assertEquals('linux', self.get_verif().try_jobs[0].builder)
     self.assertEquals(['update'], self.get_verif().try_jobs[0].failed_steps)
     self.assertPending(
         base.FAILED, 2,
-        u'Try job failure for 42-23 on linux for step update:\n'
-        u'%s/buildstatus?builder=linux&number=0' % self.try_server.server_url)
+        (u'Try job failure for 42-23 on linux for step '
+        u'"update".\n%s/buildstatus?builder=linux&number=1\n\n'
+        u'Step "update" is always a major failure.\n'
+        u'Look at the try server FAQ for more details.') %
+            self.try_server.server_url)
 
   def testFailedCompileRetryClobber(self):
     # It must retry once a non-clobber compile.
@@ -565,23 +714,25 @@ class TryServerTest(TestCase, auto_stub.AutoStubMixIn):
         ['trychange b=linux,mac c=False r=123 t=test1,test2'])
     self.try_server.add_build(
         'linux', 123, None, [SUCCESS, FAILURE, None, None])
+    self.lkgr = 122
     self.try_runner.update_status([self.pending])
     self.try_server.check_calls(
         ['builders/?select=linux&select=mac'] +
         ['builders/linux/builds/_all', 'builders/mac/builds/_all'] +
-        ['trychange b=linux c=True r=123 t=test1,test2'])
+        ['trychange b=linux c=True r=122 t=test1,test2'])
     self.assertEquals(['compile'], self.get_verif().try_jobs[0].failed_steps)
     self.assertPending(base.PROCESSING, 2, None)
 
     self.try_server.add_build(
-        'linux', 123, None, [SUCCESS, FAILURE, FAILURE, None])
+        'linux', 122, None, [SUCCESS, FAILURE, None, None])
     self.try_runner.update_status([self.pending])
     self.try_server.check_calls(
         ['builders/?select=linux&select=mac'] +
         ['builders/linux/builds/_all', 'builders/mac/builds/_all'])
     self.assertPending(
         base.FAILED, 2,
-        u'Try job failure for 42-23 on linux for steps compile, test1:\n'
+        u'Try job failure for 42-23 on linux for step "compile" (clobber build)'
+        u'.\nIt\'s a second try, previously, step "compile" failed.\n'
         u'%s/buildstatus?builder=linux&number=1' % self.try_server.server_url)