Put cleanup steps in finally block so cleanup happens after halt on failure.

build/android/buildbot/bb_device_steps.py

 #!/usr/bin/env python
 # Copyright (c) 2013 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 collections
 import glob
 import multiprocessing
 import os
 import shutil
@@ skipping 302 matching lines @@
 
 
 def GenerateTestReport(options):
   bb_annotations.PrintNamedStep('test_report')
   for report in glob.glob(
       os.path.join(CHROME_SRC, 'out', options.target, 'test_logs', '*.log')):
     RunCmd(['cat', report])
     os.remove(report)
 
 
+def KillHostHeartbeat(_):
+  # PostTestStepCmds take an option argument, but the provision script doesn't
+  # need options to kill the host heartbeat.
+  provision_devices.KillHostHeartbeat()
+
+
 def GetPostTestStepCmds():
   return [
       ('logcat_dump', LogcatDump),
-      ('test_report', GenerateTestReport)
+      ('test_report', GenerateTestReport),
+      ('kill_host_heartbeat', KillHostHeartbeat)
   ]
 
 
 def MainTestWrapper(options):
-  # Spawn logcat monitor
-  SpawnLogcatMonitor()
+  try:
+    # Spawn logcat monitor
+    SpawnLogcatMonitor()
 
-  # Run all device setup steps
-  for _, cmd in GetDeviceSetupStepCmds():
-    cmd(options)
+    # Run all device setup steps
+    for _, cmd in GetDeviceSetupStepCmds():
+      cmd(options)
 
-  if options.install:
-    test_obj = INSTRUMENTATION_TESTS[options.install]
-    InstallApk(options, test_obj, print_step=True)
+    if options.install:
+      test_obj = INSTRUMENTATION_TESTS[options.install]
+      InstallApk(options, test_obj, print_step=True)
 
-  if options.test_filter:
-    bb_utils.RunSteps(options.test_filter, GetTestStepCmds(), options)
+    if options.test_filter:
+      bb_utils.RunSteps(options.test_filter, GetTestStepCmds(), options)
 
-  if options.experimental:
-    RunTestSuites(options, gtest_config.EXPERIMENTAL_TEST_SUITES)
-    RunBrowserTestSuite(options)
-
-  # Run all post test steps
-  for _, cmd in GetPostTestStepCmds():
-    cmd(options)
+    if options.experimental:
+      RunTestSuites(options, gtest_config.EXPERIMENTAL_TEST_SUITES)
+      RunBrowserTestSuite(options)
+  finally:

[Isaac (away), 2013/08/02 02:20:26]

Could we just call the functions here and remove GetPostTestStepCmds()?  Seems
unneeded.

[navabi, 2013/08/02 16:26:43]

Sure. That way we also don't need KillHostHeartbeat() function, since it only
exists to take an options argument and ignore it.

+    # Run all post test steps
+    for _, cmd in GetPostTestStepCmds():
+      cmd(options)
 
 
 def GetDeviceStepsOptParser():
   parser = bb_utils.GetParser()
   parser.add_option('--experimental', action='store_true',
                     help='Run experiemental tests')
   parser.add_option('-f', '--test-filter', metavar='<filter>', default=[],
                     action='append',
                     help=('Run a test suite. Test suites: "%s"' %
                           '", "'.join(VALID_TESTS)))
@@ skipping 23 matching lines @@
   if args:
     return sys.exit('Unused args %s' % args)
 
   unknown_tests = set(options.test_filter) - VALID_TESTS
   if unknown_tests:
     return sys.exit('Unknown tests %s' % list(unknown_tests))
 
   setattr(options, 'target', options.factory_properties.get('target', 'Debug'))
 
   MainTestWrapper(options)
-  provision_devices.KillHostHeartbeat()
 

[navabi, 2013/08/02 00:29:32]

This moved to GetPostTestStepCmds.

 
 if __name__ == '__main__':
   sys.exit(main(sys.argv))