Android: allows ignoring results of flaky sharded perf tests.

build/android/bb_run_sharded_steps.py

 #!/usr/bin/env python
 #
 # 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.
 
 """Helper script to shard build bot steps and save results to disk.
 
 Our buildbot infrastructure requires each slave to run steps serially.
 This is sub-optimal for android, where these steps can run independently on
 multiple connected devices.
 
 The buildbots will run this script multiple times per cycle:
-- First, without params: all steps will be executed in parallel using all
-connected devices. Step results will be pickled to disk (each step has a unique
-name).
+- First: all steps listed in -s in will be executed in parallel using all
+connected devices. Step results will be pickled to disk. Each step has a unique
+name. The result code will be ignored if the step name is listed in --flaky.
 The buildbot will treat this step as a regular step, and will not process any
 graph data.
 
 - Then, with -p STEP_NAME: at this stage, we'll simply print the file with the
 step results previously saved. The buildbot will then process the graph data
 accordingly.
 
 The JSON config contains is a file containing a dictionary in the format:

[Tom Hudson, 2013/02/26 16:42:36]

While you're in here, fix the grammar of the comment?

[bulach, 2013/02/26 17:14:16]

ouch! fixed..

 {
-  'step_name_foo': 'script_to_execute foo',
-  'step_name_bar': 'script_to_execute bar'
+  "step_name_foo": "script_to_execute foo",
+  "step_name_bar": "script_to_execute bar"
 }
 
+The JSON flaky file contains a list with step names which results should be
+ignored:
+[
+  "step_name_foo",
+  "step_name_bar"
+]
+
 Note that script_to_execute necessarily have to take at least the following
 options:
   --device: the serial number to be passed to all adb commands.
   --keep_test_server_ports: indicates it's being run as a shard, and shouldn't
   reset test server port allocation.
 """
 
 
 import datetime
 import json
@@ skipping 25 matching lines @@
   results = []
   for step in steps:
     start_time = datetime.datetime.now()
     print 'Starting %s: %s %s at %s' % (step['name'], step['cmd'],
                                         start_time, step['device'])
     output, exit_code  = pexpect.run(
         step['cmd'], cwd=os.path.abspath(constants.CHROME_DIR),
         withexitstatus=True, logfile=sys.stdout, timeout=1800,
         env=os.environ)
     end_time = datetime.datetime.now()
-    print 'Finished %s: %s %s at %s' % (step['name'], step['cmd'],
-                                        end_time, step['device'])
+    exit_msg = '%s %s' % (exit_code or 0,

[Sami, 2013/02/26 16:48:00]

Maybe we should have a line above this one:

    exit_code = exit_code or 0

so we don't need to repeat the condition.

[bulach, 2013/02/26 17:14:16]

Done.

+                          '(ignored, flaky)' if step['flaky'] else '')
+    print 'Finished %s: %s %s %s at %s' % (step['name'], exit_msg, step['cmd'],
+                                           end_time, step['device'])

[Tom Hudson, 2013/02/26 16:42:36]

You're changing the format of the output string here; I assume we don't have any
scripts scraping it.

[bulach, 2013/02/26 17:14:16]

that's right, this part is purely informational.

+    if step['flaky']:
+      exit_code = 0
     result = {'name': step['name'],
               'output': output,
               'exit_code': exit_code or 0,
               'total_time': (end_time - start_time).seconds,
               'device': step['device']}
     _SaveResult(result)
     results += [result]
   return results
 
 
-def _RunShardedSteps(steps, devices):
+def _RunShardedSteps(steps, devices, flaky):

[Sami, 2013/02/26 16:48:00]

Call this something like flaky_steps to make it clearer?

[bulach, 2013/02/26 17:14:16]

that looks better, thanks! fixed, and also changed the order so it's "steps,
flaky_steps, devices"... renamed everywhere.

   assert steps
   assert devices, 'No devices connected?'
   if os.path.exists(_OUTPUT_DIR):
     assert '/step_results' in _OUTPUT_DIR
     shutil.rmtree(_OUTPUT_DIR)
   if not os.path.exists(_OUTPUT_DIR):
     os.makedirs(_OUTPUT_DIR)
   step_names = sorted(steps.keys())
   all_params = []
   num_devices = len(devices)
   shard_size = (len(steps) + num_devices - 1) / num_devices
   for i, device in enumerate(devices):
     steps_per_device = []
     for s in steps.keys()[i * shard_size:(i + 1) * shard_size]:
       steps_per_device += [{'name': s,
                             'device': device,
+                            'flaky': s in flaky,
                             'cmd': steps[s] + ' --device ' + device +
                             ' --keep_test_server_ports'}]
     all_params += [steps_per_device]
   print 'Start sharding (note: output is not synchronized...)'
   print '*' * 80
   start_time = datetime.datetime.now()
   pool = multiprocessing.Pool(processes=num_devices)
   async_results = pool.map_async(_RunStepsPerDevice, all_params)
   results_per_device = async_results.get(999999)
   end_time = datetime.datetime.now()
@@ skipping 36 matching lines @@
           os.kill(int(pid), signal.SIGQUIT)
         except Exception as e:
           logging.warning('Failed killing %s %s %s', server, pid, e)
 
 
 def main(argv):
   parser = optparse.OptionParser()
   parser.add_option('-s', '--steps',
                     help='A JSON file containing all the steps to be '
                          'sharded.')
+  parser.add_option('--flaky',
+                    help='A JSON file containing steps that are flaky and '
+                         'will have its exit code ignored.')
   parser.add_option('-p', '--print_results',
                     help='Only prints the results for the previously '
                          'executed step, do not run it again.')
   options, urls = parser.parse_args(argv)
   if options.print_results:
     return _PrintStepOutput(options.print_results)
 
   # At this point, we should kill everything that may have been left over from
   # previous runs.
   _KillPendingServers()
 
   # Reset the test port allocation. It's important to do it before starting
   # to dispatch any step.
   if not ports.ResetTestServerPortAllocation():
     raise Exception('Failed to reset test server port.')
 
   # Sort the devices so that we'll try to always run a step in the same device.
   devices = sorted(android_commands.GetAttachedDevices())
   if not devices:
     print 'You must attach a device'
     return 1
 
   with file(options.steps, 'r') as f:
     steps = json.load(f)
-  return _RunShardedSteps(steps, devices)
+  flaky = []
+  if options.flaky:
+    with file(options.flaky, 'r') as f:
+      flaky = json.load(f)

[Sami, 2013/02/26 16:48:00]

Sort super duper bonus points should we sort the steps so that flaky tests
always run last? That way the bot would show feedback sooner. Or will changing
the order break something?

[bulach, 2013/02/26 17:14:16]

hmmm... that wouldn't make much difference in terms of getting feedback,
unfortunately... :-/

the way it works, *all* of the tests run as a single step first, persist the
result to disk, then once they are all done, the buildbot will print each one of
these results...

sure, the "single" step would have the stdout sooner, but that is fairly
unreadable as it's not synchronized across the shards... (the _second_ step
guarantees that each step will be printed sequentially)....

leave as is?

+  return _RunShardedSteps(steps, devices, flaky)
 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv))