Wire FailureValue in PageTestResults (try 2).

Previous patch failed because of GC issue (GC for the first test
only happens after the second test started, and it turns out
that we rely on __del__ in that test -- not good!). This is fixed in:

https://codereview.chromium.org/409113006/

BUG=392901

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

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

tools/perf/measurements/skpicture_printer_unittest.py

@@ -28,7 +28,7 @@ class SkpicturePrinterUnitTest(
 
     # Picture printing is not supported on all platforms.
     if results.failures:
-      assert 'not supported' in results.failures[0][1]
+      assert 'not supported' in results.failures[0].exc_info[1].message
       return
 
     saved_picture_count = results.FindAllPageSpecificValuesNamed(

tools/telemetry/telemetry/page/page_runner.py

@@ -27,7 +27,7 @@ from telemetry.page.actions import page_action
 from telemetry.results import results_options
 from telemetry.util import cloud_storage
 from telemetry.util import exception_formatter
-
+from telemetry.value import failure
 
 class _RunState(object):
   def __init__(self):
@@ -498,7 +498,8 @@ def _CheckArchives(page_set, pages, results):
 
   for page in pages_missing_archive_path + pages_missing_archive_data:
     results.StartTest(page)
-    results.AddFailureMessage(page, 'Page set archive doesn\'t exist.')
+    results.AddValue(failure.FailureValue.FromMessage(
+        page, 'Page set archive doesn\'t exist.'))
     results.StopTest(page)
 
   return [page for page in pages if page not in
@@ -521,7 +522,7 @@ def _RunPage(test, page, state, expectation, results, finder_options):
       results.AddSuccess(page)
     else:
       msg = 'Exception while running %s' % page.url
-      results.AddFailure(page, sys.exc_info())
+      results.AddValue(failure.FailureValue(page, sys.exc_info()))
     exception_formatter.PrintFormattedException(msg=msg)
 
   try:
@@ -541,7 +542,7 @@ def _RunPage(test, page, state, expectation, results, finder_options):
     else:
       exception_formatter.PrintFormattedException(
           msg='Failure while running %s' % page.url)
-      results.AddFailure(page, sys.exc_info())
+      results.AddValue(failure.FailureValue(page, sys.exc_info()))
   except (util.TimeoutException, exceptions.LoginException,
           exceptions.ProfilingException):
     ProcessError()
@@ -554,7 +555,7 @@ def _RunPage(test, page, state, expectation, results, finder_options):
   except Exception:
     exception_formatter.PrintFormattedException(
         msg='Unhandled exception while running %s' % page.url)
-    results.AddFailure(page, sys.exc_info())
+    results.AddValue(failure.FailureValue(page, sys.exc_info()))
   else:
     if expectation == 'fail':
       logging.warning('%s was expected to fail, but passed.\n', page.url)

tools/telemetry/telemetry/results/buildbot_page_measurement_results_unittest.py

@@ -5,11 +5,12 @@ import os
 
 from telemetry import perf_tests_helper
 from telemetry.page import page_set
+from telemetry.results import base_test_results_unittest
+from telemetry.results import buildbot_page_measurement_results
+from telemetry.value import failure
 from telemetry.value import histogram
 from telemetry.value import list_of_scalar_values
 from telemetry.value import scalar
-from telemetry.results import base_test_results_unittest
-from telemetry.results import buildbot_page_measurement_results
 
 
 def _MakePageSet():
@@ -130,7 +131,8 @@ class BuildbotPageMeasurementResultsTest(
     measurement_results.AddValue(scalar.ScalarValue(
         test_page_set.pages[0], 'a', 'seconds', 3))
     measurement_results.DidMeasurePage()
-    measurement_results.AddFailureMessage(test_page_set.pages[0], 'message')
+    measurement_results.AddValue(
+        failure.FailureValue.FromMessage(test_page_set.pages[0], 'message'))
 
     measurement_results.WillMeasurePage(test_page_set.pages[1])
     measurement_results.AddValue(scalar.ScalarValue(
@@ -161,7 +163,8 @@ class BuildbotPageMeasurementResultsTest(
     measurement_results.AddValue(scalar.ScalarValue(
         test_page_set.pages[1], 'a', 'seconds', 7))
     measurement_results.DidMeasurePage()
-    measurement_results.AddFailureMessage(test_page_set.pages[1], 'message')
+    measurement_results.AddValue(
+        failure.FailureValue.FromMessage(test_page_set.pages[1], 'message'))
 
     measurement_results.WillMeasurePage(test_page_set.pages[0])
     measurement_results.AddValue(scalar.ScalarValue(

tools/telemetry/telemetry/results/gtest_test_results.py

@@ -6,6 +6,7 @@ import logging
 import time
 
 from telemetry.results import page_test_results
+from telemetry.value import failure
 
 
 class GTestTestResults(page_test_results.PageTestResults):
@@ -16,20 +17,19 @@ class GTestTestResults(page_test_results.PageTestResults):
   def _GetMs(self):
     return (time.time() - self._timestamp) * 1000
 
-  def _emitFailure(self, page, err):
-    print >> self._output_stream, self._GetStringFromExcInfo(err)
-    print >> self._output_stream, '[  FAILED  ]', page.display_name, (
+  def _emitFailure(self, failure_value):
+    print >> self._output_stream, failure.GetStringFromExcInfo(
+        failure_value.exc_info)
+    display_name = failure_value.page.display_name
+    print >> self._output_stream, '[  FAILED  ]', display_name, (
         '(%0.f ms)' % self._GetMs())
     self._output_stream.flush()
 
   def AddValue(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)
+    assert isinstance(value, failure.FailureValue), (
+        'GTestTestResults only accepts FailureValue.')
+    super(GTestTestResults, self).AddValue(value)
+    self._emitFailure(value)
 
   def StartTest(self, page):
     super(GTestTestResults, self).StartTest(page)
@@ -61,9 +61,9 @@ class GTestTestResults(page_test_results.PageTestResults):
       unit = 'test' if len(self.failures) == 1 else 'tests'
       print >> self._output_stream, '[  FAILED  ]', (
           '%d %s, listed below:' % (len(self.failures), unit))
-      for page, _ in self.failures:
+      for failure_value in self.failures:
         print >> self._output_stream, '[  FAILED  ] ', (
-            page.display_name)
+            failure_value.page.display_name)
       print >> self._output_stream
       count = len(self.failures)
       unit = 'TEST' if count == 1 else 'TESTS'

tools/telemetry/telemetry/results/gtest_test_results_unittest.py

@@ -8,6 +8,7 @@ from telemetry.page import page_set
 from telemetry.results import base_test_results_unittest
 from telemetry.results import gtest_test_results
 from telemetry.unittest import simple_mock
+from telemetry.value import failure
 
 
 def _MakePageSet():
@@ -56,10 +57,10 @@ class GTestTestResultsTest(
 
     results = SummaryGtestTestResults()
     results.StartTest(test_page_set.pages[0])
-    exception = self.CreateException()
-    results.AddFailure(test_page_set.pages[0], exception)
+    exc_info = self.CreateException()
+    results.AddValue(failure.FailureValue(test_page_set.pages[0], exc_info))
     results.PrintSummary()
-    exception_trace = ''.join(traceback.format_exception(*exception))
+    exception_trace = ''.join(traceback.format_exception(*exc_info))
     expected = ('[ RUN      ] http://www.foo.com/\n'
                 '%s\n'
                 '[  FAILED  ] http://www.foo.com/ (0 ms)\n'
@@ -84,7 +85,7 @@ class GTestTestResultsTest(
   def testPassAndFailedPages(self):
     test_page_set = _MakePageSet()
     results = SummaryGtestTestResults()
-    exception = self.CreateException()
+    exc_info = self.CreateException()
 
     results.StartTest(test_page_set.pages[0])
     self._mock_timer.SetTime(0.007)
@@ -92,18 +93,18 @@ class GTestTestResultsTest(
 
     results.StartTest(test_page_set.pages[1])
     self._mock_timer.SetTime(0.009)
-    results.AddFailure(test_page_set.pages[1], exception)
+    results.AddValue(failure.FailureValue(test_page_set.pages[1], exc_info))
 
     results.StartTest(test_page_set.pages[2])
     self._mock_timer.SetTime(0.015)
-    results.AddFailure(test_page_set.pages[2], exception)
+    results.AddValue(failure.FailureValue(test_page_set.pages[2], exc_info))
 
     results.StartTest(test_page_set.pages[3])
     self._mock_timer.SetTime(0.020)
     results.AddSuccess(test_page_set.pages[3])
 
     results.PrintSummary()
-    exception_trace = ''.join(traceback.format_exception(*exception))
+    exception_trace = ''.join(traceback.format_exception(*exc_info))
     expected = ('[ RUN      ] http://www.foo.com/\n'
                 '[       OK ] http://www.foo.com/ (7 ms)\n'
                 '[ RUN      ] http://www.bar.com/\n'
@@ -124,7 +125,7 @@ class GTestTestResultsTest(
   def testStreamingResults(self):
     test_page_set = _MakePageSet()
     results = SummaryGtestTestResults()
-    exception = self.CreateException()
+    exc_info = self.CreateException()
 
     results.StartTest(test_page_set.pages[0])
     self._mock_timer.SetTime(0.007)
@@ -135,8 +136,8 @@ class GTestTestResultsTest(
 
     results.StartTest(test_page_set.pages[1])
     self._mock_timer.SetTime(0.009)
-    exception_trace = ''.join(traceback.format_exception(*exception))
-    results.AddFailure(test_page_set.pages[1], exception)
+    exception_trace = ''.join(traceback.format_exception(*exc_info))
+    results.AddValue(failure.FailureValue(test_page_set.pages[1], exc_info))
     expected = ('[ RUN      ] http://www.foo.com/\n'
                 '[       OK ] http://www.foo.com/ (7 ms)\n'
                 '[ RUN      ] http://www.bar.com/\n'

tools/telemetry/telemetry/results/page_measurement_results_unittest.py

@@ -7,6 +7,7 @@ import unittest
 from telemetry import perf_tests_helper
 from telemetry.page import page_set
 from telemetry.results import page_measurement_results
+from telemetry.value import failure
 from telemetry.value import histogram
 from telemetry.value import scalar
 
@@ -112,12 +113,12 @@ class PageMeasurementResultsTest(unittest.TestCase):
     results.WillMeasurePage(self.pages[0])
     results.AddValue(scalar.ScalarValue(self.pages[0], 'a', 'seconds', 3))
     results.DidMeasurePage()
-    results.AddFailureMessage(self.pages[0], 'message')
+    results.AddValue(failure.FailureValue.FromMessage(self.pages[0], 'message'))
 
     results.WillMeasurePage(self.pages[1])
     results.AddValue(scalar.ScalarValue(self.pages[1], 'a', 'seconds', 7))
     results.DidMeasurePage()
-    results.AddFailureMessage(self.pages[1], 'message')
+    results.AddValue(failure.FailureValue.FromMessage(self.pages[1], 'message'))
 
     results.PrintSummary()
     self.assertEquals(results.results, [])
@@ -135,37 +136,39 @@ class PageMeasurementResultsTest(unittest.TestCase):
   def test_get_all_values_for_successful_pages(self):
     results = SummarySavingPageMeasurementResults()
     results.WillMeasurePage(self.pages[0])
-    results.AddValue(scalar.ScalarValue(self.pages[0], 'a', 'seconds', 3))
+    value1 = scalar.ScalarValue(self.pages[0], 'a', 'seconds', 3)
+    results.AddValue(value1)
     results.DidMeasurePage()
 
     results.WillMeasurePage(self.pages[1])
-    results.AddValue(scalar.ScalarValue(self.pages[1], 'a', 'seconds', 3))
+    value2 = scalar.ScalarValue(self.pages[1], 'a', 'seconds', 3)
+    results.AddValue(value2)
     results.DidMeasurePage()
 
     results.WillMeasurePage(self.pages[2])
-    results.AddValue(scalar.ScalarValue(self.pages[2], 'a', 'seconds', 3))
+    value3 = scalar.ScalarValue(self.pages[2], 'a', 'seconds', 3)
+    results.AddValue(value3)
     results.DidMeasurePage()
 
-    values = results.all_page_specific_values
-    self.assertEquals(3, len(values))
-    self.assertEquals([self.pages[0], self.pages[1], self.pages[2]],
-                      [v.page for v in values])
+    self.assertEquals(
+        [value1, value2, value3], results.all_page_specific_values)
 
   def test_get_all_values_for_successful_pages_one_page_fails(self):
     results = SummarySavingPageMeasurementResults()
     results.WillMeasurePage(self.pages[0])
-    results.AddValue(scalar.ScalarValue(self.pages[0], 'a', 'seconds', 3))
+    value1 = scalar.ScalarValue(self.pages[0], 'a', 'seconds', 3)
+    results.AddValue(value1)
     results.DidMeasurePage()
 
     results.WillMeasurePage(self.pages[1])
-    results.AddFailureMessage(self.pages[1], "Failure")
+    value2 = failure.FailureValue.FromMessage(self.pages[1], 'Failure')
+    results.AddValue(value2)
     results.DidMeasurePage()
 
     results.WillMeasurePage(self.pages[2])
-    results.AddValue(scalar.ScalarValue(self.pages[2], 'a', 'seconds', 3))
+    value3 = scalar.ScalarValue(self.pages[2], 'a', 'seconds', 3)
+    results.AddValue(value3)
     results.DidMeasurePage()
 
-    values = results.all_page_specific_values
-    self.assertEquals(2, len(values))
-    self.assertEquals([self.pages[0], self.pages[2]],
-                      [v.page for v in values])
+    self.assertEquals(
+        [value1, value2, value3], results.all_page_specific_values)

tools/telemetry/telemetry/results/page_test_results.py

@@ -5,18 +5,18 @@
 import collections
 import copy
 import logging
-import sys
 import traceback
 
 from telemetry import value as value_module
+from telemetry.value import failure
 
 class PageTestResults(object):
   def __init__(self, output_stream=None):
     super(PageTestResults, self).__init__()
     self._output_stream = output_stream
-    self.pages_that_had_failures = set()
+    # TODO(chrishenry,eakuefner): Remove self.successes once they can
+    # be inferred.
     self.successes = []
-    self.failures = []
     self.skipped = []
 
     self._representative_value_for_each_value_name = {}
@@ -42,10 +42,21 @@ class PageTestResults(object):
 
   @property
   def pages_that_succeeded(self):
-    pages = set([value.page for value in self._all_page_specific_values])
+    """Returns the set of pages that succeeded."""
+    pages = set(value.page for value in self._all_page_specific_values)
     pages.difference_update(self.pages_that_had_failures)
     return pages
 
+  @property
+  def pages_that_had_failures(self):
+    """Returns the set of failed pages."""
+    return set(v.page for v in self.failures)
+
+  @property
+  def failures(self):
+    values = self._all_page_specific_values
+    return [v for v in values if isinstance(v, failure.FailureValue)]
+
   def _GetStringFromExcInfo(self, err):
     return ''.join(traceback.format_exception(*err))
 
@@ -72,26 +83,16 @@ class PageTestResults(object):
         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)))
-
   def AddSkip(self, page, reason):
     self.skipped.append((page, reason))
 
   def AddSuccess(self, page):
     self.successes.append(page)
 
-  def AddFailureMessage(self, page, message):
-    try:
-      raise Exception(message)
-    except Exception:
-      self.AddFailure(page, sys.exc_info())
-
   def PrintSummary(self):
     if self.failures:
       logging.error('Failed pages:\n%s', '\n'.join(
-          p.display_name for p in zip(*self.failures)[0]))
+          p.display_name for p in self.pages_that_had_failures))
 
     if self.skipped:
       logging.warning('Skipped pages:\n%s', '\n'.join(

tools/telemetry/telemetry/results/page_test_results_unittest.py

@@ -6,6 +6,7 @@ from telemetry.results import base_test_results_unittest
 
 from telemetry.page import page_set
 from telemetry.results import page_test_results
+from telemetry.value import failure
 
 class NonPrintingPageTestResults(
     page_test_results.PageTestResults):
@@ -28,8 +29,8 @@ class PageTestResultsTest(base_test_results_unittest.BaseTestResultsUnittest):
 
   def test_failures(self):
     results = NonPrintingPageTestResults()
-    results.AddFailure(self.pages[0], self.CreateException())
+    results.AddValue(
+        failure.FailureValue(self.pages[0], self.CreateException()))
     results.AddSuccess(self.pages[1])
-    self.assertEquals(results.pages_that_had_failures,
-                      set([self.pages[0]]))
+    self.assertEquals(results.pages_that_had_failures, set([self.pages[0]]))
     self.assertEquals(results.successes, [self.pages[1]])