Move Html and BuildbotPageMeasurementResults to {Html,Buildbot}OutputFormatter.

Move Html and BuildbotPageMeasurementResults to {Html,Buildbot}OutputFormatter.

This preserved the current output of each
--output-format=buildbot|html. We may want to simplify the output for
--output-format=html.

Refactored results_options.py slightly to better accommodate
OutputFormatter (now --output-format=none is also using
OutputFormatter code path).

BUG=346956
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286445

[nednguyen, 2014-07-29 23:53:52]

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/buildbot_output_formatter.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/buildbot_output_formatter.py:21:
self.output_stream.flush()
Why is the output_stream public?

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/value/summary_unittest.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/value/summary_unittest.py:4: 
I think this file can be removed.

[nednguyen, 2014-07-29 23:55:16]

On 2014/07/29 23:53:52, nednguyen wrote:
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/results/buildbot_output_formatter.py (right):
> 
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/results/buildbot_output_formatter.py:21:
> self.output_stream.flush()
> Why is the output_stream public?
> 
nvm, please make it "immutable" with @property method defined in
output_formatter.py

>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/value/summary_unittest.py (right):
> 
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/value/summary_unittest.py:4: 
> I think this file can be removed.

[nednguyen, 2014-07-29 23:56:43]

lgtm

[nednguyen, 2014-07-30 00:02:54]

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/results_options.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/results_options.py:55: # Maybe we should have
--output-dir instead of --output-file?
how about --output-format be able to take in a comma-delimited of format choice?

Then we can output: results.html, results.json, results.csv. 

The other two gtest & buildbot are streaming to stdout for now.

Default options is probably "html,buildbot".

[chrishenry, 2014-07-30 00:37:21]

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/buildbot_output_formatter.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/buildbot_output_formatter.py:21:
self.output_stream.flush()
On 2014/07/29 23:53:52, nednguyen wrote:
> Why is the output_stream public?

Acknowledged.

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/results_options.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/results_options.py:55: # Maybe we should have
--output-dir instead of --output-file?
On 2014/07/30 00:02:54, nednguyen wrote:
> how about --output-format be able to take in a comma-delimited of format
choice?
> 
> Then we can output: results.html, results.json, results.csv. 
> 
> The other two gtest & buildbot are streaming to stdout for now.

Yeah, that's a good alternative. Was thinking of --output-dir since we also want
to output other files in the future (trace, etc), so specifying a single
--output-dir may be better.

> 
> Default options is probably "html,buildbot".

I think the fact that html also output buildbot format is accidental. So let's
not expose that. I think I want to make progress reporter reports the values and
remove BuildbotOF from the "html" code path.

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/value/summary_unittest.py (right):

https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/value/summary_unittest.py:4: 
On 2014/07/29 23:53:52, nednguyen wrote:
> I think this file can be removed.

Let's keep it so that there is a pointer from here to buildbot_output_formatter,
so that people don't accidentally write more unit tests that duplicate those in
buildbot_output_formatter (I dislike repeating tests twice -- harder to fix when
broken).

[chrishenry, 2014-07-30 00:38:21]

On 2014/07/29 23:55:16, nednguyen wrote:
> On 2014/07/29 23:53:52, nednguyen wrote:
> >
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> > File tools/telemetry/telemetry/results/buildbot_output_formatter.py (right):
> > 
> >
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/results/buildbot_output_formatter.py:21:
> > self.output_stream.flush()
> > Why is the output_stream public?
> > 
> nvm, please make it "immutable" with @property method defined in
> output_formatter.py

Done.

> 
> >
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> > File tools/telemetry/telemetry/value/summary_unittest.py (right):
> > 
> >
>
https://codereview.chromium.org/421503005/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/value/summary_unittest.py:4: 
> > I think this file can be removed.

[chrishenry, 2014-07-30 00:38:49]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-07-30 00:41:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishenry@google.com/421503005/60001

[tonyg, 2014-07-30 01:35:30]

belated lgtm

[chrishenry, 2014-07-30 02:57:11]

The CQ bit was unchecked by chrishenry@google.com

[chrishenry, 2014-07-30 02:57:14]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-07-30 02:57:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishenry@google.com/421503005/60001

[commit-bot: I haz the power, 2014-07-30 08:33:11]

Change committed as 286445

[tonyg, 2014-07-30 16:07:13]

A revert of this CL has been created in
https://codereview.chromium.org/423383004/ by tonyg@chromium.org.

The reason for reverting is: [ RUN      ]
page_runner_unittest.PageRunnerTests.testPagesetRepeat
WARNING:root:Could not find Flash at
/mnt/data/b/build/slave/Linux_Tests/build/src/third_party/adobe/flash/binaries/ppapi/linux/libpepflashplayer.so.
Continuing without Flash.
To run with Flash, check it out via http://go/read-src-internal

Traceback (most recent call last):
  run at /usr/lib/python2.7/unittest/case.py:327
    testMethod()
  wrapper at tools/telemetry/telemetry/decorators.py:55
    func(*args, **kwargs)
  testPagesetRepeat at
tools/telemetry/telemetry/page/page_runner_unittest.py:241
    results._output_stream.close()  # pylint: disable=W0212
AttributeError: 'NoneType' object has no attribute 'close'

Locals:
  Measurement  : <class 'telemetry.page.page_runner_unittest.Measurement'>
  expectations : <telemetry.page.test_expectations.TestExpectations object at
