download and rebaseline images using server

tools/jsondiff.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.
 '''
 
 '''
@@ skipping 36 matching lines @@
         pass
 
     def _GetFileContentsAsString(self, filepath):
         """Returns the full contents of a file, as a single string.
         If the filename looks like a URL, download its contents..."""
         if filepath.startswith('http:') or filepath.startswith('https:'):
             return urllib2.urlopen(filepath).read()
         else:
             return open(filepath, 'r').read()
 
-    def _GetExpectedResults(self, filepath):
-        """Returns the dictionary of expected results from a JSON file,
+    def _GetExpectedResults(self, contents):
+        """Returns the dictionary of expected results from a JSON string,
         in this form:
 
         {
           'test1' : 14760033689012826769,
           'test2' : 9151974350149210736,
           ...
         }
 
         We make these simplifying assumptions:
         1. Each test has either 0 or 1 allowed results.
         2. All expectations are of type JSONKEY_HASHTYPE_BITMAP_64BITMD5.
 
         Any tests which violate those assumptions will cause an exception to
         be raised.
 
         Any tests for which we have no expectations will be left out of the
         returned dictionary.
         """
         result_dict = {}
-        contents = self._GetFileContentsAsString(filepath)
         json_dict = gm_json.LoadFromString(contents)
         all_expectations = json_dict[gm_json.JSONKEY_EXPECTEDRESULTS]
         for test_name in all_expectations.keys():
             test_expectations = all_expectations[test_name]
             allowed_digests = test_expectations[
                 gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]
             if allowed_digests:
                 num_allowed_digests = len(allowed_digests)
                 if num_allowed_digests > 1:
                     raise ValueError(
                         'test %s in file %s has %d allowed digests' % (
                             test_name, filepath, num_allowed_digests))
                 digest_pair = allowed_digests[0]
                 if digest_pair[0] != gm_json.JSONKEY_HASHTYPE_BITMAP_64BITMD5:
                     raise ValueError(
                         'test %s in file %s has unsupported hashtype %s' % (
                             test_name, filepath, digest_pair[0]))
                 result_dict[test_name] = digest_pair[1]
         return result_dict
 
-    def _GetActualResults(self, filepath):
-        """Returns the dictionary of actual results from a JSON file,
+    def _GetActualResults(self, contents):
+        """Returns the dictionary of actual results from a JSON string,
         in this form:
 
         {
           'test1' : 14760033689012826769,
           'test2' : 9151974350149210736,
           ...
         }
 
         We make these simplifying assumptions:
         1. All results are of type JSONKEY_HASHTYPE_BITMAP_64BITMD5.
 
         Any tests which violate those assumptions will cause an exception to
         be raised.
 
         Any tests for which we have no actual results will be left out of the
         returned dictionary.
         """
         result_dict = {}
-        contents = self._GetFileContentsAsString(filepath)
         json_dict = gm_json.LoadFromString(contents)
         all_result_types = json_dict[gm_json.JSONKEY_ACTUALRESULTS]
         for result_type in all_result_types.keys():
             results_of_this_type = all_result_types[result_type]
             if results_of_this_type:
                 for test_name in results_of_this_type.keys():
                     digest_pair = results_of_this_type[test_name]
                     if digest_pair[0] != gm_json.JSONKEY_HASHTYPE_BITMAP_64BITMD5:
                         raise ValueError(
                             'test %s in file %s has unsupported hashtype %s' % (
                                 test_name, filepath, digest_pair[0]))

[Zach Reizner, 2013/08/02 21:30:58]

Just a quick thing my linter caught: filepath doesn't exist. I'm not sure how
you'd like to fix this, so I haven't done anything to it.

[epoger, 2013/08/06 15:26:43]

Ah.  That's because, before this CL, filepath was the argument to this function
(see https://codereview.chromium.org/20654006/diff/12001/tools/jsondiff.py ).

I guess just remove the "file %s" and filepath parts.

Another vote in favor of self-tests that exercise as many cases as possible. :-)

[epoger, 2013/08/06 15:28:06]

P.S. I'm not suggesting you take the time to add self-tests for this right now. 
Just saying this is an example of why they are generally useful.

                     result_dict[test_name] = digest_pair[1]
         return result_dict
 
     def _DictionaryDiff(self, old_dict, new_dict):
         """Generate a dictionary showing the diffs between old_dict and new_dict.
         Any entries which are identical across them will be left out."""
         diff_dict = {}
         all_keys = set(old_dict.keys() + new_dict.keys())
         for key in all_keys:
             if old_dict.get(key) != new_dict.get(key):
                 new_entry = {}
                 new_entry['old'] = old_dict.get(key)
                 new_entry['new'] = new_dict.get(key)
                 diff_dict[key] = new_entry
         return diff_dict
 
     def GenerateDiffDict(self, oldfile, newfile=None):
         """Generate a dictionary showing the diffs:
         old = expectations within oldfile
         new = expectations within newfile
 
         If newfile is not specified, then 'new' is the actual results within
         oldfile.
         """
-        old_results = self._GetExpectedResults(oldfile)
+        old_results = self._GetExpectedResults(self._GetFileContentsAsString(oldfile))

[epoger, 2013/08/02 14:33:06]

I think it would be a bit cleaner to have this method call
GenerateDiffDictFromStrings(), like so:

return self.GenerateDiffDictFromStrings(
    oldjson=self._GetFileContentsAsString(oldfile),
    newjson=self._GetFileContentsAsString(newfile)
)

That way, the logic around what-to-compare-against-if-newfile-is-None can be in
exactly one place, rather than copy-pasted into 2 places.

To do this, you'll need to update _GetFileContentsAsString() to return None if
filepath=None, which seems reasonable.

[Zach Reizner, 2013/08/02 21:30:58]

Done.

         if newfile:
-            new_results = self._GetExpectedResults(newfile)
+            new_results = self._GetExpectedResults(self._GetFileContentsAsString(newfile))
         else:
-            new_results = self._GetActualResults(oldfile)
+            new_results = self._GetActualResults(self._GetFileContentsAsString(oldfile))
+        return self._DictionaryDiff(old_results, new_results)
+
+    def GenerateDiffDictFromStrings(self, oldjson, newjson=None):
+        """Generate a dictionary showing the diffs:
+        old = expectations within oldjson
+        new = expectations within newjson
+
+        If newfile is not specified, then 'new' is the actual results within
+        oldfile.
+        """
+        old_results = self._GetExpectedResults(oldjson)
+        if newjson:
+            new_results = self._GetExpectedResults(newjson)
+        else:
+            new_results = self._GetActualResults(oldjson)
         return self._DictionaryDiff(old_results, new_results)
 
 
 def _Main():
     parser = argparse.ArgumentParser()
     parser.add_argument(
         'old',
         help='Path to JSON file whose expectations to display on ' +
         'the "old" side of the diff. This can be a filepath on ' +
         'local storage, or a URL.')
     parser.add_argument(
         'new', nargs='?',
         help='Path to JSON file whose expectations to display on ' +
         'the "new" side of the diff; if not specified, uses the ' +
         'ACTUAL results from the "old" JSON file. This can be a ' +
         'filepath on local storage, or a URL.')
     args = parser.parse_args()
     differ = GMDiffer()
     diffs = differ.GenerateDiffDict(oldfile=args.old, newfile=args.new)
     json.dump(diffs, sys.stdout, sort_keys=True, indent=2)
 
 
 if __name__ == '__main__':
     _Main()