rebaseline.py: add --keep-going-on-failure option, off by default

R=borenet@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@10109 2bbb7eff-a529-9590-31e7-b0007b416f81

3 files changed, 67 insertions, 12 deletions

diff --git a/tools/rebaseline.py b/tools/rebaseline.py
index 1abc1dc91c..ec5c1c99c2 100755
--- a/tools/rebaseline.py
+++ b/tools/rebaseline.py
@@ -74,6 +74,49 @@ SUBDIR_MAPPING = {
 class _InternalException(Exception):
     pass
 
+# Object that handles exceptions, either raising them immediately or collecting
+# them to display later on.
+class ExceptionHandler(object):
+
+    # params:
+    #  keep_going_on_failure: if False, report failures and quit right away;
+    #                         if True, collect failures until
+    #                         ReportAllFailures() is called
+    def __init__(self, keep_going_on_failure=False):
+        self._keep_going_on_failure = keep_going_on_failure
+        self._failures_encountered = []
+        self._exiting = False
+
+    # Exit the program with the given status value.
+    def _Exit(self, status=1):
+        self._exiting = True
+        sys.exit(status)
+
+    # We have encountered an exception; either collect the info and keep going,
+    # or exit the program right away.
+    def RaiseExceptionOrContinue(self, e):
+        # If we are already quitting the program, propagate any exceptions
+        # so that the proper exit status will be communicated to the shell.
+        if self._exiting:
+            raise e
+
+        if self._keep_going_on_failure:
+            print >> sys.stderr, 'WARNING: swallowing exception %s' % e
+            self._failures_encountered.append(e)
+        else:
+            print >> sys.stderr, e
+            print >> sys.stderr, (
+                'Halting at first exception; to keep going, re-run ' +
+                'with the --keep-going-on-failure option set.')
+            self._Exit()
+
+    def ReportAllFailures(self):
+        if self._failures_encountered:
+            print >> sys.stderr, ('Encountered %d failures (see above).' %
+                                  len(self._failures_encountered))
+            self._Exit()
+
+
 # Object that rebaselines a JSON expectations file (not individual image files).
 class JsonRebaseliner(object):
 
@@ -85,6 +128,7 @@ class JsonRebaseliner(object):
     #  actuals_base_url: base URL from which to read actual-result JSON files
     #  actuals_filename: filename (under actuals_base_url) from which to read a
     #                    summary of results; typically "actual-results.json"
+    #  exception_handler: reference to rebaseline.ExceptionHandler object
     #  tests: list of tests to rebaseline, or None if we should rebaseline
     #         whatever files the JSON results summary file tells us to
     #  configs: which configs to run for each test, or None if we should
@@ -92,7 +136,7 @@ class JsonRebaseliner(object):
     #           us to
     #  add_new: if True, add expectations for tests which don't have any yet
     def __init__(self, expectations_root, expectations_filename,
-                 actuals_base_url, actuals_filename,
+                 actuals_base_url, actuals_filename, exception_handler,
                  tests=None, configs=None, add_new=False):
         self._expectations_root = expectations_root
         self._expectations_filename = expectations_filename
@@ -100,6 +144,7 @@ class JsonRebaseliner(object):
         self._configs = configs
         self._actuals_base_url = actuals_base_url
         self._actuals_filename = actuals_filename
+        self._exception_handler = exception_handler
         self._add_new = add_new
         self._testname_pattern = re.compile('(\S+)_(\S+).png')
 
@@ -243,6 +288,10 @@ parser.add_argument('--expectations-root',
                     'contain one or more base-* subdirectories. Defaults to ' +
                     '%(default)s',
                     default='.')
+parser.add_argument('--keep-going-on-failure', action='store_true',
+                    help='instead of halting at the first error encountered, ' +
+                    'keep going and rebaseline as many tests as possible, ' +
+                    'and then report the full set of errors at the end')
 parser.add_argument('--subdirs', metavar='SUBDIR', nargs='+',
                     help='which platform subdirectories to rebaseline; ' +
                     'if unspecified, rebaseline all subdirs, same as ' +
@@ -254,6 +303,8 @@ parser.add_argument('--tests', metavar='TEST', nargs='+',
                     'set of results in ACTUALS_FILENAME; if unspecified, ' +
                     'rebaseline *all* tests that are available.')
 args = parser.parse_args()
+exception_handler = ExceptionHandler(
+    keep_going_on_failure=args.keep_going_on_failure)
 if args.subdirs:
     subdirs = args.subdirs
     missing_json_is_fatal = True
@@ -283,6 +334,7 @@ for subdir in subdirs:
             tests=args.tests, configs=args.configs,
             actuals_base_url=args.actuals_base_url,
             actuals_filename=args.actuals_filename,
+            exception_handler=exception_handler,
             add_new=args.add_new)
     else:
         # TODO(epoger): When we get rid of the ImageRebaseliner implementation,
@@ -297,10 +349,13 @@ for subdir in subdirs:
             dry_run=args.dry_run,
             json_base_url=args.actuals_base_url,
             json_filename=args.actuals_filename,
+            exception_handler=exception_handler,
             add_new=args.add_new,
             missing_json_is_fatal=missing_json_is_fatal)