0x315f610>
  f            : <closed file '/tmp/tmpJPzPHo', mode 'r' at 0x2b549c0>
  options      : [('android_device', None), ('android_rndis', False),
('browser_executable', None), ('browser_options', [('_extra_browser_args',
set([])), ('browser_type', 'release'), ('browser_user_agent_type', None),
('clear_sytem_cache_for_browser_and_profile_on_start', False),
('disable_component_extensions_with_background_pages', True),
('dont_override_profile', False), ('extra_wpr_args', []), ('netsim', None),
('no_proxy_server', False), ('profile_dir', None), ('profile_type', 'clean'),
('show_stdout', False), ('st ... _order_file', None), ('positional_args', []),
('print_bootstrap_deps', None), ('profile_dir', None), ('profile_type',
'clean'), ('profiler', None), ('repeat_count', 1), ('reset_results', None),
('results_label', None), ('run_disabled_tests', False), ('show_stdout', None),
('skip_navigate_on_repeat', False), ('synthetic_gesture_source_type',
'default'), ('upload_results', None), ('use_devtools_active_port', None),
('use_live_sites', None), ('verbosity', 0), ('write_full_results_to',
'/tmp/tmpjhD2qP.json')] (truncated)
  output_file  : '/tmp/tmpJPzPHo'
  ps           : <telemetry.page.page_set.PageSet object at 0x315f250>
  results      : <telemetry.results.page_test_results.PageTestResults object at
0x315f0d0>
  stdout       : 'RESULT metric: blank.html= [1,3] unit\nAvg metric:
2.000000unit\nSd  metric: 1.414214unit\nRESULT metric: green_rect.html= [2,4]
unit\nAvg metric: 3.000000unit\nSd  metric: 1.414214unit\n*RESULT metric:
metric= [1,2,3,4] unit\nAvg metric: 2.500000unit\nSd  metric:
1.290994unit\nRESULT telemetry_page_measurement_results: num_failed= 0
count\nRESULT telemetry_page_measurement_results: num_errored= 0 count\n'

Pages: [blank.html,green_rect.html]
[  FAILED  ] page_runner_unittest.PageRunnerTests.testPagesetRepeat (1883 ms).

[chrishenry, 2014-07-30 16:13:12]

On 2014/07/30 16:07:13, tonyg wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/423383004/ by mailto:tonyg@chromium.org.
> 
> The reason for reverting is: [ RUN      ]
> page_runner_unittest.PageRunnerTests.testPagesetRepeat
> WARNING:root:Could not find Flash at
>
/mnt/data/b/build/slave/Linux_Tests/build/src/third_party/adobe/flash/binaries/ppapi/linux/libpepflashplayer.so.
> Continuing without Flash.
> To run with Flash, check it out via http://go/read-src-internal
> 
> Traceback (most recent call last):
>   run at /usr/lib/python2.7/unittest/case.py:327
>     testMethod()
>   wrapper at tools/telemetry/telemetry/decorators.py:55
>     func(*args, **kwargs)
>   testPagesetRepeat at
> tools/telemetry/telemetry/page/page_runner_unittest.py:241
>     results._output_stream.close()  # pylint: disable=W0212
> AttributeError: 'NoneType' object has no attribute 'close'
> 
> Locals:
>   Measurement  : <class 'telemetry.page.page_runner_unittest.Measurement'>
>   expectations : <telemetry.page.test_expectations.TestExpectations object at
> 0x315f610>
>   f            : <closed file '/tmp/tmpJPzPHo', mode 'r' at 0x2b549c0>
>   options      : [('android_device', None), ('android_rndis', False),
> ('browser_executable', None), ('browser_options', [('_extra_browser_args',
> set([])), ('browser_type', 'release'), ('browser_user_agent_type', None),
> ('clear_sytem_cache_for_browser_and_profile_on_start', False),
> ('disable_component_extensions_with_background_pages', True),
> ('dont_override_profile', False), ('extra_wpr_args', []), ('netsim', None),
> ('no_proxy_server', False), ('profile_dir', None), ('profile_type', 'clean'),
> ('show_stdout', False), ('st ... _order_file', None), ('positional_args', []),
> ('print_bootstrap_deps', None), ('profile_dir', None), ('profile_type',
> 'clean'), ('profiler', None), ('repeat_count', 1), ('reset_results', None),
> ('results_label', None), ('run_disabled_tests', False), ('show_stdout', None),
> ('skip_navigate_on_repeat', False), ('synthetic_gesture_source_type',
> 'default'), ('upload_results', None), ('use_devtools_active_port', None),
> ('use_live_sites', None), ('verbosity', 0), ('write_full_results_to',
> '/tmp/tmpjhD2qP.json')] (truncated)
>   output_file  : '/tmp/tmpJPzPHo'
>   ps           : <telemetry.page.page_set.PageSet object at 0x315f250>
>   results      : <telemetry.results.page_test_results.PageTestResults object
at
> 0x315f0d0>
>   stdout       : 'RESULT metric: blank.html= [1,3] unit\nAvg metric:
> 2.000000unit\nSd  metric: 1.414214unit\nRESULT metric: green_rect.html= [2,4]
> unit\nAvg metric: 3.000000unit\nSd  metric: 1.414214unit\n*RESULT metric:
> metric= [1,2,3,4] unit\nAvg metric: 2.500000unit\nSd  metric:
> 1.290994unit\nRESULT telemetry_page_measurement_results: num_failed= 0
> count\nRESULT telemetry_page_measurement_results: num_errored= 0 count\n'
> 
> Pages: [blank.html,green_rect.html]
> [  FAILED  ] page_runner_unittest.PageRunnerTests.testPagesetRepeat (1883 ms).

I see the problem, but how the heck is this flaky and not just failing
outright...