Ensure host_forwarder is killed when setting up/tearing down sharding.

build/android/pylib/base_test_sharder.py

Index: build/android/pylib/base_test_sharder.py
diff --git a/build/android/pylib/base_test_sharder.py b/build/android/pylib/base_test_sharder.py
index fa05abec4e202b77d422fd2a29bcd5f20dfe996d..396d98d2b78244f92498599534aefbb911d3ca95 100644
--- a/build/android/pylib/base_test_sharder.py
+++ b/build/android/pylib/base_test_sharder.py
@@ -66,10 +66,13 @@ class BaseTestSharder(object):
 
   def SetupSharding(self, tests):
     """Called before starting the shards."""
-    Forwarder.KillHost(self.build_type)
+    pass
 
   def OnTestsCompleted(self, test_runners, test_results):
     """Notifies that we completed the tests."""
+    pass
+
+  def _KillHostForwarder(self):
     Forwarder.KillHost(self.build_type)
 
   def RunShardedTests(self):
@@ -85,61 +88,66 @@ class BaseTestSharder(object):
     logging.warning('Look for the "Final result" banner in the end.')
     logging.warning('*' * 80)
     final_results = TestResults()
-    for retry in xrange(self.retries):
-      logging.warning('Try %d of %d', retry + 1, self.retries)
-      self.SetupSharding(self.tests)
-      test_runners = []
-
-      # Try to create N shards, and retrying on failure.
-      try:
-        for index, device in enumerate(self.attached_devices):
-          logging.warning('*' * 80)
-          logging.warning('Creating shard %d for %s', index, device)
-          logging.warning('*' * 80)
-          test_runner = self.CreateShardedTestRunner(device, index)
-          test_runners += [test_runner]
-      except errors.DeviceUnresponsiveError as e:
-        logging.critical('****Failed to create a shard: [%s]', e)
-        self.attached_devices.remove(device)
-        continue
-
-      logging.warning('Starting...')
-      pool = multiprocessing.Pool(len(self.attached_devices),
-                                  SetTestsContainer,
-                                  [BaseTestSharder.tests_container])
-      # map can't handle KeyboardInterrupt exception. It's a python bug.
-      # So use map_async instead.
-      async_results = pool.map_async(_ShardedTestRunnable, test_runners)
-      try:
-        results_lists = async_results.get(999999)
-      except errors.DeviceUnresponsiveError as e:
-        logging.critical('****Failed to run test: [%s]', e)
-        self.attached_devices = android_commands.GetAttachedDevices()
-        continue
-      test_results = TestResults.FromTestResults(results_lists)
-      # Re-check the attached devices for some devices may
-      # become offline
-      retry_devices = set(android_commands.GetAttachedDevices())
-      # Remove devices that had exceptions.
-      retry_devices -= TestResults.DeviceExceptions(results_lists)
-      # Retry on devices that didn't have any exception.
-      self.attached_devices = list(retry_devices)
-      if (retry == self.retries - 1 or
-          len(self.attached_devices) == 0):
-        all_passed = final_results.ok + test_results.ok
-        final_results = test_results
-        final_results.ok = all_passed
-        break
-      else:
-        final_results.ok += test_results.ok
-        self.tests = []
-        for t in test_results.GetAllBroken():
-          self.tests += [t.name]
-        if not self.tests:
+    self._KillHostForwarder()
+    try:

[bulach, 2012/11/29 18:36:24]

hmm, this try..finally block is really long and makes it harder to read with the
other ones inside... since the only exception that can be thrown is on 148, at
which point all hope is lost, I'd suggest keeping as it was, and then just call
self._KillHostForwarder() at the end....

[Philippe, 2012/11/30 09:01:54]

Done. Yes I agree. It's not worth making the code more complicated given that
this wouldn't cover 100% of the cases.

+      for retry in xrange(self.retries):
+        logging.warning('Try %d of %d', retry + 1, self.retries)
+        self.SetupSharding(self.tests)
+        test_runners = []
+
+        # Try to create N shards, and retrying on failure.
+        try:
+          for index, device in enumerate(self.attached_devices):
+            logging.warning('*' * 80)
+            logging.warning('Creating shard %d for %s', index, device)
+            logging.warning('*' * 80)
+            test_runner = self.CreateShardedTestRunner(device, index)
+            test_runners += [test_runner]
+        except errors.DeviceUnresponsiveError as e:
+          logging.critical('****Failed to create a shard: [%s]', e)
+          self.attached_devices.remove(device)
+          continue
+
+        logging.warning('Starting...')
+        pool = multiprocessing.Pool(len(self.attached_devices),
+                                    SetTestsContainer,
+                                    [BaseTestSharder.tests_container])
+        # map can't handle KeyboardInterrupt exception. It's a python bug.
+        # So use map_async instead.
+        async_results = pool.map_async(_ShardedTestRunnable, test_runners)
+        try:
+          results_lists = async_results.get(999999)
+        except errors.DeviceUnresponsiveError as e:
+          logging.critical('****Failed to run test: [%s]', e)
+          self.attached_devices = android_commands.GetAttachedDevices()
+          continue
+        test_results = TestResults.FromTestResults(results_lists)
+        # Re-check the attached devices for some devices may
+        # become offline
+        retry_devices = set(android_commands.GetAttachedDevices())
+        # Remove devices that had exceptions.
+        retry_devices -= TestResults.DeviceExceptions(results_lists)
+        # Retry on devices that didn't have any exception.
+        self.attached_devices = list(retry_devices)
+        if (retry == self.retries - 1 or
+            len(self.attached_devices) == 0):
+          all_passed = final_results.ok + test_results.ok
+          final_results = test_results
+          final_results.ok = all_passed
           break
-    else:
-      # We ran out retries, possibly out of healthy devices.
-      # There's no recovery at this point.
-      raise Exception('Unrecoverable error while retrying test runs.')
-    self.OnTestsCompleted(test_runners, final_results)
+        else:
+          final_results.ok += test_results.ok
+          self.tests = []
+          for t in test_results.GetAllBroken():
+            self.tests += [t.name]
+          if not self.tests:
+            break
+      else:
+        # We ran out retries, possibly out of healthy devices.
+        # There's no recovery at this point.
+        raise Exception('Unrecoverable error while retrying test runs.')
+      self.OnTestsCompleted(test_runners, final_results)
+    finally:
+      self._KillHostForwarder()
+
     return final_results