Android: allows ignoring results of flaky sharded perf tests.

build/android/bb_run_sharded_steps.py

Index: build/android/bb_run_sharded_steps.py
diff --git a/build/android/bb_run_sharded_steps.py b/build/android/bb_run_sharded_steps.py
index 9010d774b71066871b9f488fa430ff5aff3a17f2..5c8d22787b07317b563751922ef83dde879853c6 100755
--- a/build/android/bb_run_sharded_steps.py
+++ b/build/android/bb_run_sharded_steps.py
@@ -11,9 +11,9 @@ 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.
 
@@ -23,10 +23,17 @@ 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.
@@ -72,8 +79,12 @@ def _RunStepsPerDevice(steps):
         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,
@@ -84,7 +95,7 @@ def _RunStepsPerDevice(steps):
   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):
@@ -101,6 +112,7 @@ def _RunShardedSteps(steps, devices):
     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]
@@ -157,6 +169,9 @@ def main(argv):
   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.')
@@ -181,7 +196,11 @@ def main(argv):
 
   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__':