Merge all logic in PageMeasurementResults to PageTestResults.

Merge all logic in PageMeasurementResults to PageTestResults.

This consists of:
1) Deletion of WillMeasurePage/DidMeasurePage and
page_specific_results_for_current_page (which does not
make sense since a results can be added any time between
StartTest/StopTest.
2) Moving trace_tag and current_page property to PageTestResults;
current_page is filled at StartTest and reset at StopTest.

The class itself will be deleted separately after all
of its subclasses are deleted or switched to OutputFormatter
(as per thread on telemetry@chromium.org).

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

[chrishenry, 2014-07-24 23:34:22]

Btw, this is blocked on:
https://codereview.chromium.org/394953002/

[chrishenry, 2014-07-25 16:01:46]

On 2014/07/24 23:34:22, chrishenry wrote:
> Btw, this is blocked on:
> https://codereview.chromium.org/394953002/

The blocking patch is submitted, and I've rebased this patch. This is ready for
review.

[nednguyen, 2014-07-25 16:14:45]

LGTM with some comments

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
File tools/perf/measurements/page_cycler_unittest.py (right):

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
tools/perf/measurements/page_cycler_unittest.py:171: results.StartTest(page)
I think we use one results throughout the whole telemetry test run, isn't it?
If so, a better test maybe asserting that the results contain all the values
across 3 runs.

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

https://codereview.chromium.org/403093002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:14: def __init__(self,
output_stream=None, trace_tag=''):
It's not clear to me what "trace_tage" means and how it is used. Can you add
documentation for it?

https://codereview.chromium.org/403093002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:48: return
self._current_page
I think the current_page thing is lame, we should get rid of it eventually.

If so, the end goal of StartTest & StopTest is that they are only used for
reporting the status of test, is that right?

[chrishenry, 2014-07-25 16:24:28]

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
File tools/perf/measurements/page_cycler_unittest.py (right):

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
tools/perf/measurements/page_cycler_unittest.py:171: results.StartTest(page)
On 2014/07/25 16:14:45, nednguyen wrote:
> I think we use one results throughout the whole telemetry test run, isn't it?
> If so, a better test maybe asserting that the results contain all the values
> across 3 runs.

I think that will make the test unnecessarily complicated. Another way to do
this is to check all_page_specific_values after every MeasurePage and check for
the additional values, but then that would be no better than this logic. s:

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

https://codereview.chromium.org/403093002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:14: def __init__(self,
output_stream=None, trace_tag=''):
On 2014/07/25 16:14:45, nednguyen wrote:
> It's not clear to me what "trace_tage" means and how it is used. Can you add
> documentation for it?

I don't know what it is either. Seems to only be used from Buildbot. Also,
documentation for --output-trace-tag is pretty vague: "Append a tag to the key
of each result trace."

Here is the final usage:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

Still no clue what it's doing.

https://codereview.chromium.org/403093002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/results/page_test_results.py:48: return
self._current_page
On 2014/07/25 16:14:45, nednguyen wrote:
> I think the current_page thing is lame, we should get rid of it eventually.
> 
> If so, the end goal of StartTest & StopTest is that they are only used for
> reporting the status of test, is that right?

Perhaps. A lot of the usage of current_page is what you added as you cleaned up
Add(). I don't think it's super critical to clean this up tho. It seems ok,
especially since we'll likely also have page_run information soon. <- this
answer your second question too, StartTest will create a new page_run.

[nednguyen, 2014-07-25 16:36:58]

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
File tools/perf/measurements/page_cycler_unittest.py (right):

https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
tools/perf/measurements/page_cycler_unittest.py:171: results.StartTest(page)
On 2014/07/25 16:24:28, chrishenry wrote:
> On 2014/07/25 16:14:45, nednguyen wrote:
> > I think we use one results throughout the whole telemetry test run, isn't
it?
> > If so, a better test maybe asserting that the results contain all the values
> > across 3 runs.
> 
> I think that will make the test unnecessarily complicated. Another way to do
> this is to check all_page_specific_values after every MeasurePage and check
for
> the additional values, but then that would be no better than this logic. s:

Ok. Chatted offline, we probably should fix the page_cylcer WillMeasure method
later.

[nednguyen, 2014-07-25 16:38:00]

On 2014/07/25 16:36:58, nednguyen wrote:
>
https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
> File tools/perf/measurements/page_cycler_unittest.py (right):
> 
>
https://codereview.chromium.org/403093002/diff/60001/tools/perf/measurements/...
> tools/perf/measurements/page_cycler_unittest.py:171: results.StartTest(page)
> On 2014/07/25 16:24:28, chrishenry wrote:
> > On 2014/07/25 16:14:45, nednguyen wrote:
> > > I think we use one results throughout the whole telemetry test run, isn't
> it?
> > > If so, a better test maybe asserting that the results contain all the
values
> > > across 3 runs.
> > 
> > I think that will make the test unnecessarily complicated. Another way to do
> > this is to check all_page_specific_values after every MeasurePage and check
> for
> > the additional values, but then that would be no better than this logic. s:
> 
> Ok. Chatted offline, we probably should fix the page_cylcer WillMeasure method
> later.

err, MeasurePage

[chrishenry, 2014-07-25 16:47:54]

The CQ bit was checked by chrishenry@google.com

[commit-bot: I haz the power, 2014-07-25 16:49:59]

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

[commit-bot: I haz the power, 2014-07-25 17:25:48]

Change committed as 285612