rebaseline_server: allow live queries to examine JSONKEY_EXPECTEDRESULTS or JSONKEY_ACTUALRESULTS

See https://goto.google.com/CorrectnessTestingBigIssues for full context

BUG=skia:1942
NOTRY=true
R=rmistry@google.com

Author: epoger@google.com

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

7 files changed, 98 insertions, 49 deletions

diff --git a/gm/rebaseline_server/compare_rendered_pictures.py b/gm/rebaseline_server/compare_rendered_pictures.py
index 4f8f6cd076..f472be2681 100755
--- a/gm/rebaseline_server/compare_rendered_pictures.py
+++ b/gm/rebaseline_server/compare_rendered_pictures.py
@@ -61,16 +61,24 @@ REPO_URL_PREFIX = 'repo:'
 REPO_BASEPATH = os.path.abspath(os.path.join(
     os.path.dirname(os.path.abspath(__file__)), os.pardir, os.pardir))
 
+# Which sections within a JSON summary file can contain results.
+ALLOWED_SECTION_NAMES = [
+    gm_json.JSONKEY_ACTUALRESULTS,
+    gm_json.JSONKEY_EXPECTEDRESULTS,
+]
+
 
 class RenderedPicturesComparisons(results.BaseComparisons):
   """Loads results from multiple render_pictures runs into an ImagePairSet.
   """
 
