rebaseline_server: allow client to pull all results, or just failures

(SkipBuildbotRuns)

This will be handy for constrained networks or devices, where we don't want
to bother downloading info about all the successful tests.

R=jcgregorio@google.com

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

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

4 files changed, 120 insertions, 81 deletions

diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py
index 84c45b943d..c0d31871cf 100755
--- a/gm/rebaseline_server/results.py
+++ b/gm/rebaseline_server/results.py
@@ -12,6 +12,7 @@ Repackage expected/actual GM results as needed by our HTML rebaseline viewer.
 # System-level imports
 import fnmatch
 import json
+import logging
 import os
 import re
 import sys
@@ -32,10 +33,16 @@ IMAGE_FILENAME_RE = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
 CATEGORIES_TO_SUMMARIZE = [
     'builder', 'test', 'config', 'resultType',
 ]
+RESULTS_ALL = 'all'
+RESULTS_FAILURES = 'failures'
 
 class Results(object):
   """ Loads actual and expected results from all builders, supplying combined
-  reports as requested. """
+  reports as requested.
+
+  Once this object has been constructed, the results are immutable.  If you
+  want to update the results based on updated JSON file contents, you will
+  need to create a new Results object."""
 
   def __init__(self, actuals_root, expected_root):
     """
@@ -43,14 +50,18 @@ class Results(object):
       actuals_root: root directory containing all actual-results.json files
       expected_root: root directory containing all expected-results.json files
     """
-    self._actual_builder_dicts = Results._GetDictsFromRoot(actuals_root)
-    self._expected_builder_dicts = Results._GetDictsFromRoot(expected_root)
-    self._all_results = Results._Combine(
-        actual_builder_dicts=self._actual_builder_dicts,
-        expected_builder_dicts=self._expected_builder_dicts)
+    self._actual_builder_dicts = Results._get_dicts_from_root(actuals_root)
+    self._expected_builder_dicts = Results._get_dicts_from_root(expected_root)
+    self._combine_actual_and_expected()
+
+  def get_results_of_type(self, type):
+    """Return results of some/all tests (depending on 'type' parameter).
+
+    Args:
+      type: string describing which types of results to include; must be one
+            of the RESULTS_* constants
 
-  def GetAll(self):
-    """Return results of all tests, as a dictionary in this form:
+    Results are returned as a dictionary in this form:
 
        {
          'categories': # dictionary of categories listed in
@@ -76,7 +87,6 @@ class Results(object):
          'testData': # list of test results, with a dictionary for each
          [
            {
-             'index': 0,   # index of this result within testData list
              'builder': 'Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug',
              'test': 'bigmatrix',
              'config': '8888',
@@ -90,10 +100,10 @@ class Results(object):
          ], # end of 'testData' list
        }
     """
-    return self._all_results
+    return self._results[type]
 
   @staticmethod
