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

build/android/pylib/base_test_sharder.py

 # Copyright (c) 2012 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 android_commands
+import atexit
 import logging
 import multiprocessing
 
 from android_commands import errors
 from forwarder import Forwarder
 from test_result import TestResults
 
 
 def _ShardedTestRunnable(test):
   """Standalone function needed by multiprocessing.Pool."""
@@ skipping 42 matching lines @@
       device: Device serial where this shard will run
       index: Index of this device in the pool.
 
     Returns:
       An object of BaseTestRunner type (that can provide a "Run()" method).
     """
     pass
 
   def SetupSharding(self, tests):
     """Called before starting the shards."""
-    Forwarder.KillHost(self.build_type)
 
   def OnTestsCompleted(self, test_runners, test_results):
     """Notifies that we completed the tests."""

[bulach, 2012/11/29 15:50:30]

add a "pass" here..

[Philippe, 2012/11/29 17:53:32]

Oops :)

+
+  def _KillHostForwarder(self):
     Forwarder.KillHost(self.build_type)
 
   def RunShardedTests(self):
     """Runs the tests in all connected devices.
 
     Returns:
       A TestResults object.
     """
     logging.warning('*' * 80)
     logging.warning('Sharding in ' + str(len(self.attached_devices)) +
                     ' devices.')
     logging.warning('Note that the output is not synchronized.')
     logging.warning('Look for the "Final result" banner in the end.')
     logging.warning('*' * 80)
     final_results = TestResults()
+    atexit.register(self._KillHostForwarder)

[bulach, 2012/11/29 15:50:30]

does this work? self._KillHostForwarder is a method, which requires "self" as a
first param... I've never used atexit before, but maybe this needs to pass
"self" as param..

registering atexits though, is pretty hairy... I'd much rather ensure that the
entry-point cleans up everything that may have been left behind, than trying to
cleanup via atexit... wdyt?

[Philippe, 2012/11/29 17:53:32]

This seems to work, it is one of the ways to pass a method and the instance it
is called on in Python (which I discovered recently to be honest).

Yeah I totally forgot that pylib was a library used outside the regular testing
infrastructure (e.g. telemetry).
We do kill the host_forwarder during setup so I guess I could just get rid of
the tear down step assuming that all the clients using the forwarder do kill
host_forwarder during setup as well (I will double check that).
But I would still be a bit more confident if the tear down happens. I added a
try/finally that should cover us for most of the cases (the others being when
python quits for a reason other than an exception).

+    self._KillHostForwarder()
     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)
@@ skipping 38 matching lines @@
         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)
     return final_results