-  def __init__(self, setA_dirs, setB_dirs, image_diff_db,
-               image_base_gs_url=DEFAULT_IMAGE_BASE_GS_URL,
-               diff_base_url=None, setA_label='setA',
-               setB_label='setB', gs=None,
-               truncate_results=False, prefetch_only=False,
+  def __init__(self,
+               setA_dirs, setB_dirs,
+               setA_section, setB_section,
+               image_diff_db,
+               image_base_gs_url=DEFAULT_IMAGE_BASE_GS_URL, diff_base_url=None,
+               setA_label='setA', setB_label='setB',
+               gs=None, truncate_results=False, prefetch_only=False,
                download_all_images=False):
     """Constructor: downloads images and generates diffs.
 
@@ -86,6 +94,10 @@ class RenderedPicturesComparisons(results.BaseComparisons):
       setB_dirs: list of root directories to copy all JSON summaries from,
           and to use as setB within the comparisons. These directories may be
           gs:// URLs, special "repo:" URLs, or local filepaths.
+      setA_section: which section within setA to examine; must be one of
+          ALLOWED_SECTION_NAMES
+      setB_section: which section within setB to examine; must be one of
+          ALLOWED_SECTION_NAMES
       image_diff_db: ImageDiffDB instance
       image_base_gs_url: "gs://" URL pointing at the Google Storage bucket/dir
           under which all render_pictures result images can
@@ -132,12 +144,9 @@ class RenderedPicturesComparisons(results.BaseComparisons):
         self._copy_dir_contents(source_dir=source_dir, dest_dir=setB_root)
 
       time_start = int(time.time())
-      # TODO(epoger): For now, this assumes that we are always comparing two
-      # sets of actual results, not actuals vs expectations.  Allow the user
-      # to control this.
       self._results = self._load_result_pairs(
-          setA_root=setA_root, setA_section=gm_json.JSONKEY_ACTUALRESULTS,
-          setB_root=setB_root, setB_section=gm_json.JSONKEY_ACTUALRESULTS)
+          setA_root=setA_root, setA_section=setA_section,
+          setB_root=setB_root, setB_section=setB_section)
       if self._results:
         self._timestamp = int(time.time())
         logging.info('Number of download file collisions: %s' %
@@ -231,20 +240,21 @@ class RenderedPicturesComparisons(results.BaseComparisons):
             source_skp_name=skp_name, tilenum=None))
 
         tiled_images_A = self.get_default(
-            dictA_results, None,
+            dictA_results, [],
             skp_name, gm_json.JSONKEY_SOURCE_TILEDIMAGES)
         tiled_images_B = self.get_default(
-            dictB_results, None,
+            dictB_results, [],
             skp_name, gm_json.JSONKEY_SOURCE_TILEDIMAGES)
-        # TODO(epoger): Report an error if we find tiles for A but not B?
-        if tiled_images_A and tiled_images_B:
-          # TODO(epoger): Report an error if we find a different number of tiles
-          # for A and B?
-          num_tiles = len(tiled_images_A)
+        if tiled_images_A or tiled_images_B:
+          num_tiles_A = len(tiled_images_A)
+          num_tiles_B = len(tiled_images_B)
+          num_tiles = max(num_tiles_A, num_tiles_B)
           for tile_num in range(num_tiles):
             imagepairs_for_this_skp.append(self._create_image_pair(
-                image_dict_A=tiled_images_A[tile_num],
-                image_dict_B=tiled_images_B[tile_num],
+                image_dict_A=(tiled_images_A[tile_num]
+                              if tile_num < num_tiles_A else None),
+                image_dict_B=(tiled_images_B[tile_num]
+                              if tile_num < num_tiles_B else None),
                 source_skp_name=skp_name, tilenum=tile_num))
 
         for one_imagepair in imagepairs_for_this_skp:
diff --git a/gm/rebaseline_server/compare_rendered_pictures_test.py b/gm/rebaseline_server/compare_rendered_pictures_test.py
index c6e0e79c2b..e761c8a5c2 100755
--- a/gm/rebaseline_server/compare_rendered_pictures_test.py
+++ b/gm/rebaseline_server/compare_rendered_pictures_test.py
@@ -20,6 +20,7 @@ within self._output_dir_expected, which wouldn't be good...
 
 # System-level imports
 import os
+import posixpath
 import subprocess
 
 # Must fix up PYTHONPATH before importing from within Skia
@@ -57,6 +58,8 @@ class CompareRenderedPicturesTest(base_unittest.TestCase):
     results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
         setA_dirs=[os.path.join(self.temp_dir, setA_label)],
         setB_dirs=[os.path.join(self.temp_dir, setB_label)],
+        setA_section=gm_json.JSONKEY_ACTUALRESULTS,
+        setB_section=gm_json.JSONKEY_ACTUALRESULTS,
         image_diff_db=imagediffdb.ImageDiffDB(self.temp_dir),
         image_base_gs_url='gs://fakebucket/fake/path',
         diff_base_url='/static/generated-images',
@@ -70,11 +73,12 @@ class CompareRenderedPicturesTest(base_unittest.TestCase):
 
   def test_repo_url(self):
     """Use repo: URL to specify summary files."""
+    base_repo_url = 'repo:gm/rebaseline_server/testdata/inputs/skp-summaries'
     results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
-        setA_dirs=[
-            'repo:gm/rebaseline_server/testdata/inputs/skp-actuals/setA'],
-        setB_dirs=[
-            'repo:gm/rebaseline_server/testdata/inputs/skp-actuals/setB'],
+        setA_dirs=[posixpath.join(base_repo_url, 'expectations')],
+        setB_dirs=[posixpath.join(base_repo_url, 'actuals')],
+        setA_section=gm_json.JSONKEY_EXPECTEDRESULTS,
+        setB_section=gm_json.JSONKEY_ACTUALRESULTS,
         image_diff_db=imagediffdb.ImageDiffDB(self.temp_dir),
         image_base_gs_url='gs://fakebucket/fake/path',
         diff_base_url='/static/generated-images',
diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py
index 03bbe74b01..8f59eb6895 100755
--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@@ -88,12 +88,12 @@ GET__PRECOMPUTED_RESULTS = 'results'
 GET__PREFETCH_RESULTS = 'prefetch'
 GET__STATIC_CONTENTS = 'static'
 
-# Parameters we use within do_GET_live_results()
+# Parameters we use within do_GET_live_results() and do_GET_prefetch_results()
+LIVE_PARAM__DOWNLOAD_ONLY_DIFFERING = 'downloadOnlyDifferingImages'
 LIVE_PARAM__SET_A_DIR = 'setADir'
+LIVE_PARAM__SET_A_SECTION = 'setASection'
 LIVE_PARAM__SET_B_DIR = 'setBDir'
-
-# Parameters we use within do_GET_prefetch_results()
-PREFETCH_PARAM__DOWNLOAD_ONLY_DIFFERING = 'downloadOnlyDifferingImages'
+LIVE_PARAM__SET_B_SECTION = 'setBSection'
 
 # How often (in seconds) clients should reload while waiting for initial
 # results to load.
@@ -533,12 +533,8 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     """
     logging.debug('do_GET_live_results: url_remainder="%s"' % url_remainder)
     param_dict = urlparse.parse_qs(url_remainder)
-    results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
-        setA_dirs=param_dict[LIVE_PARAM__SET_A_DIR],
-        setB_dirs=param_dict[LIVE_PARAM__SET_B_DIR],
-        image_diff_db=_SERVER.image_diff_db,
-        diff_base_url='/static/generated-images',
-        gs=_SERVER.gs, truncate_results=_SERVER.truncate_results)
+    results_obj = self._call_compare_rendered_pictures(
+        param_dict=param_dict, prefetch_only=False)
     self.send_json_dict(results_obj.get_packaged_results_of_type(
         results_mod.KEY__HEADER__RESULTS_ALL))
 