-  def _GetDictsFromRoot(root, pattern='*.json'):
+  def _get_dicts_from_root(root, pattern='*.json'):
     """Read all JSON dictionaries within a directory tree.
 
     Args:
@@ -114,37 +124,32 @@ class Results(object):
         meta_dict[builder] = gm_json.LoadFromFile(fullpath)
     return meta_dict
 
-  @staticmethod
-  def _Combine(actual_builder_dicts, expected_builder_dicts):
+  def _combine_actual_and_expected(self):
     """Gathers the results of all tests, across all builders (based on the
-    contents of actual_builder_dicts and expected_builder_dicts).
-
-    This is a static method, because once we start refreshing results
-    asynchronously, we need to make sure we are not corrupting the object's
-    member variables.
-
-    Args:
-      actual_builder_dicts: a meta-dictionary of all actual JSON results,
-          as returned by _GetDictsFromRoot().
-      actual_builder_dicts: a meta-dictionary of all expected JSON results,
-          as returned by _GetDictsFromRoot().
-
-    Returns:
-      A list of all the results of all tests, in the same form returned by
-      self.GetAll().
+    contents of self._actual_builder_dicts and self._expected_builder_dicts),
+    and stores them in self._results.
     """
-    test_data = []
-    category_dict = {}
-    Results._EnsureIncludedInCategoryDict(category_dict, 'resultType', [
+    categories_all = {}
+    categories_failures = {}
+    Results._ensure_included_in_category_dict(categories_all,
+                                              'resultType', [
         gm_json.JSONKEY_ACTUALRESULTS_FAILED,
         gm_json.JSONKEY_ACTUALRESULTS_FAILUREIGNORED,
         gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON,
         gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED,
         ])
+    Results._ensure_included_in_category_dict(categories_failures,
+                                              'resultType', [
+        gm_json.JSONKEY_ACTUALRESULTS_FAILED,
+        gm_json.JSONKEY_ACTUALRESULTS_FAILUREIGNORED,
+        gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON,
+        ])
 
-    for builder in sorted(actual_builder_dicts.keys()):
+    data_all = []
+    data_failures = []
+    for builder in sorted(self._actual_builder_dicts.keys()):
       actual_results_for_this_builder = (
-          actual_builder_dicts[builder][gm_json.JSONKEY_ACTUALRESULTS])
+          self._actual_builder_dicts[builder][gm_json.JSONKEY_ACTUALRESULTS])
       for result_type in sorted(actual_results_for_this_builder.keys()):
         results_of_this_type = actual_results_for_this_builder[result_type]
         if not results_of_this_type:
@@ -154,7 +159,7 @@ class Results(object):
           try:
             # TODO(epoger): assumes a single allowed digest per test
             expected_image = (
-                expected_builder_dicts
+                self._expected_builder_dicts
                     [builder][gm_json.JSONKEY_EXPECTEDRESULTS]
                     [image_name][gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]
                     [0])
@@ -186,11 +191,11 @@ class Results(object):
             if result_type not in [
                 gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON,
                 gm_json.JSONKEY_ACTUALRESULTS_FAILUREIGNORED] :
-              print 'WARNING: No expectations found for test: %s' % {
+              logging.warning('No expectations found for test: %s' % {
                   'builder': builder,
                   'image_name': image_name,
                   'result_type': result_type,
-                  }
+                  })
             expected_image = [None, None]
 
           # If this test was recently rebaselined, it will remain in
@@ -213,7 +218,6 @@ class Results(object):
 
           (test, config) = IMAGE_FILENAME_RE.match(image_name).groups()
           results_for_this_test = {
-              'index': len(test_data),
               'builder': builder,
               'test': test,
               'config': config,
@@ -223,14 +227,25 @@ class Results(object):
               'expectedHashType': expected_image[0],
               'expectedHashDigest': str(expected_image[1]),
           }
-          Results._AddToCategoryDict(category_dict, results_for_this_test)
-          test_data.append(results_for_this_test)
-    return {'categories': category_dict, 'testData': test_data}
+          Results._add_to_category_dict(categories_all, results_for_this_test)
+          data_all.append(results_for_this_test)
+          if updated_result_type != gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED:
+            Results._add_to_category_dict(categories_failures,
+                                       results_for_this_test)
+            data_failures.append(results_for_this_test)
+
+    self._results = {
+      RESULTS_ALL:
+        {'categories': categories_all, 'testData': data_all},
+      RESULTS_FAILURES:
+        {'categories': categories_failures, 'testData': data_failures},
+    }
 
   @staticmethod
-  def _AddToCategoryDict(category_dict, test_results):
+  def _add_to_category_dict(category_dict, test_results):
     """Add test_results to the category dictionary we are building.
-    (See documentation of self.GetAll() for the format of this dictionary.)
+    (See documentation of self.get_results_of_type() for the format of this
+    dictionary.)
 
     Args:
       category_dict: category dict-of-dicts to add to; modify this in-place
@@ -252,11 +267,12 @@ class Results(object):
       category_dict[category][category_value] += 1
 
   @staticmethod
