rebaseline_server: add tabs, and ability to submit new baselines to the server

gm/rebaseline_server/results.py

 #!/usr/bin/python
 
 """
 Copyright 2013 Google Inc.
 
 Use of this source code is governed by a BSD-style license that can be
 found in the LICENSE file.
 
 Repackage expected/actual GM results as needed by our HTML rebaseline viewer.
 """
@@ skipping 13 matching lines @@
 # that directory.  That script allows us to parse the actual-results.json file
 # written out by the GM tool.
 # Make sure that the 'gm' dir is in the PYTHONPATH, but add it at the *end*
 # so any dirs that are already in the PYTHONPATH will be preferred.
 GM_DIRECTORY = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
 if GM_DIRECTORY not in sys.path:
   sys.path.append(GM_DIRECTORY)
 import gm_json
 
 IMAGE_FILENAME_RE = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
+IMAGE_FILENAME_FORMATTER = '%s_%s.png'  # pass in (testname, config)
+
 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.
 
-  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."""
+  Once this object has been constructed, the results (in self._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):
     """
     Args:
       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._get_dicts_from_root(actuals_root)
-    self._expected_builder_dicts = Results._get_dicts_from_root(expected_root)
-    self._combine_actual_and_expected()
+    self._actuals_root = actuals_root
+    self._expected_root = expected_root
+    self._load_actual_and_expected()
     self._timestamp = int(time.time())
 
   def get_timestamp(self):
     """Return the time at which this object was created, in seconds past epoch
     (UTC).
     """
     return self._timestamp
 
+  def edit_expectations(self, modifications):
+    """Edit the expectations stored within this object and write them back
+    to disk.
+
+    Note that this will NOT update the results stored in self._results[] ;
+    in order to see those updates, you must instantiate a new Results object
+    based on the (now updated) files on disk.
+
+    Args:
+      modifications: a list of dictionaries, one for each expectation to update:
+
+         [
+           {
+             'builder': 'Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug',
+             'test': 'bigmatrix',
+             'config': '8888',
+             'expectedHashType': 'bitmap-64bitMD5',
+             'expectedHashDigest': '10894408024079689926',
+           },
+           ...
+         ]
+
+    TODO(epoger): For now, this does not allow the caller to set any fields
+    other than expectedHashType/expectedHashDigest, and assumes that
+    ignore-failure should be set to False.  We need to add support
+    for other fields (notes, bugs, etc.) and ignore-failure=True.
+    """
+    expected_builder_dicts = Results._read_dicts_from_root(self._expected_root)
+    for mod in modifications:
+      image_name = IMAGE_FILENAME_FORMATTER % (mod['test'], mod['config'])

[borenet, 2013/10/22 19:33:16]

Should we verify that there are no funny characters in the input from the
website which might cause us to modify files other than the ones we're expecting
to modify?

[epoger, 2013/10/22 21:17:45]

Added a "Security note" to _write_dicts_to_root().  I think we're ok.  Thanks
for asking.

+      # TODO(epoger): assumes a single allowed digest per test
+      allowed_digests = [[mod['expectedHashType'],
+                          int(mod['expectedHashDigest'])]]
+      new_expectations = {
+          gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS: allowed_digests,
+          gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE: False,
+      }
+      builder_dict = expected_builder_dicts[mod['builder']]
+      builder_expectations = builder_dict.get(gm_json.JSONKEY_EXPECTEDRESULTS)
+      if not builder_expectations:
+        builder_expectations = {}
+        builder_dict[gm_json.JSONKEY_EXPECTEDRESULTS] = builder_expectations
+      builder_expectations[image_name] = new_expectations
+    Results._write_dicts_to_root(expected_builder_dicts, self._expected_root)

[borenet, 2013/10/22 19:33:16]

I'm scared of what happens when multiple users try to update expectations
simultaneously.  Do we need some kind of synchronization?

[epoger, 2013/10/22 21:17:45]

If you mean multiple users committing edits to the same rebaseline_server: the
oldResultsHash check (see line 336 of server.py) should prevent multiple users'
updates from interfering with each other.  (The first one to submit his edits
will win; the others will have to recreate them.)