@@ -550,16 +546,8 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     """
     logging.debug('do_GET_prefetch_results: url_remainder="%s"' % url_remainder)
     param_dict = urlparse.parse_qs(url_remainder)
-    download_all_images = (
-        param_dict.get(PREFETCH_PARAM__DOWNLOAD_ONLY_DIFFERING, [''])[0].lower()
-        not in ['1', 'true'])
-    compare_rendered_pictures.RenderedPicturesComparisons(
-        setA_dirs=param_dict[LIVE_PARAM__SET_A_DIR],
-        setB_dirs=param_dict[LIVE_PARAM__SET_B_DIR],
-        image_diff_db=_SERVER.image_diff_db,
-        diff_base_url='/static/generated-images',
-        gs=_SERVER.gs, truncate_results=_SERVER.truncate_results,
-        prefetch_only=True, download_all_images=download_all_images)
+    self._call_compare_rendered_pictures(
+        param_dict=param_dict, prefetch_only=True)
     self.send_response(200)
 
   def do_GET_static(self, path):
@@ -707,6 +695,47 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     self.end_headers()
     json.dump(json_dict, self.wfile)
 
+  def _call_compare_rendered_pictures(self, param_dict, prefetch_only):
+    """Instantiates RenderedPicturesComparisons object to serve a GET request.
+
+    Args:
+      param_dict: dictionary of URL parameters
+      prefetch_only: parameter to pass into RenderedPicturesComparisons
+          constructor
+
+    Returns: a reference to the new RenderedPicturesComparisons object.
+    """
+    download_all_images = (
+        param_dict.get(LIVE_PARAM__DOWNLOAD_ONLY_DIFFERING, [''])[0].lower()
+        not in ['1', 'true'])
+    setA_section = self._validate_summary_section(
+        param_dict.get(LIVE_PARAM__SET_A_SECTION, [None])[0])
+    setB_section = self._validate_summary_section(
+        param_dict.get(LIVE_PARAM__SET_B_SECTION, [None])[0])
+    return compare_rendered_pictures.RenderedPicturesComparisons(
+        setA_dirs=param_dict[LIVE_PARAM__SET_A_DIR],
+        setB_dirs=param_dict[LIVE_PARAM__SET_B_DIR],
+        setA_section=setA_section, setB_section=setB_section,
+        image_diff_db=_SERVER.image_diff_db,
+        diff_base_url='/static/generated-images',
+        gs=_SERVER.gs, truncate_results=_SERVER.truncate_results,
+        prefetch_only=prefetch_only, download_all_images=download_all_images)
+
+  def _validate_summary_section(self, section_name):
+    """Validates the section we have been requested to read within JSON summary.
+
+    Args:
+      section_name: which section of the JSON summary file has been requested
+
+    Returns: the validated section name
+
+    Raises: Exception if an invalid section_name was requested.
+    """
+    if section_name not in compare_rendered_pictures.ALLOWED_SECTION_NAMES:
+      raise Exception('requested section name "%s" not in allowed list %s' % (
+          section_name, compare_rendered_pictures.ALLOWED_SECTION_NAMES))
+    return section_name
+
 
 def main():
   logging.basicConfig(format='%(asctime)s %(levelname)s %(message)s',
diff --git a/gm/rebaseline_server/static/live-loader.js b/gm/rebaseline_server/static/live-loader.js
index f171ff0fe9..00800682f3 100644
--- a/gm/rebaseline_server/static/live-loader.js
+++ b/gm/rebaseline_server/static/live-loader.js
@@ -128,7 +128,9 @@ Loader.controller(
     $scope.constants = constants;
     $scope.windowTitle = "Loading GM Results...";
     $scope.setADir = $location.search().setADir;
+    $scope.setASection = $location.search().setASection;
     $scope.setBDir = $location.search().setBDir;
+    $scope.setBSection = $location.search().setBSection;
     $scope.loadingMessage = "please wait...";
 
     /**
@@ -138,7 +140,9 @@ Loader.controller(
      */
     var liveQueryUrl =
        "/live-results/setADir=" + encodeURIComponent($scope.setADir) +
-       "&setBDir=" + encodeURIComponent($scope.setBDir);
+       "&setASection=" + encodeURIComponent($scope.setASection) +
+       "&setBDir=" + encodeURIComponent($scope.setBDir) +
+       "&setBSection=" + encodeURIComponent($scope.setBSection);
     $http.get(liveQueryUrl).success(
       function(data, status, header, config) {
         var dataHeader = data[constants.KEY__ROOT__HEADER];
@@ -252,7 +256,9 @@ Loader.controller(
           // parameter name -> copier object to load/save parameter value
           $scope.queryParameters.map = {
             'setADir':               $scope.queryParameters.copiers.simple,
+            'setASection':           $scope.queryParameters.copiers.simple,
             'setBDir':               $scope.queryParameters.copiers.simple,
+            'setBSection':           $scope.queryParameters.copiers.simple,
             'displayLimitPending':   $scope.queryParameters.copiers.simple,
             'showThumbnailsPending': $scope.queryParameters.copiers.simple,
             'mergeIdenticalRowsPending': $scope.queryParameters.copiers.simple,
diff --git a/gm/rebaseline_server/static/live-view.html b/gm/rebaseline_server/static/live-view.html
index b19008cec9..5292f3b966 100644
--- a/gm/rebaseline_server/static/live-view.html
+++ b/gm/rebaseline_server/static/live-view.html
@@ -24,8 +24,8 @@
   <em ng-show="!readyToDisplay">
     Loading results of query:
     <ul>
-      <li>setA: {{setADir}}</li>
-      <li>setB: {{setBDir}}</li>
+      <li>setA: "{{setASection}}" within {{setADir}}</li>
+      <li>setB: "{{setBSection}}" within {{setBDir}}</li>
     </ul>
     <br>
     {{loadingMessage}}
@@ -48,8 +48,8 @@
     </div>
 
     <div ng-show="header[constants.KEY__HEADER__TIME_UPDATED]">
-      setA: {{setADir}}<br>
-      setB: {{setBDir}}<br>
+      setA: "{{setASection}}" within {{setADir}}<br>
+      setB: "{{setBSection}}" within {{setBDir}}<br>
       These results current as of
       {{localTimeString(header[constants.KEY__HEADER__TIME_UPDATED])}}
     </div>
diff --git a/gm/rebaseline_server/testdata/inputs/skp-actuals/setB/summary.json b/gm/rebaseline_server/testdata/inputs/skp-summaries/actuals/summary.json
index 67e8409117..67e8409117 100644
--- a/gm/rebaseline_server/testdata/inputs/skp-actuals/setB/summary.json
+++ b/gm/rebaseline_server/testdata/inputs/skp-summaries/actuals/summary.json
diff --git a/gm/rebaseline_server/testdata/inputs/skp-actuals/setA/summary.json b/gm/rebaseline_server/testdata/inputs/skp-summaries/expectations/summary.json
index d74010746f..9d64d75365 100644
--- a/gm/rebaseline_server/testdata/inputs/skp-actuals/setA/summary.json
+++ b/gm/rebaseline_server/testdata/inputs/skp-summaries/expectations/summary.json
@@ -1,5 +1,5 @@
 {
-   "actual-results" : {
+   "expected-results" : {
       "changed.skp" : {
          "whole-image" : {
             "checksumAlgorithm" : "bitmap-64bitMD5",