Have all --output-format produces progress report (in gtest-style).

Have all --output-format produces progress report (in gtest-style).

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

[chrishenry, 2014-08-01 06:28:42]

This is what I'm thinking, in lieu of
https://codereview.chromium.org/411143003/.

[Ken Russell (switch to Gerrit), 2014-08-01 23:10:27]

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/page_test_results.py (right):

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:95:
self._progress_reporter.WillRunPage(page)
It looks to me like the progress reporter is implementing the same exact
interface as the output formatter. I see no reason to have a distinction between
the two interfaces. Instead, it would seem more natural to me to support
multiple simultaneous output formatters with multiple parameters. One of them
could be the gtest output formatter which would stream to stdout. Of course
having multiple output formatters streaming to stdout simultaneously would
collide, but that would be user error.

[chrishenry, 2014-08-01 23:20:51]

On 2014/08/01 23:10:27, Ken Russell wrote:
>
https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/results/page_test_results.py (right):
> 
>
https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/results/page_test_results.py:95:
> self._progress_reporter.WillRunPage(page)
> It looks to me like the progress reporter is implementing the same exact
> interface as the output formatter. I see no reason to have a distinction
between
> the two interfaces. Instead, it would seem more natural to me to support
> multiple simultaneous output formatters with multiple parameters. One of them
> could be the gtest output formatter which would stream to stdout. Of course
> having multiple output formatters streaming to stdout simultaneously would
> collide, but that would be user error.

It's not the same (unless you meant the unittest verson of output formatter,
which is not the same as the benchmark version, which is here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...).
We decided to not support streaming for output formatter, which makes the
interface really simple. Progress reporter is a kind of output formatter that
reports progress, so the interface is different, more complicated.

(And, btw, we _already_ support multiple output formatter. In fact
--output-format=html is implemented as buildbot+html output formatter.)

[nduca, 2014-08-02 00:36:34]

lgtm puzzled at why there aren't DidRunPage, etc on the reporter but nitty

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/page_test_results.py (right):

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:105:
self._progress_reporter.AddValue(value)
please use Did and Will convention.

DidAddValue.

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:125:
self._progress_reporter.PrintSummary(self)
PrintSummary is a horrible horrible hack. DidFinishAllTests() is a better word.
Maybe there is better. Since this is a fresh class, lets pick the right thing
even though its still called PrintSummary in the caller.

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/progress_reporter.py (right):

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/progress_reporter.py:39: class
GTestProgressReporter(ProgressReporter):
move to separate file -- if we dont intend to subclass or have fakes, then call
it gtest progress reporter. if we have fakes, which makes sense, then one class
per file

[chrishenry, 2014-08-02 01:11:05]

The DidRunTest is in a separate patch to follow up (I've done the
implementation, just need to land this patch first).

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/page_test_results.py (right):

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:105:
self._progress_reporter.AddValue(value)
On 2014/08/02 00:36:33, nduca wrote:
> please use Did and Will convention.
> 
> DidAddValue.

Done.

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:125:
self._progress_reporter.PrintSummary(self)
On 2014/08/02 00:36:33, nduca wrote:
> PrintSummary is a horrible horrible hack. DidFinishAllTests() is a better
word.
> Maybe there is better. Since this is a fresh class, lets pick the right thing
> even though its still called PrintSummary in the caller.

Done.

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/results/progress_reporter.py (right):

https://codereview.chromium.org/430383003/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/progress_reporter.py:39: class
GTestProgressReporter(ProgressReporter):
On 2014/08/02 00:36:33, nduca wrote:
> move to separate file -- if we dont intend to subclass or have fakes, then
call
> it gtest progress reporter. if we have fakes, which makes sense, then one
class
> per file

Done.

[chrishenry, 2014-08-02 05:30:36]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-08-02 05:31:50]

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

[commit-bot: I haz the power, 2014-08-02 13:21:13]

Change committed as 287213