[Telemetry] Make --chartjson output chart.json instead of to stdout.

[Telemetry] Make --chartjson output chart.json instead of to stdout.

The new Chart JSON handling code in the buildbot will use the file chart.json
instead of standard out, so that Buildbot will not have to do any parsing of
logging output at all. This CL brings the Chart JSON handling code in line with
this approach.

R=sullivan,nednguyen
CC=nduca

Committed: https://crrev.com/c8e067f6f8d6d1445a42ec2196d8a194af60ca1f
Cr-Commit-Position: refs/heads/master@{#294020}

[sullivan, 2014-09-08 19:27:59]

https://codereview.chromium.org/551973002/diff/1/tools/telemetry/telemetry/be...
File tools/telemetry/telemetry/benchmark.py (right):

https://codereview.chromium.org/551973002/diff/1/tools/telemetry/telemetry/be...
tools/telemetry/telemetry/benchmark.py:105: 'chart.json'))
From the buildbot perspective, it would be better to pass in the name of the
output file on the command line. The code that needs to read the file won't get
called until the test has been run with both the build under test and the ref
build, so it'd be hard to keep the ref build run from overwriting the previous
run.

[eakuefner, 2014-09-08 20:15:32]

Annie: The latest patch set allows Chart JSON to be output to standard out by
default, or an arbitrary filename otherwise, by passing
--output-format=chartjson and optionally --output=file.json.

Ned: If you wouldn't mind taking a look at the latest patch set, it removes the
old --chartjson hack and just makes chart JSON an output formatter for now.

[sullivan, 2014-09-08 20:17:58]

On 2014/09/08 20:15:32, eakuefner wrote:
> Annie: The latest patch set allows Chart JSON to be output to standard out by
> default, or an arbitrary filename otherwise, by passing
> --output-format=chartjson and optionally --output=file.json.
> 
> Ned: If you wouldn't mind taking a look at the latest patch set, it removes
the
> old --chartjson hack and just makes chart JSON an output formatter for now.

The new arguments lgtm

[nednguyen, 2014-09-08 20:26:12]

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

https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:69: return
self._benchmark_metadata
Why does this need to be exposed? Please limit the accessibility as much as
possible.

[nednguyen, 2014-09-08 20:28:46]

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

