Better error messages on archive failures, and stop running with partial page...

Better error messages on archive failures, and stop running with partial page sets in user_story_runner.

BUG=435063

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

Cr-Commit-Position: refs/heads/master@{#307074}

tools/telemetry/telemetry/user_story/user_story_runner.py

@@ -201,7 +201,10 @@ def Run(test, user_story_set, expectations, finder_options, results):
       # patch.
       isinstance(user_story_set, page_set_module.PageSet)):
     _UpdateUserStoryArchivesIfChanged(user_story_set)
-    user_stories = _CheckArchives(user_story_set, user_stories, results)
+    if not _CheckArchives(
+        user_story_set.archive_data_file, user_story_set.wpr_archive_info,
+        user_story_set.pages):
+      return
 
   for user_story in list(user_stories):
     if not test.CanRunForPage(user_story):
@@ -275,51 +278,57 @@ def _ShuffleAndFilterUserStorySet(user_story_set, finder_options):
   return user_stories
 
 
-def _CheckArchives(page_set, pages, results):
-  """Returns a subset of pages that are local or have WPR archives.
+def _CheckArchives(archive_data_file, wpr_archive_info, pages):
+  """Verifies that all pages are local or have WPR archives.
 
-  Logs warnings if any are missing.
+  Logs warnings and returns False if any are missing.
   """
-  # Warn of any problems with the entire page set.
+  # Report any problems with the entire page set.
   if any(not p.is_local for p in pages):
-    if not page_set.archive_data_file:
-      logging.warning('The page set is missing an "archive_data_file" '
-                      'property. Skipping any live sites. To include them, '
-                      'pass the flag --use-live-sites.')
-    if not page_set.wpr_archive_info:
-      logging.warning('The archive info file is missing. '
+    if not archive_data_file:
+      logging.error('The page set is missing an "archive_data_file" '
+                    'property.\nTo run from live sites pass the flag '
+                    '--use-live-sites.\nTo create an archive file add an '
+                    'archive_data_file property to the page set and then '
+                    'run record_wpr.')
+      return False
+    if not wpr_archive_info:
+      logging.error('The archive info file is missing.\n'
                     'To fix this, either add svn-internal to your '
                     '.gclient using http://goto/read-src-internal, '
                     'or create a new archive using record_wpr.')
+      return False
 
-  # Warn of any problems with individual pages and return valid pages.
+  # Report any problems with individual pages.
   pages_missing_archive_path = []
   pages_missing_archive_data = []
-  valid_pages = []
   for page in pages:
     if not page.is_local and not page.archive_path:
       pages_missing_archive_path.append(page)
     elif not page.is_local and not os.path.isfile(page.archive_path):
       pages_missing_archive_data.append(page)
-    else:
-      valid_pages.append(page)
   if pages_missing_archive_path:
-    logging.warning('The page set archives for some pages do not exist. '
-                    'Skipping those pages. To fix this, record those pages '
-                    'using record_wpr. To ignore this warning and run '
-                    'against live sites, pass the flag --use-live-sites.')
+    logging.error('The page set archives for some pages do not exist.\n'
+                  'To fix this, record those pages using record_wpr.\n'
+                  'To ignore this warning and run against live sites, '
+                  'pass the flag --use-live-sites.')
+    logging.error(
+        'Pages without archives: %s',
+        ', '.join(page.display_name for page in pages_missing_archive_path))
   if pages_missing_archive_data:
-    logging.warning('The page set archives for some pages are missing. '
-                    'Someone forgot to check them in, or they were deleted. '
-                    'Skipping those pages. To fix this, record those pages '
-                    'using record_wpr. To ignore this warning and run '
-                    'against live sites, pass the flag --use-live-sites.')
-  for page in pages_missing_archive_path + pages_missing_archive_data:
-    results.WillRunPage(page)
-    results.AddValue(failure.FailureValue.FromMessage(
-        page, 'Page set archive doesn\'t exist.'))
-    results.DidRunPage(page)
-  return valid_pages
+    logging.error('The page set archives for some pages are missing.\n'
+                  'Someone forgot to check them in, uploaded them to the '
+                  'wrong cloud storage bucket, or they were deleted.\n'
+                  'To fix this, record those pages using record_wpr.\n'
+                  'To ignore this warning and run against live sites, '
+                  'pass the flag --use-live-sites.')
+    logging.error(
+        'Pages missing archives: %s',
+        ', '.join(page.display_name for page in pages_missing_archive_data))
+  if pages_missing_archive_path or pages_missing_archive_data:
+    return False
+  # Only run valid pages if no problems with the page set or individual pages.
+  return True
 
 
 def _WaitForThermalThrottlingIfNeeded(platform):

tools/telemetry/telemetry/user_story/user_story_runner_unittest.py

@@ -9,9 +9,11 @@ import sys
 from telemetry import benchmark
 from telemetry import user_story
 from telemetry.core import exceptions
+from telemetry.page import page_set
 from telemetry.page import page_test
 from telemetry.page import test_expectations
 from telemetry.results import results_options
+from unittest_data import test_simple_one_page_set
 from telemetry.unittest_util import options_for_unittests
 from telemetry.unittest_util import system_stub
 from telemetry.user_story import shared_user_story_state
@@ -340,3 +342,37 @@ class UserStoryRunnerTest(unittest.TestCase):
       self.assertIn('*RESULT metric: metric= [1,2,3,4] unit', contents)
     finally:
       sys.stdout = real_stdout
+
+  def testCheckArchives(self):
+    ps = page_set.PageSet()
+    ps.AddPageWithDefaultRunNavigate('http://www.testurl.com')
+    # Page set missing archive_data_file.
+    self.assertFalse(user_story_runner._CheckArchives(
+        ps.archive_data_file, ps.wpr_archive_info, ps.pages))
+
+    ps = page_set.PageSet(archive_data_file='missing_archive_data_file.json')
+    ps.AddPageWithDefaultRunNavigate('http://www.testurl.com')
+    # Page set missing json file specified in archive_data_file.
+    self.assertFalse(user_story_runner._CheckArchives(
+       ps.archive_data_file, ps.wpr_archive_info, ps.pages))
+
+    ps = page_set.PageSet(archive_data_file='../../unittest_data/test.json',
+                          bucket=page_set.PUBLIC_BUCKET)
+    ps.AddPageWithDefaultRunNavigate('http://www.testurl.com')
+    # Page set with valid archive_data_file.
+    self.assertTrue(user_story_runner._CheckArchives(
+        ps.archive_data_file, ps.wpr_archive_info, ps.pages))
+    ps.AddPageWithDefaultRunNavigate('http://www.google.com')
+    # Page set with an archive_data_file which exists but is missing a page.
+    self.assertFalse(user_story_runner._CheckArchives(
+        ps.archive_data_file, ps.wpr_archive_info, ps.pages))
+
+    ps = page_set.PageSet(
+        archive_data_file='../../unittest_data/test_missing_wpr_file.json',
+        bucket=page_set.PUBLIC_BUCKET)
+    ps.AddPageWithDefaultRunNavigate('http://www.testurl.com')
+    ps.AddPageWithDefaultRunNavigate('http://www.google.com')
+    # Page set with an archive_data_file which exists and contains all pages
+    # but fails to find a wpr file.
+    self.assertFalse(user_story_runner._CheckArchives(
+        ps.archive_data_file, ps.wpr_archive_info, ps.pages))

tools/telemetry/unittest_data/test.json

+{
+    "description": "Describes the Web Page Replay archives for a page set. Don't edit by hand! Use record_wpr for updating.", 
+    "archives": {
+        "test_000.wpr": [
+            "http://www.testurl.com"
+        ]
+    }
+}

tools/telemetry/unittest_data/test_000.wpr

+File needed for user_story_runner_unittests testCheckArchives.

tools/telemetry/unittest_data/test_000.wpr.sha1

+6bd6f4b08b44abc206f68471c0444df156d0fd1a
\ No newline at end of file

tools/telemetry/unittest_data/test_missing_wpr_file.json

+{
+    "description": "Describes the Web Page Replay archives for a page set. Don't edit by hand! Use record_wpr for updating.", 
+    "archives": {
+        "test_000.wpr": [
+            "http://www.testurl.com"
+        ],
+        "test_missing_wpr_file.wpr": [
+            "http://www.google.com"
+        ]
+    }
+}