Add gm/confirm_no_failures_in_json.py

gm/confirm_no_failures_in_json.py

+#!/usr/bin/env python
+# Copyright (c) 2013 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Utility to confirm that a JSON summary written by GM contains no failures.
+
+Usage:
+  python confirm_no_failures_in_json.py <filename>
+"""
+
+__author__ = 'Elliot Poger'
+
+
+import json
+import sys
+
+
+# These constants must be kept in sync with the kJsonKey_ constants in
+# gm_expectations.cpp !
+JSONKEY_ACTUALRESULTS = 'actual-results'
+JSONKEY_ACTUALRESULTS_FAILED = 'failed'
+
+# This is the same indent level as used by jsoncpp, just for consistency.
+JSON_INDENTLEVEL = 3
+

[rmistry, 2013/05/13 12:25:39]

Nit: Please add one more new line here.

[epoger, 2013/05/14 15:03:00]

Done.

+def Assert(filepath):

[rmistry, 2013/05/13 12:25:39]

Should this function (and the one below) be private? Do you envision them being
used from any other modules?

[epoger, 2013/05/14 15:03:00]

I actually do envision them being used by other modules.  Particularly:
- the buildbot's CompareGMs step will call Assert()
- some process that wants to upload the results of failed tests may call
GetFailedTests(), and then use the resulting dictionary

Does that sound reasonable?

[rmistry, 2013/05/14 18:36:44]

Yes.

+  """Raises an exception if the JSON summary at filepath contains any failed
+  tests, or if we were unable to read the JSON summary."""
+  failed_tests = GetFailedTests(filepath)
+  if failed_tests:
+    raise Exception('JSON file %s contained these test failures...\n%s' % (
+        filepath, json.dumps(failed_tests, indent=JSON_INDENTLEVEL)))
+
+
+def GetFailedTests(filepath):
+  """Returns the dictionary of failed tests from the JSON file at filepath."""
+  json_dict = json.load(open(filepath))
+  actual_results = json_dict.get(JSONKEY_ACTUALRESULTS)

[rmistry, 2013/05/13 12:25:39]

If the json_dict does not have an 'actual-results' key then it will return a
None here instead of failing immediately. This will cause the below line to fail
with:
AttributeError: 'NoneType' object has no attribute 'get'.

We can instead throw a better exception by changing this line to:
actual_results = json_dict[JSONKEY_ACTUALRESULTS]
then it will fail with:
KeyError: actual-results.

[epoger, 2013/05/14 15:03:00]

That's much better, thanks!  It also signals an error if there is no "failed"
key within the "actual-results" dictionary, rather than acting the same as if
the "failed" key pointed to an empty sub-dictionary.

[rmistry, 2013/05/14 18:36:44]

The failed key will always be present? or are we assuming that when
GetFailedTests is called it should always exist?

[epoger, 2013/05/14 18:56:08]

The "failed" key should always be present... if there were no failures, it will
point to an empty subdictionary.  See for example
https://code.google.com/p/skia/source/browse/trunk/gm/tests/outputs/compared-...
:

   "actual-results" : {
      "failed" : null,
      "failure-ignored" : null,
      "no-comparison" : null,
      "succeeded" : {
         "565/selftest1.png" : {
            "bitmap-64bitMD5" : 12927999507540085554
         },
       ...

+  return actual_results.get(JSONKEY_ACTUALRESULTS_FAILED)
+
+
+if '__main__' == __name__:
+  if len(sys.argv) != 2:
+    raise Exception('usage: %s <input-json-filepath>' % sys.argv[0])
+  Assert(sys.argv[1])