Wire up SkipValue to page_test_results

Now that SkipValues are a part of the Telemetry value system, this CL modifies
the existing code to use SkipValues and removes the old AddSkip logic that is
no longer used.

BUG=395306

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

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

5 files changed, 40 insertions, 24 deletions

diff --git a/tools/telemetry/telemetry/page/page_runner.py b/tools/telemetry/telemetry/page/page_runner.py
index 389c03b..d5f0c94 100644
--- a/tools/telemetry/telemetry/page/page_runner.py
+++ b/tools/telemetry/telemetry/page/page_runner.py
@@ -28,6 +28,7 @@ from telemetry.results import results_options
 from telemetry.util import cloud_storage
 from telemetry.util import exception_formatter
 from telemetry.value import failure
+from telemetry.value import skip
 
 class _RunState(object):
   def __init__(self):
@@ -391,7 +392,7 @@ def Run(test, page_set, expectations, finder_options):
     if not test.CanRunForPage(page):
       results.StartTest(page)
       logging.debug('Skipping test: it cannot run for %s', page.url)
-      results.AddSkip(page, 'Test cannot run')
+      results.AddValue(skip.SkipValue(page, 'Test cannot run'))
       results.StopTest(page)
       pages.remove(page)
 
@@ -509,7 +510,7 @@ def _CheckArchives(page_set, pages, results):
 def _RunPage(test, page, state, expectation, results, finder_options):
   if expectation == 'skip':
     logging.debug('Skipping test: Skip expectation for %s', page.url)
-    results.AddSkip(page, 'Skipped by test expectations')
+    results.AddValue(skip.SkipValue(page, 'Skipped by test expectations'))
     return
 
   logging.info('Running %s', page.url)
@@ -551,7 +552,7 @@ def _RunPage(test, page, state, expectation, results, finder_options):
     # Run() catches these exceptions to relaunch the tab/browser, so re-raise.
     raise
   except page_action.PageActionNotSupported as e:
-    results.AddSkip(page, 'Unsupported page action: %s' % e)
+    results.AddValue(skip.SkipValue(page, 'Unsupported page action: %s' % e))
   except Exception:
     exception_formatter.PrintFormattedException(
         msg='Unhandled exception while running %s' % page.url)
diff --git a/tools/telemetry/telemetry/page/record_wpr.py b/tools/telemetry/telemetry/page/record_wpr.py
index c88df05..92cdad9 100755
--- a/tools/telemetry/telemetry/page/record_wpr.py
+++ b/tools/telemetry/telemetry/page/record_wpr.py
@@ -170,7 +170,7 @@ class WprRecorder(object):
                            test_expectations.TestExpectations(), self._options)
 
   def HandleResults(self, results):
-    if results.failures or results.skipped:
+    if results.failures or results.skipped_values:
       logging.warning('Some pages failed and/or were skipped. The recording '
                       'has not been updated for these pages.')
     results.PrintSummary()
diff --git a/tools/telemetry/telemetry/results/gtest_test_results.py b/tools/telemetry/telemetry/results/gtest_test_results.py
index c5ff8a3..82a4049 100644
--- a/tools/telemetry/telemetry/results/gtest_test_results.py
+++ b/tools/telemetry/telemetry/results/gtest_test_results.py
@@ -7,6 +7,7 @@ import time
 
 from telemetry.results import page_test_results
 from telemetry.value import failure
+from telemetry.value import skip
 
 
 class GTestTestResults(page_test_results.PageTestResults):
@@ -17,7 +18,7 @@ class GTestTestResults(page_test_results.PageTestResults):
   def _GetMs(self):
     return (time.time() - self._timestamp) * 1000
 
-  def _emitFailure(self, failure_value):
+  def _EmitFailure(self, failure_value):
     print >> self._output_stream, failure.GetStringFromExcInfo(
         failure_value.exc_info)
     display_name = failure_value.page.display_name
@@ -25,11 +26,29 @@ class GTestTestResults(page_test_results.PageTestResults):
         '(%0.f ms)' % self._GetMs())
     self._output_stream.flush()
 
+  def _EmitSkip(self, skip_value):
+    page = skip_value.page
+    reason = skip_value.reason
+    logging.warning('===== SKIPPING TEST %s: %s =====',
+                    page.display_name, reason)
+    if self._timestamp == None:
+      self._timestamp = time.time()
+    print >> self._output_stream, '[       OK ]', page.display_name, (
+        '(%0.f ms)' % self._GetMs())
+    self._output_stream.flush()
+
   def AddValue(self, value):