-  def _EnsureIncludedInCategoryDict(category_dict,
-                                    category_name, category_values):
+  def _ensure_included_in_category_dict(category_dict,
+                                        category_name, category_values):
     """Ensure that the category name/value pairs are included in category_dict,
     even if there aren't any results with that name/value pair.
-    (See documentation of self.GetAll() for the format of this dictionary.)
+    (See documentation of self.get_results_of_type() for the format of this
+    dictionary.)
 
     Args:
       category_dict: category dict-of-dicts to modify
diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py
index 7b87d6f07c..bfc690b570 100755
--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@@ -13,11 +13,13 @@ HTTP server for our HTML rebaseline viewer.
 import argparse
 import BaseHTTPServer
 import json
+import logging
 import os
 import posixpath
 import re
 import shutil
 import sys
+import urlparse
 
 # Imports from within Skia
 #
@@ -91,16 +93,17 @@ class Server(object):
     the gm-actuals and expectations will automatically be updated every few
     minutes.  See discussion in https://codereview.chromium.org/24274003/ .
     """
-    print 'Checking out latest actual GM results from %s into %s ...' % (
-        ACTUALS_SVN_REPO, self._actuals_dir)
+    logging.info('Checking out latest actual GM results from %s into %s ...' % (
+        ACTUALS_SVN_REPO, self._actuals_dir))
     actuals_repo = svn.Svn(self._actuals_dir)
     if not os.path.isdir(self._actuals_dir):
       os.makedirs(self._actuals_dir)
       actuals_repo.Checkout(ACTUALS_SVN_REPO, '.')
     else:
       actuals_repo.Update('.')
-    print 'Parsing results from actuals in %s and expectations in %s ...' % (
-        self._actuals_dir, self._expectations_dir)
+    logging.info(
+        'Parsing results from actuals in %s and expectations in %s ...' % (
+        self._actuals_dir, self._expectations_dir))
     self.results = results.Results(
       actuals_root=self._actuals_dir,
       expected_root=self._expectations_dir)
@@ -109,13 +112,13 @@ class Server(object):
     self.fetch_results()
     if self._export:
       server_address = ('', self._port)
-      print ('WARNING: Running in "export" mode. Users on other machines will '
-             'be able to modify your GM expectations!')
+      logging.warning('Running in "export" mode. Users on other machines will '
+                      'be able to modify your GM expectations!')
     else:
       server_address = ('127.0.0.1', self._port)
     http_server = BaseHTTPServer.HTTPServer(server_address, HTTPRequestHandler)
-    print 'Ready for requests on http://%s:%d' % (
-        http_server.server_name, http_server.server_port)
+    logging.info('Ready for requests on http://%s:%d' % (
+        http_server.server_name, http_server.server_port))
     http_server.serve_forever()
 
 
@@ -127,7 +130,7 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     """ Handles all GET requests, forwarding them to the appropriate
         do_GET_* dispatcher. """
     if self.path == '' or self.path == '/' or self.path == '/index.html' :
-      self.redirect_to('/static/view.html')
+      self.redirect_to('/static/view.html?resultsToLoad=all')
       return
     if self.path == '/favicon.ico' :
       self.redirect_to('/static/favicon.ico')
@@ -146,21 +149,20 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     dispatcher = dispatchers[dispatcher_name]
     dispatcher(remainder)
 
-  def do_GET_results(self, result_type):
+  def do_GET_results(self, type):
     """ Handle a GET request for GM results.
-    For now, we ignore the remaining path info, because we only know how to
-    return all results.
 
     Args:
-      result_type: currently unused
-
-    TODO(epoger): Unless we start making use of result_type, remove that
-    parameter."""
-    print 'do_GET_results: sending results of type "%s"' % result_type
-    # TODO(epoger): Cache response_dict rather than the results object, to save
-    # time on subsequent fetches (no need to regenerate the header, etc.)
-    response_dict = _SERVER.results.GetAll()
-    if response_dict:
+      type: string indicating which set of results to return;
+            must be one of the results.RESULTS_* constants
+    """
+    logging.debug('do_GET_results: sending results of type "%s"' % type)
+    try:
+      # TODO(epoger): Rather than using a global variable for the handler
+      # to refer to the Server object, make Server a subclass of
+      # HTTPServer, and then it could be available to the handler via
+      # the handler's .server instance variable.
+      response_dict = _SERVER.results.get_results_of_type(type)
       response_dict['header'] = {
         # Hash of testData, which the client must return with any edits--
         # this ensures that the edits were made to a particular dataset.
@@ -176,7 +178,7 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
         'isExported': _SERVER.is_exported(),
       }
       self.send_json_dict(response_dict)
-    else:
+    except:
       self.send_error(404)
 
   def do_GET_static(self, path):
@@ -187,14 +189,18 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     Args:
       path: path to file (under static directory) to retrieve
     """
