Add ability to rebaseline skimage to rebaseline.py

tools/rebaseline.py

 #!/usr/bin/python
 
 '''
 Copyright 2012 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 '''
 
 '''
@@ skipping 159 matching lines @@
   #                   Currently, there is no way to make this script mark
   #                   expectations as reviewed-by-human=True.
   #                   TODO(epoger): Add that capability to a review tool.
   #  mark_ignore_failure: if True, mark failures of a given test as being
   #                       ignored.
   def __init__(self, expectations_root, expectations_input_filename,
                expectations_output_filename, actuals_base_url,
                actuals_filename, exception_handler,
                tests=None, configs=None, add_new=False, bugs=None, notes=None,
                mark_unreviewed=None, mark_ignore_failure=False,
-               from_trybot=False):
+               from_trybot=False, skip_pattern_matching=False):

[epoger, 2013/10/10 15:05:09]

please update the docstring above

[scroggo, 2013/10/10 19:32:36]

Done.

     self._expectations_root = expectations_root
     self._expectations_input_filename = expectations_input_filename
     self._expectations_output_filename = expectations_output_filename
     self._tests = tests
     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._bugs = bugs
     self._notes = notes
     self._mark_unreviewed = mark_unreviewed
     self._mark_ignore_failure = mark_ignore_failure;
-    self._image_filename_re = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
+    # TODO(scroggo): This is a hack. Since the filenames will not match
+    # the pattern in skimage, we skip the pattern matching entirely.
+    if skip_pattern_matching:
+      self._image_filename_re = None
+    else:
+      self._image_filename_re = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
     self._using_svn = os.path.isdir(os.path.join(expectations_root, '.svn'))
     self._from_trybot = from_trybot
 
   # Executes subprocess.call(cmd).
   # Raises an Exception if the command fails.
   def _Call(self, cmd):
     if subprocess.call(cmd) != 0:
       raise _InternalException('error running command: ' + ' '.join(cmd))
 
   # Returns the full contents of filepath, as a single string.
   # If filepath looks like a URL, try to read it that way instead of as
   # a path on local storage.
   #
   # Raises _InternalException if there is a problem.
   def _GetFileContents(self, filepath):
     if filepath.startswith('http:') or filepath.startswith('https:'):
       try:
         return urllib2.urlopen(filepath).read()
       except urllib2.HTTPError as e:
         raise _InternalException('unable to read URL %s: %s' % (
             filepath, e))
+    elif filepath.startswith('gs:'):

[epoger, 2013/10/10 15:05:09]

As discussed live: I think it's better to download files from Google Storage
using HTTP instead of gsutil.  (Assuming that the files have been uploaded as
publicly readable)

1. no need to install gsutil package
2. no need for gsutil keys

[scroggo, 2013/10/10 19:32:36]

Done.

+      try:
+        return subprocess.check_output(['gsutil', 'cat', filepath])
+      except subprocess.CalledProcessError:
+        raise _InternalException('unable to read Google Storage URL %s: %s' % (
+            filepath, e))
     else:
       return open(filepath, 'r').read()
 
   # Returns a dictionary of actual results from actual-results.json file.
   #
   # The dictionary returned has this format:
   # {
   #  u'imageblur_565.png': [u'bitmap-64bitMD5', 3359963596899141322],
   #  u'imageblur_8888.png': [u'bitmap-64bitMD5', 4217923806027861152],
   #  u'shadertext3_8888.png': [u'bitmap-64bitMD5', 3713708307125704716]
@@ skipping 60 matching lines @@
     expected_results = expectations_dict.get(gm_json.JSONKEY_EXPECTEDRESULTS)
     if not expected_results:
       expected_results = {}
       expectations_dict[gm_json.JSONKEY_EXPECTEDRESULTS] = expected_results
 
     # Update the expectations in memory, skipping any tests/configs that
     # the caller asked to exclude.
     skipped_images = []
     if results_to_update:
       for (image_name, image_results) in results_to_update.iteritems():
-        (test, config) = self._image_filename_re.match(image_name).groups()
-        if self._tests:
-          if test not in self._tests:
-            skipped_images.append(image_name)
-            continue
-        if self._configs:
-          if config not in self._configs:
-            skipped_images.append(image_name)
-            continue
+        if self._image_filename_re:
+          (test, config) = self._image_filename_re.match(image_name).groups()
+          if self._tests:
+            if test not in self._tests:
+              skipped_images.append(image_name)
+              continue
+          if self._configs:
+            if config not in self._configs:
+              skipped_images.append(image_name)
+              continue
         if not expected_results.get(image_name):
           expected_results[image_name] = {}
         expected_results[image_name]\
                         [gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]\
                         = [image_results]
         if self._mark_unreviewed:
           expected_results[image_name]\
                           [gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED]\
                           = False
         if self._mark_ignore_failure:
@@ skipping 92 matching lines @@
                     help=('mark all expectations modified by this run as '
                           '"%s": False' %
                           gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED))
 parser.add_argument('--ignore-failure', action='store_true',
                     help=('mark all expectations modified by this run as '
                           '"%s": True' %
                           gm_json.JSONKEY_ACTUALRESULTS_FAILUREIGNORED))
 parser.add_argument('--from-trybot', action='store_true',
                     help=('pull the actual-results.json file from the '
                           'corresponding trybot, rather than the main builder'))
+parser.add_argument('--skimage', action='store_true',
+                    help=('Rebaseline skimage results instead of gm. Defaults '
+                          'to False.'))
 args = parser.parse_args()
 exception_handler = ExceptionHandler(
     keep_going_on_failure=args.keep_going_on_failure)
 if args.builders:
   builders = args.builders
   missing_json_is_fatal = True
 else:
   builders = sorted(TEST_BUILDERS)
   missing_json_is_fatal = False
+if args.skimage:
+  # Hardcode variables for skimage.
+  args.actuals_base_url = 'gs://chromium-skia-gm/skimage/actuals'

[epoger, 2013/10/10 15:05:09]

I think it will be surprising that these args are ignored, depending on the
settings of other args.

Maybe you could do one of these instead?
- fail if the user specifies conflicting args
OR
- if args.skimage has been set then use these as DEFAULTS instead of ignoring
any user-specified values

... and then note the behavior in the description of the affected args

[scroggo, 2013/10/10 19:32:36]

Done, for --tests and --configs.
For these, I have now set them to the skimage values ONLY if they are not
already set to the defaults.

One result of the current patch is that if the user specified:

./tools/rebaseline.py --skimage --actuals-base-url '<the real default value>' it
would get overwritten... (I wasn't sure how I could differentiate between 'user
left it unset, so it is set to the default' and 'user specified the default')
That said, this use case is illogical (rebaseline skimage using gm actual
results).
Done.

+  args.expectations_root = os.path.join('expectations', 'skimage')
 for builder in builders:
   if not builder in TEST_BUILDERS:
     raise Exception(('unrecognized builder "%s"; ' +
                      'should be one of %s') % (
                          builder, TEST_BUILDERS))
 
   expectations_json_file = os.path.join(args.expectations_root, builder,
                                         args.expectations_filename)
   if os.path.isfile(expectations_json_file):
     rebaseliner = JsonRebaseliner(
         expectations_root=args.expectations_root,
         expectations_input_filename=args.expectations_filename,
         expectations_output_filename=(args.expectations_filename_output or
                                       args.expectations_filename),
         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, bugs=args.bugs, notes=args.notes,
         mark_unreviewed=args.unreviewed,
         mark_ignore_failure=args.ignore_failure,
-        from_trybot=args.from_trybot)
+        from_trybot=args.from_trybot,
+        skip_pattern_matching=args.skimage)
     try:
       rebaseliner.RebaselineSubdir(builder=builder)
     except:
       exception_handler.RaiseExceptionOrContinue()
   else:
     try:
       raise _InternalException('expectations_json_file %s not found' %
                                expectations_json_file)
     except:
       exception_handler.RaiseExceptionOrContinue()
 
 exception_handler.ReportAllFailures()