-    assert isinstance(value, failure.FailureValue), (
-        'GTestTestResults only accepts FailureValue.')
+    is_failure = isinstance(value, failure.FailureValue)
+    is_skip = isinstance(value, skip.SkipValue)
+
+    assert is_failure or is_skip, (
+        'GTestTestResults only accepts FailureValue or SkipValue.')
     super(GTestTestResults, self).AddValue(value)
-    self._emitFailure(value)
+    # TODO(eakuefner/chrishenry): move emit failure/skip output to DidRunPage.
+    if is_failure:
+      self._EmitFailure(value)
+    elif is_skip:
+      self._EmitSkip(value)
 
   def StartTest(self, page):
     super(GTestTestResults, self).StartTest(page)
@@ -43,16 +62,6 @@ class GTestTestResults(page_test_results.PageTestResults):
         '(%0.f ms)' % self._GetMs())
     self._output_stream.flush()
 
-  def AddSkip(self, page, reason):
-    super(GTestTestResults, self).AddSkip(page, reason)
-    logging.warning('===== SKIPPING TEST %s: %s =====',
-                    page.display_name, reason)
-    if self._timestamp == None:
-      self._timestamp = time.time()
-    print >> self._output_stream, '[       OK ]', page.display_name, (
-        '(%0.f ms)' % self._GetMs())
-    self._output_stream.flush()
-
   def PrintSummary(self):
     unit = 'test' if len(self.successes) == 1 else 'tests'
     print >> self._output_stream, '[  PASSED  ]', (
diff --git a/tools/telemetry/telemetry/results/gtest_test_results_unittest.py b/tools/telemetry/telemetry/results/gtest_test_results_unittest.py
index a77f76f..39731e70 100644
--- a/tools/telemetry/telemetry/results/gtest_test_results_unittest.py
+++ b/tools/telemetry/telemetry/results/gtest_test_results_unittest.py
@@ -9,6 +9,7 @@ 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
+from telemetry.value import skip
 
 
 def _MakePageSet():
@@ -75,7 +76,8 @@ class GTestTestResultsTest(
     results = SummaryGtestTestResults()
     results.StartTest(test_page_set.pages[0])
     self._mock_timer.SetTime(0.007)
-    results.AddSkip(test_page_set.pages[0], 'Page skipped for testing reason')
+    results.AddValue(skip.SkipValue(test_page_set.pages[0],
+        'Page skipped for testing reason'))
     results.PrintSummary()
     expected = ('[ RUN      ] http://www.foo.com/\n'
                 '[       OK ] http://www.foo.com/ (7 ms)\n'
diff --git a/tools/telemetry/telemetry/results/page_test_results.py b/tools/telemetry/telemetry/results/page_test_results.py
index 6b6c09a..c5a7641 100644
--- a/tools/telemetry/telemetry/results/page_test_results.py
+++ b/tools/telemetry/telemetry/results/page_test_results.py
@@ -9,6 +9,7 @@ import traceback
 
 from telemetry import value as value_module
 from telemetry.value import failure
+from telemetry.value import skip
 
 class PageTestResults(object):
   def __init__(self, output_stream=None, output_formatters=None, trace_tag=''):
@@ -33,7 +34,6 @@ class PageTestResults(object):
     # TODO(chrishenry,eakuefner): Remove self.successes once they can
     # be inferred.
     self.successes = []
-    self.skipped = []
 
     self._representative_value_for_each_value_name = {}
     self._all_page_specific_values = []
@@ -77,6 +77,11 @@ class PageTestResults(object):
     values = self._all_page_specific_values
     return [v for v in values if isinstance(v, failure.FailureValue)]
 
+  @property
+  def skipped_values(self):
+    values = self._all_page_specific_values
+    return [v for v in values if isinstance(v, skip.SkipValue)]
+
   def _GetStringFromExcInfo(self, err):
     return ''.join(traceback.format_exception(*err))
 
@@ -88,6 +93,8 @@ class PageTestResults(object):
 
   def AddValue(self, value):
     self._ValidateValue(value)
+    # TODO(eakuefner/chrishenry): move emit failure/skip output to DidRunPage.
+    assert len(self.skipped_values) == 0
     self._all_page_specific_values.append(value)
 
   def AddSummaryValue(self, value):
@@ -103,9 +110,6 @@ class PageTestResults(object):
         value.name]
     assert value.IsMergableWith(representative_value)
 
-  def AddSkip(self, page, reason):
-    self.skipped.append((page, reason))
-
   def AddSuccess(self, page):
     self.successes.append(page)
 
@@ -117,7 +121,7 @@ class PageTestResults(object):
       logging.error('Failed pages:\n%s', '\n'.join(
           p.display_name for p in self.pages_that_had_failures))
 
-    if self.skipped:
+    if self.skipped_values:
       logging.warning('Skipped pages:\n%s', '\n'.join(
           p.display_name for p in zip(*self.skipped)[0]))