-    print 'do_GET_static: sending file "%s"' % path
+    # Strip arguments ('?resultsToLoad=all') from the path
+    path = urlparse.urlparse(path).path
+
+    logging.debug('do_GET_static: sending file "%s"' % path)
     static_dir = os.path.realpath(os.path.join(PARENT_DIRECTORY, 'static'))
     full_path = os.path.realpath(os.path.join(static_dir, path))
     if full_path.startswith(static_dir):
       self.send_file(full_path)
     else:
-      print ('Attempted do_GET_static() of path [%s] outside of static dir [%s]'
-             % (full_path, static_dir))
+      logging.error(
+          'Attempted do_GET_static() of path [%s] outside of static dir [%s]'
+          % (full_path, static_dir))
       self.send_error(404)
 
   def redirect_to(self, url):
@@ -246,6 +252,7 @@ class HTTPRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
 
 def main():
+  logging.basicConfig(level=logging.INFO)
   parser = argparse.ArgumentParser()
   parser.add_argument('--actuals-dir',
                     help=('Directory into which we will check out the latest '
diff --git a/gm/rebaseline_server/static/loader.js b/gm/rebaseline_server/static/loader.js
index c8606cce87..46d28fca9c 100644
--- a/gm/rebaseline_server/static/loader.js
+++ b/gm/rebaseline_server/static/loader.js
@@ -31,14 +31,24 @@ Loader.filter(
 
 Loader.controller(
   'Loader.Controller',
-  function($scope, $http, $filter) {
-    $http.get("/results/all").then(
-      function(response) {
-        $scope.header = response.data.header;
-        $scope.categories = response.data.categories;
-        $scope.testData = response.data.testData;
+  function($scope, $http, $filter, $location) {
+    var resultsToLoad = $location.search().resultsToLoad;
+    $scope.loadingMessage = "Loading results of type '" + resultsToLoad +
+        "', please wait...";
+
+    $http.get("/results/" + resultsToLoad).success(
+      function(data, status, header, config) {
+        $scope.loadingMessage = "Processing data, please wait...";
+
+        $scope.header = data.header;
+        $scope.categories = data.categories;
+        $scope.testData = data.testData;
         $scope.sortColumn = 'test';
 
+        for (var i = 0; i < $scope.testData.length; i++) {
+          $scope.testData[i].index = i;
+        }
+
         $scope.hiddenResultTypes = {
           'failure-ignored': true,
           'no-comparison': true,
@@ -48,6 +58,12 @@ Loader.controller(
         $scope.selectedItems = {};
 
         $scope.updateResults();
+        $scope.loadingMessage = "";
+      }
+    ).error(
+      function(data, status, header, config) {
+        $scope.loadingMessage = "Failed to load results of type '"
+            + resultsToLoad + "'";
       }
     );
 
diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html
index c317b91146..0451b6519e 100644
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@@ -14,8 +14,8 @@
   <!-- TODO(epoger): Add some indication of how old the
   expected/actual data is -->
 
-  <em ng-hide="categories">
-    Loading data, please wait...
+  <em>
+    {{loadingMessage}}
   </em>
 
   <div ng-hide="!categories">
@@ -143,13 +143,13 @@
           <td>{{result.test}}</td>
           <td>{{result.config}}</td>
           <td>
-	    <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.expectedHashType}}/{{result.test}}/{{result.expectedHashDigest}}.png">
+            <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.expectedHashType}}/{{result.test}}/{{result.expectedHashDigest}}.png">
               <img width="{{imageSize}}" src="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.expectedHashType}}/{{result.test}}/{{result.expectedHashDigest}}.png"/>
             </a>
           </td>
           <td>
-	    <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png">
-	      <img width="{{imageSize}}" src="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png"/>
+            <a target="_blank" href="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png">
+              <img width="{{imageSize}}" src="http://chromium-skia-gm.commondatastorage.googleapis.com/gm/{{result.actualHashType}}/{{result.test}}/{{result.actualHashDigest}}.png"/>
             </a>
           </td>
           <td ng-hide="!header.isEditable">