[telemetry] Add Html2OutputFormatter for generating results2.html

telemetry/telemetry/internal/results/html2_output_formatter.py

+# Copyright 2016 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 datetime
+import json
+import logging
+import os
+
+from catapult_base import cloud_storage  # pylint: disable=import-error

[petrcermak, 2016/05/18 10:43:20]

Why does this trigger an import error?

[benjhayden, 2016/05/18 18:23:33]

Copy-pasta. presubmit seems ok without it.

+
+from telemetry.internal.results import output_formatter
+
+import tracing_project
+
+from py_vulcanize import generate
+
+
+class Html2OutputFormatter(output_formatter.OutputFormatter):
+  def __init__(self, output_stream, reset_results, upload_results):
+    super(Html2OutputFormatter, self).__init__(output_stream)
+    self._upload_results = upload_results
+    if reset_results or not output_stream.read():
+      output_stream.write(self._GetHtmlTemplate())
+      output_stream.close()
+    self._output_filename = output_stream.name

[petrcermak, 2016/05/18 10:43:20]

Is there any point in storing this when you can access it via
self._output_stream.name? I would suggest adding a getter instead.

[benjhayden, 2016/05/18 18:23:33]

This is a bit of a work-around for how this output_stream is constructed.
This could be simplified if the existing HtmlOutputFormatter is removed.

+
+  def _GetHtmlTemplate(self):
+    project = tracing_project.TracingProject()
+    vulcanizer = project.CreateVulcanizer()
+    # TODO move results2html to telemetry and use source paths magic from

[petrcermak, 2016/05/18 10:43:20]

nit: should be "TODO(username):" or at least "TODO: "

[benjhayden, 2016/05/18 18:23:33]

I'm not sure who will do this or when.

[eakuefner, 2016/05/18 19:34:48]

In this case I'd usually do TODO(#bug).

[benjhayden, 2016/05/18 20:03:55]

Done.

+    # perf_insights_project

[petrcermak, 2016/05/18 10:43:20]

nit: Add a period

[benjhayden, 2016/05/18 18:23:33]

Done.

+    modules = ['tracing.results2html']
+    load_sequence = vulcanizer.CalcLoadSequenceForModuleNames(modules)
+    return generate.GenerateStandaloneHTMLAsString(load_sequence)
+
+  def Format(self, page_test_results):
+    with file(self._output_filename, 'a') as htmlfp:

[petrcermak, 2016/05/18 10:43:20]

out of curiosity, what does "htmlfp" abbreviate? Why not just use "html_file" or
simply "f"?

[benjhayden, 2016/05/18 18:23:33]

"file pointer" from C, but "f" sgtm.

+      htmlfp.write('\n<script>\nvalues.addValueDicts(' +

[petrcermak, 2016/05/18 10:43:20]

the following might be more readable:

htmlfp.write('\n'.join([
    '',
    '<script>',
    'values.addValueDicts(%s);' % json.dumps(...),
    '</script>',
    ''
]))

Alternatively, I would at least suggest using %s and %.

[benjhayden, 2016/05/18 18:23:33]

Done.

+                   json.dumps(page_test_results.value_set) +
+                   ');\n</script>\n')
+
+    if self._upload_results:
+      file_path = os.path.abspath(self._output_stream.name)
+      file_name = 'html-results/results-%s' % datetime.datetime.now().strftime(
+          '%Y-%m-%d_%H-%M-%S')
+      try:
+        cloud_storage.Insert(cloud_storage.PUBLIC_BUCKET, file_name, file_path)
+        print
+        print ('View online at '
+               'http://storage.googleapis.com/chromium-telemetry/%s'
+               % file_name)
+      except cloud_storage.PermissionError as e:
+        logging.error('Cannot upload profiling files to cloud storage due to '
+                      ' permission error: %s' % e.message)
+    print
+    print 'View result at file://%s' % os.path.abspath(
+        self._output_stream.name)