Move AddValue/AddSummaryValue to PageTestResults.

This is needed for FailureValue patch (so that one can call
AddValue(FailureValue)). This also brings us closer to merging
PageTestResults and PageMeasurementResults -- to kill the rest of the
methods, we will need to make more progress on merging PageTest and
PageMeasurement.

Unit tests aren't moved because they still depend on
{Will,Did}MeasurePage.

Additional cleanup: kill dead code in page_measurement_results.py
(self._done).

BUG=383635,392901

Review URL: https://codereview.chromium.org/398583002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283785 0039d316-1c4b-4281-b951-d872f2087c98

tools/telemetry/telemetry/results/buildbot_page_measurement_results.py

@@ -80,7 +80,7 @@ class BuildbotPageMeasurementResults(
     # If there were no failed pages, output the overall results (results not
     # associated with a page).
     if not self.had_failures:
-      for value in self._all_summary_values:
+      for value in self.all_summary_values:
         buildbot_value = value.GetBuildbotValue()
         buildbot_data_type = value.GetBuildbotDataType(
             output_context=value_module.SUMMARY_RESULT_OUTPUT_CONTEXT)

tools/telemetry/telemetry/results/gtest_test_results.py

@@ -22,6 +22,11 @@ class GTestTestResults(page_test_results.PageTestResults):
         '(%0.f ms)' % self._GetMs())
     self._output_stream.flush()
 
+  def ValidateValue(self, value):
+    # TODO(chrishenry): When FailureValue is added, this should instead
+    # validate that isinstance(value, FailureValue) is true.
+    raise Exception('GTestTestResults does not support AddValue().')
+
   def AddFailure(self, page, err):
     super(GTestTestResults, self).AddFailure(page, err)
     self._emitFailure(page, err)

tools/telemetry/telemetry/results/page_measurement_results.py

@@ -2,38 +2,21 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-from telemetry import value as value_module
 from telemetry.results import page_test_results
 from telemetry.value import value_backcompat
 
 class PageMeasurementResults(page_test_results.PageTestResults):
   def __init__(self, output_stream=None, trace_tag=''):
     super(PageMeasurementResults, self).__init__(output_stream)
-    self._done = False
     self._trace_tag = trace_tag
 
     self._current_page = None
     self._page_specific_values_for_current_page = None
 
-    self._representative_value_for_each_value_name = {}
-
-    self._all_summary_values = []
-    self._all_page_specific_values = []
-
-  @property
-  def pages_that_succeeded(self):
-    pages = set([value.page for value in self._all_page_specific_values])
-    pages.difference_update(self.pages_that_had_failures)
-    return pages
-
   @property
   def current_page(self):
     return self._current_page
 
-  @property
-  def all_page_specific_values(self):
-    return self._all_page_specific_values
-
   @property
   def page_specific_values_for_current_page(self):
     assert self._current_page
@@ -44,7 +27,9 @@ class PageMeasurementResults(page_test_results.PageTestResults):
     self._current_page = page
     self._page_specific_values_for_current_page = []
 
+  # TODO(nednguyen): Ned has a patch to kill this.
   def Add(self, trace_name, units, value, chart_name=None, data_type='default'):
+    assert self._current_page
     # TODO(isherman): Remove this as well.
     value = value_backcompat.ConvertOldCallingConventionToValue(
       self._current_page,
@@ -52,40 +37,10 @@ class PageMeasurementResults(page_test_results.PageTestResults):
     self.AddValue(value)
 
   def AddValue(self, value):
-    self._ValidateValue(value)
+    super(PageMeasurementResults, self).AddValue(value)
     self._page_specific_values_for_current_page.append(value)
-    self._all_page_specific_values.append(value)
-
-  def AddSummaryValue(self, value):
-    assert value.page is None
-    self._ValidateValue(value)
-    self._all_summary_values.append(value)
-
-  def _ValidateValue(self, value):
-    assert isinstance(value, value_module.Value)
-    if value.name not in self._representative_value_for_each_value_name:
-      self._representative_value_for_each_value_name[value.name] = value
-    representative_value = self._representative_value_for_each_value_name[
-        value.name]
-    assert value.IsMergableWith(representative_value)
 
   def DidMeasurePage(self):
     assert self._current_page, 'Failed to call WillMeasurePage'
     self._current_page = None
     self._page_specific_values_for_current_page = None
-
-
-  def FindPageSpecificValuesForPage(self, page, value_name):
-    values = []
-    for value in self._all_page_specific_values:
-      if value.page == page and value.name == value_name:
-        values.append(value)
-    return values
-
-
-  def FindAllPageSpecificValuesNamed(self, value_name):
-    values = []
-    for value in self._all_page_specific_values:
-      if value.name == value_name:
-        values.append(value)
-    return values

tools/telemetry/telemetry/results/page_test_results.py

@@ -8,6 +8,7 @@ import logging
 import sys
 import traceback
 
+from telemetry import value as value_module
 
 class PageTestResults(object):
   def __init__(self, output_stream=None):
@@ -18,6 +19,10 @@ class PageTestResults(object):
     self.failures = []
     self.skipped = []
 
+    self._representative_value_for_each_value_name = {}
+    self._all_page_specific_values = []
+    self._all_summary_values = []
+
   def __copy__(self):
     cls = self.__class__
     result = cls.__new__(cls)
@@ -27,6 +32,20 @@ class PageTestResults(object):
       setattr(result, k, v)
     return result
 
+  @property
+  def all_page_specific_values(self):
+    return self._all_page_specific_values
+
+  @property
+  def all_summary_values(self):
+    return self._all_summary_values
+
+  @property
+  def pages_that_succeeded(self):
+    pages = set([value.page for value in self._all_page_specific_values])
+    pages.difference_update(self.pages_that_had_failures)
+    return pages
+
   def _GetStringFromExcInfo(self, err):
     return ''.join(traceback.format_exception(*err))
 
@@ -36,6 +55,23 @@ class PageTestResults(object):
   def StopTest(self, page):
     pass
 
+  def AddValue(self, value):
+    self.ValidateValue(value)
+    self._all_page_specific_values.append(value)
+
+  def AddSummaryValue(self, value):
+    assert value.page is None
+    self.ValidateValue(value)
+    self._all_summary_values.append(value)
+
+  def ValidateValue(self, value):
+    assert isinstance(value, value_module.Value)
+    if value.name not in self._representative_value_for_each_value_name:
+      self._representative_value_for_each_value_name[value.name] = value
+    representative_value = self._representative_value_for_each_value_name[
+        value.name]
+    assert value.IsMergableWith(representative_value)
+
   def AddFailure(self, page, err):
     self.pages_that_had_failures.add(page)
     self.failures.append((page, self._GetStringFromExcInfo(err)))
@@ -60,3 +96,17 @@ class PageTestResults(object):
     if self.skipped:
       logging.warning('Skipped pages:\n%s', '\n'.join(
           p.display_name for p in zip(*self.skipped)[0]))
+
+  def FindPageSpecificValuesForPage(self, page, value_name):
+    values = []
+    for value in self.all_page_specific_values:
+      if value.page == page and value.name == value_name:
+        values.append(value)
+    return values
+
+  def FindAllPageSpecificValuesNamed(self, value_name):
+    values = []
+    for value in self.all_page_specific_values:
+      if value.name == value_name:
+        values.append(value)
+    return values