+
     try:
         rebaseliner.RebaselineSubdir(subdir=subdir, builder=builder)
     except BaseException as e:
-        print >> sys.stderr, e
-        sys.exit(1)
+        exception_handler.RaiseExceptionOrContinue(e)
+
+exception_handler.ReportAllFailures()
diff --git a/tools/rebaseline_imagefiles.py b/tools/rebaseline_imagefiles.py
index da3d87cf37..38594ba1d5 100755
--- a/tools/rebaseline_imagefiles.py
+++ b/tools/rebaseline_imagefiles.py
@@ -53,6 +53,7 @@ class ImageRebaseliner(object):
     #  json_base_url: base URL from which to read json_filename
     #  json_filename: filename (under json_base_url) from which to read a
     #                 summary of results; typically "actual-results.json"
+    #  exception_handler: reference to rebaseline.ExceptionHandler object
     #  tests: list of tests to rebaseline, or None if we should rebaseline
     #         whatever files the JSON results summary file tells us to
     #  configs: which configs to run for each test, or None if we should
@@ -65,13 +66,14 @@ class ImageRebaseliner(object):
     #  missing_json_is_fatal: whether to halt execution if we cannot read a
     #                         JSON actual result summary file
     def __init__(self, expectations_root, json_base_url, json_filename,
-                 tests=None, configs=None, dry_run=False,
+                 exception_handler, tests=None, configs=None, dry_run=False,
                  add_new=False, missing_json_is_fatal=False):
         self._expectations_root = expectations_root
         self._tests = tests
         self._configs = configs
         self._json_base_url = json_base_url
         self._json_filename = json_filename
+        self._exception_handler = exception_handler
         self._dry_run = dry_run
         self._add_new = add_new
         self._missing_json_is_fatal = missing_json_is_fatal
@@ -254,10 +256,11 @@ class ImageRebaseliner(object):
     #  builder : e.g. 'Test-Win7-ShuttleA-HD2000-x86-Release'
     def RebaselineSubdir(self, subdir, builder):
         if not os.path.isdir(os.path.join(self._expectations_root, subdir)):
-            raise Exception((
+            self._exception_handler.RaiseExceptionOrContinue(Exception((
                 'Could not find "%s" subdir within expectations_root "%s".  ' +
                 'Are you sure --expectations-root is pointing at a valid ' +
-                'gm-expected directory?') % (subdir, self._expectations_root))
+                'gm-expected directory?') % (subdir, self._expectations_root)))
+            return
 
         json_url = '/'.join([self._json_base_url,
                              subdir, builder, subdir,
@@ -278,15 +281,11 @@ class ImageRebaseliner(object):
                     continue
             outfilename = os.path.join(self._expectations_root, subdir,
                                        filename);
-            # TODO(epoger): Until we resolve
-            # https://code.google.com/p/skia/issues/detail?id=1410 ('some GM
-            # result images not available for download from Google Storage'),
-            # keep going in the face of missing results for any one test.
             try:
                 self._RebaselineOneFile(expectations_subdir=subdir,
                                         builder_name=builder,
                                         infilename=filename,
                                         outfilename=outfilename,
                                         all_results=all_results)
-            except Exception as e:
-                print 'WARNING: swallowing exception %s' % e
+            except BaseException as e:
+                self._exception_handler.RaiseExceptionOrContinue(e)
diff --git a/tools/tests/rebaseline/output/exercise-bug1403/output-expected/stdout b/tools/tests/rebaseline/output/exercise-bug1403/output-expected/stdout
index dab8d45459..04d976411a 100644
--- a/tools/tests/rebaseline/output/exercise-bug1403/output-expected/stdout
+++ b/tools/tests/rebaseline/output/exercise-bug1403/output-expected/stdout
@@ -1 +1,2 @@
 Could not find "base-android-galaxy-nexus" subdir within expectations_root "tools/tests/rebaseline/input".  Are you sure --expectations-root is pointing at a valid gm-expected directory?
+Halting at first exception; to keep going, re-run with the --keep-going-on-failure option set.