(And besides, we won't have a common server that multiple developers submit
rebaselines to, in the near term.  Instead, each developer will launch his own
rebaseline_server for now.  Hopefully we'll figure out a way that developers
don't have to launch rebaseline_server or manually commit their baselines to
svn/git, but we're not there yet.)

If you mean multiple users, each committing edits to his own rebaseline_server,
and the clash happens when they commit to the repo: that's outside the scope of
this tool.  Hopefully, svn/git will handle it reasonably well.

[borenet, 2013/10/23 14:22:03]

Gotcha. I was envisioning this as a single server that everyone shares. Running
it on your own machine avoids a lot of my concerns.

+
   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
 
     Results are returned as a dictionary in this form:
 
        {
@@ skipping 29 matching lines @@
              'actualHashType': 'bitmap-64bitMD5',
              'actualHashDigest': '2409857384569',
            },
            ...
          ], # end of 'testData' list
        }
     """
     return self._results[type]
 
   @staticmethod
-  def _get_dicts_from_root(root, pattern='*.json'):
+  def _read_dicts_from_root(root, pattern='*.json'):
     """Read all JSON dictionaries within a directory tree.
 
     Args:
       root: path to root of directory tree
       pattern: which files to read within root (fnmatch-style pattern)
 
     Returns:
       A meta-dictionary containing all the JSON dictionaries found within
       the directory tree, keyed by the builder name of each dictionary.
 
     Raises:
       IOError if root does not refer to an existing directory
     """
     if not os.path.isdir(root):
       raise IOError('no directory found at path %s' % root)
     meta_dict = {}
     for dirpath, dirnames, filenames in os.walk(root):
       for matching_filename in fnmatch.filter(filenames, pattern):
         builder = os.path.basename(dirpath)
         if builder.endswith('-Trybot'):
           continue
         fullpath = os.path.join(dirpath, matching_filename)
         meta_dict[builder] = gm_json.LoadFromFile(fullpath)
     return meta_dict
 
-  def _combine_actual_and_expected(self):
-    """Gathers the results of all tests, across all builders (based on the
-    contents of self._actual_builder_dicts and self._expected_builder_dicts),
+  @staticmethod
+  def _write_dicts_to_root(meta_dict, root, pattern='*.json'):
+    """Write all per-builder dictionaries within meta_dict to files under
+    the root path.
+
+    Args:
+      meta_dict: a builder-keyed meta-dictionary containing all the JSON
+                 dictionaries we want to write out
+      root: path to root of directory tree within which to write files
+      pattern: which files to write within root (fnmatch-style pattern)
+
+    Raises:
+      IOError if root does not refer to an existing directory
+      KeyError if the set of per-builder dictionaries written out was
+               different than expected
+    """
+    if not os.path.isdir(root):
+      raise IOError('no directory found at path %s' % root)
+    actual_builders_written = []
+    for dirpath, dirnames, filenames in os.walk(root):
+      for matching_filename in fnmatch.filter(filenames, pattern):
+        builder = os.path.basename(dirpath)
+        if builder.endswith('-Trybot'):
+          continue
+        per_builder_dict = meta_dict.get(builder)
+        if per_builder_dict:
+          fullpath = os.path.join(dirpath, matching_filename)
+          gm_json.WriteToFile(per_builder_dict, fullpath)
+          actual_builders_written.append(builder)
+
+    # Check: did we write out the set of per-builder dictionaries we
+    # expected to?
+    expected_builders_written = sorted(meta_dict.keys())
+    actual_builders_written.sort()
+    if expected_builders_written != actual_builders_written:
+      raise KeyError(
+          'expected to write dicts for builders %s, but actually wrote them '
+          'for builders %s' % (
+              expected_builders_written, actual_builders_written))
+
+  def _load_actual_and_expected(self):
+    """Loads the results of all tests, across all builders (based on the
+    files within self._actuals_root and self._expected_root),
     and stores them in self._results.
     """
+    actual_builder_dicts = Results._read_dicts_from_root(self._actuals_root)
+    expected_builder_dicts = Results._read_dicts_from_root(self._expected_root)
+
     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,
         ])
 
     data_all = []
     data_failures = []
-    for builder in sorted(self._actual_builder_dicts.keys()):
+    for builder in sorted(actual_builder_dicts.keys()):
       actual_results_for_this_builder = (
-          self._actual_builder_dicts[builder][gm_json.JSONKEY_ACTUALRESULTS])
+          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:
           continue
         for image_name in sorted(results_of_this_type.keys()):
           actual_image = results_of_this_type[image_name]
           try:
             # TODO(epoger): assumes a single allowed digest per test
             expected_image = (
-                self._expected_builder_dicts
+                expected_builder_dicts
                     [builder][gm_json.JSONKEY_EXPECTEDRESULTS]
                     [image_name][gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]
                     [0])
           except (KeyError, TypeError):
             # There are several cases in which we would expect to find
             # no expectations for a given test:
             #
             # 1. result_type == NOCOMPARISON
             #   There are no expectations for this test yet!
             #
@@ skipping 105 matching lines @@
       category_dict: category dict-of-dicts to modify
       category_name: category name, as a string
       category_values: list of values we want to make sure are represented
                        for this category
     """
     if not category_dict.get(category_name):
       category_dict[category_name] = {}
     for category_value in category_values:
       if not category_dict[category_name].get(category_value):
         category_dict[category_name][category_value] = 0