https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:12: def
ResultsAsChartDict(benchmark_metadata, page_specific_values,
This should be made private too.

[eakuefner, 2014-09-09 01:21:02]

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

https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:12: def
ResultsAsChartDict(benchmark_metadata, page_specific_values,
On 2014/09/08 20:28:45, nednguyen wrote:
> This should be made private too.

Done.

https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:69: return
self._benchmark_metadata
On 2014/09/08 20:26:12, nednguyen wrote:
> Why does this need to be exposed? Please limit the accessibility as much as
> possible. 

Done.

[eakuefner, 2014-09-09 01:29:52]

On 2014/09/09 01:21:02, eakuefner wrote:
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/results/chart_json_output_formatter.py (right):
> 
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/results/chart_json_output_formatter.py:12: def
> ResultsAsChartDict(benchmark_metadata, page_specific_values,
> On 2014/09/08 20:28:45, nednguyen wrote:
> > This should be made private too.
> 
> Done.
> 
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/results/chart_json_output_formatter.py:69: return
> self._benchmark_metadata
> On 2014/09/08 20:26:12, nednguyen wrote:
> > Why does this need to be exposed? Please limit the accessibility as much as
> > possible. 
> 
> Done.

Ned, one issue is that I would like to unittest on _ResultsAsChartDict, but
making it private it makes PyLint complain. Would it be better to keep it
public, or to suppress the warning in the unittests? Or is there something else
I ought to consider?

[nednguyen, 2014-09-09 02:17:13]

On 2014/09/09 01:29:52, eakuefner wrote:
> On 2014/09/09 01:21:02, eakuefner wrote:
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > File tools/telemetry/telemetry/results/chart_json_output_formatter.py
(right):
> > 
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/results/chart_json_output_formatter.py:12: def
> > ResultsAsChartDict(benchmark_metadata, page_specific_values,
> > On 2014/09/08 20:28:45, nednguyen wrote:
> > > This should be made private too.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > tools/telemetry/telemetry/results/chart_json_output_formatter.py:69: return
> > self._benchmark_metadata
> > On 2014/09/08 20:26:12, nednguyen wrote:
> > > Why does this need to be exposed? Please limit the accessibility as much
as
> > > possible. 
> > 
> > Done.
> 
> Ned, one issue is that I would like to unittest on _ResultsAsChartDict, but
> making it private it makes PyLint complain. Would it be better to keep it
> public, or to suppress the warning in the unittests? Or is there something
else
> I ought to consider?

Let's suppress the warning in unittest.

[nednguyen, 2014-09-09 02:19:01]

https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/chart_json_output_formatter.py (right):

https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:68: def
benchmark_metadata(self):
Can you remove this as well?

[eakuefner, 2014-09-09 17:05:18]

On 2014/09/09 02:17:13, nednguyen wrote:
> On 2014/09/09 01:29:52, eakuefner wrote:
> > On 2014/09/09 01:21:02, eakuefner wrote:
> > >
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > > File tools/telemetry/telemetry/results/chart_json_output_formatter.py
> (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > > tools/telemetry/telemetry/results/chart_json_output_formatter.py:12: def
> > > ResultsAsChartDict(benchmark_metadata, page_specific_values,
> > > On 2014/09/08 20:28:45, nednguyen wrote:
> > > > This should be made private too.
> > > 
> > > Done.
> > > 
> > >
> >
>
https://codereview.chromium.org/551973002/diff/20001/tools/telemetry/telemetr...
> > > tools/telemetry/telemetry/results/chart_json_output_formatter.py:69:
return
> > > self._benchmark_metadata
> > > On 2014/09/08 20:26:12, nednguyen wrote:
> > > > Why does this need to be exposed? Please limit the accessibility as much
> as
> > > > possible. 
> > > 
> > > Done.
> > 
> > Ned, one issue is that I would like to unittest on _ResultsAsChartDict, but
> > making it private it makes PyLint complain. Would it be better to keep it
> > public, or to suppress the warning in the unittests? Or is there something
> else
> > I ought to consider?
> 
> Let's suppress the warning in unittest.

Done.

[eakuefner, 2014-09-09 17:05:25]

https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/chart_json_output_formatter.py (right):

https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/chart_json_output_formatter.py:68: def
benchmark_metadata(self):
On 2014/09/09 02:19:01, nednguyen wrote:
> Can you remove this as well? 

Done. Weirdly, I killed this but it didn't appear in the diff for this patch
set.

[nednguyen, 2014-09-09 17:20:14]

On 2014/09/09 17:05:25, eakuefner wrote:
>
https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/results/chart_json_output_formatter.py (right):
> 
>
https://codereview.chromium.org/551973002/diff/60001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/results/chart_json_output_formatter.py:68: def
> benchmark_metadata(self):
> On 2014/09/09 02:19:01, nednguyen wrote:
> > Can you remove this as well? 
> 
> Done. Weirdly, I killed this but it didn't appear in the diff for this patch
> set.

lgtm

FYI, if you want to test chart_json_output_formatter without needed to touch it
private implementation, you can still do so through a fake output_stream object
(we have that in other output formatter unittests). Then you can rebuild the
dictionary from string through json.loads() and check its content.

[eakuefner-google (do not use), 2014-09-09 17:45:32]

The CQ bit was checked by eakuefner@google.com

[commit-bot: I haz the power, 2014-09-09 17:46:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/eakuefner@chromium.org/551973002/80001

[commit-bot: I haz the power, 2014-09-09 19:36:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-09 19:36:34]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eakuefner, 2014-09-09 20:35:27]

The CQ bit was checked by eakuefner@chromium.org

[commit-bot: I haz the power, 2014-09-09 20:58:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/eakuefner@chromium.org/551973002/80001

[commit-bot: I haz the power, 2014-09-09 22:05:08]

Committed patchset #5 (id:80001) as 844ead01070d2cad92a1ff1858301beac5c3676f

[commit-bot: I haz the power, 2014-09-10 03:56:03]

Patchset 5 (id:??) landed as
https://crrev.com/c8e067f6f8d6d1445a42ec2196d8a194af60ca1f
Cr-Commit-Position: refs/heads/master@{#294020}