Updating isolate scripts to read in chartjson telemetry results

testing/scripts/run_telemetry_benchmark_as_googletest.py

 #!/usr/bin/env python
 # Copyright 2015 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.
 
 """Runs an isolate bundled Telemetry benchmark.
 
 This script attempts to emulate the contract of gtest-style tests
 invoked via recipes. The main contract is that the caller passes the
 argument:
@@ skipping 44 matching lines @@
   # all the time on Linux.
   env[CHROME_SANDBOX_ENV] = CHROME_SANDBOX_PATH
   if args.xvfb and xvfb.should_start_xvfb(env):
     xvfb_proc, openbox_proc, xcompmgr_proc = xvfb.start_xvfb(env=env,
                                                              build_dir='.')
     assert xvfb_proc and openbox_proc and xcompmgr_proc, 'Failed to start xvfb'
   try:
     tempfile_dir = tempfile.mkdtemp('telemetry')
     valid = True
     failures = []
+    chartjson = False
+    if '--output-format=chartjson' in rest_args:
+      chartjson = True
+    chartresults = None
     try:
       rc = common.run_command([sys.executable] + rest_args + [
         '--output-dir', tempfile_dir,
         '--output-format=json'
       ], env=env)
       tempfile_name = os.path.join(tempfile_dir, 'results.json')
       with open(tempfile_name) as f:
         results = json.load(f)
       for value in results['per_page_values']:
         if value['type'] == 'failure':
           failures.append(results['pages'][str(value['page_id'])]['name'])
       valid = bool(rc == 0 or failures)
+      # If we have also output chartjson read it in and return it
+      if chartjson:
+        chart_tempfile_name = os.path.join(tempfile_dir, 'results-chart.json')
+        with open(chart_tempfile_name) as f:
+          chartresults = json.load(f)
     except Exception:
       traceback.print_exc()
       valid = False
     finally:
       shutil.rmtree(tempfile_dir)
 
     if not valid and not failures:
       failures = ['(entire test suite)']
       if rc == 0:
         rc = 1  # Signal an abnormal exit.
 
-    json.dump({
-        'valid': valid,
-        'failures': failures,
-    }, args.isolated_script_test_output)
+    if chartjson:
+      json.dump({
+          'valid': valid,
+          'failures': failures,
+          'chartjson': chartresults
+      }, args.isolated_script_test_output)
+    else:
+      json.dump({
+          'valid': valid,
+          'failures': failures
+      }, args.isolated_script_test_output)

[Ken Russell (switch to Gerrit), 2016/09/12 19:31:42]

Rather than overloading the meaning of --isolated-script-test-output, I think it
is a better direction to add a new command line argument
--isolated-script-chartjson-output taking a separate file to which the chartjson
results are written. This simplified JSON format is mainly used to display
results on the waterfall, but it's known that we are going to need to output
more "full" JSON results for the flakiness dashboard. It would be better to
start extending the contract between recipes and IsolatedScriptTest now. Do you
agree?

[eyaich1, 2016/09/13 11:53:41]

Good to know there is another use case coming down the pipe.  Adding another
output was one of my options, but I wasn't sure if I was still going to need the
regular json output or not.  My plan was to make the output just the chartjson
once I concluded that.  Therefore, I was going to iterate on a solution once I
got something up on the waterfall and figure out if I needed both from there.

I am open to going down this route first though, I don't think it was that much
more work, I just chose this route while I was still in investigation phase.  I
will ping this CL again when changes are made.

     return rc
 
   finally:
     xvfb.kill(xvfb_proc)
     xvfb.kill(openbox_proc)
     xvfb.kill(xcompmgr_proc)
 
 
 # This is not really a "script test" so does not need to manually add
 # any additional compile targets.
 def main_compile_targets(args):
   json.dump([], args.output)
 
 
 if __name__ == '__main__':
   # Conform minimally to the protocol defined by ScriptTest.
   if 'compile_targets' in sys.argv:
     funcs = {
       'run': None,
       'compile_targets': main_compile_targets,
     }
     sys.exit(common.run_script(sys.argv[1:], funcs))
   sys.exit(main())