rebaseline_server: extend returned JSON dict to allow for result-editing in coming CL
(SkipBuildbotRuns)
R=jcgregorio@google.com
Review URL: https://codereview.chromium.org/26659002
git-svn-id: http://skia.googlecode.com/svn/trunk@11679 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py
index 6ac397e..84c45b9 100755
--- a/gm/rebaseline_server/results.py
+++ b/gm/rebaseline_server/results.py
@@ -1,15 +1,13 @@
#!/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.
-'''
+"""
# System-level imports
import fnmatch
@@ -41,7 +39,7 @@
def __init__(self, actuals_root, expected_root):
"""
- params:
+ Args:
actuals_root: root directory containing all actual-results.json files
expected_root: root directory containing all expected-results.json files
"""
@@ -55,52 +53,56 @@
"""Return results of all tests, as a dictionary in this form:
{
- "categories": # dictionary of categories listed in
+ 'categories': # dictionary of categories listed in
# CATEGORIES_TO_SUMMARIZE, with the number of times
# each value appears within its category
{
- "resultType": # category name
+ 'resultType': # category name
{
- "failed": 29, # category value and total number found of that value
- "failure-ignored": 948,
- "no-comparison": 4502,
- "succeeded": 38609,
+ 'failed': 29, # category value and total number found of that value
+ 'failure-ignored': 948,
+ 'no-comparison': 4502,
+ 'succeeded': 38609,
},
- "builder":
+ 'builder':
{
- "Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug": 1286,
- "Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release": 1134,
+ 'Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug': 1286,
+ 'Test-Mac10.6-MacMini4.1-GeForce320M-x86-Release': 1134,
...
},
... # other categories from CATEGORIES_TO_SUMMARIZE
- }, # end of "categories" dictionary
+ }, # end of 'categories' dictionary
- "testData": # list of test results, with a dictionary for each
+ 'testData': # list of test results, with a dictionary for each
[
{
- "builder": "Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug",
- "test": "bigmatrix",
- "config": "8888",
- "resultType": "failed",
- "expectedHashType": "bitmap-64bitMD5",
- "expectedHashDigest": "10894408024079689926",
- "actualHashType": "bitmap-64bitMD5",
- "actualHashDigest": "2409857384569",
+ 'index': 0, # index of this result within testData list
+ 'builder': 'Test-Mac10.6-MacMini4.1-GeForce320M-x86-Debug',
+ 'test': 'bigmatrix',
+ 'config': '8888',
+ 'resultType': 'failed',
+ 'expectedHashType': 'bitmap-64bitMD5',
+ 'expectedHashDigest': '10894408024079689926',
+ 'actualHashType': 'bitmap-64bitMD5',
+ 'actualHashDigest': '2409857384569',
},
...
- ], # end of "testData" list
+ ], # end of 'testData' list
}
"""
return self._all_results
@staticmethod
def _GetDictsFromRoot(root, pattern='*.json'):
- """Read all JSON dictionaries within a directory tree, returning them within
- a meta-dictionary (keyed by the builder name for each dictionary).
+ """Read all JSON dictionaries within a directory tree.
- params:
+ 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.
"""
meta_dict = {}
for dirpath, dirnames, filenames in os.walk(root):
@@ -115,12 +117,21 @@
@staticmethod
def _Combine(actual_builder_dicts, expected_builder_dicts):
"""Gathers the results of all tests, across all builders (based on the
- contents of actual_builder_dicts and expected_builder_dicts)
- and returns it in a list in the same form needed for self.GetAll().
+ 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().
"""
test_data = []
category_dict = {}
@@ -183,14 +194,14 @@
expected_image = [None, None]
# If this test was recently rebaselined, it will remain in
- # the "failed" set of actuals until all the bots have
+ # the 'failed' set of actuals until all the bots have
# cycled (although the expectations have indeed been set
# from the most recent actuals). Treat these as successes
# instead of failures.
#
# TODO(epoger): Do we need to do something similar in
# other cases, such as when we have recently marked a test
- # as ignoreFailure but it still shows up in the "failed"
+ # as ignoreFailure but it still shows up in the 'failed'
# category? Maybe we should not rely on the result_type
# categories recorded within the gm_actuals AT ALL, and
# instead evaluate the result_type ourselves based on what
@@ -202,30 +213,31 @@
(test, config) = IMAGE_FILENAME_RE.match(image_name).groups()
results_for_this_test = {
- "builder": builder,
- "test": test,
- "config": config,
- "resultType": updated_result_type,
- "actualHashType": actual_image[0],
- "actualHashDigest": str(actual_image[1]),
- "expectedHashType": expected_image[0],
- "expectedHashDigest": str(expected_image[1]),
+ 'index': len(test_data),
+ 'builder': builder,
+ 'test': test,
+ 'config': config,
+ 'resultType': updated_result_type,
+ 'actualHashType': actual_image[0],
+ 'actualHashDigest': str(actual_image[1]),
+ '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}
+ return {'categories': category_dict, 'testData': test_data}
@staticmethod
def _AddToCategoryDict(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.)
- params:
+ Args:
category_dict: category dict-of-dicts to add to; modify this in-place
test_results: test data with which to update category_list, in a dict:
{
- "category_name": "category_value",
- "category_name": "category_value",
+ 'category_name': 'category_value',
+ 'category_name': 'category_value',
...
}
"""
@@ -246,7 +258,7 @@
even if there aren't any results with that name/value pair.
(See documentation of self.GetAll() for the format of this dictionary.)
- params:
+ Args:
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
diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py
index 439d5da..7b87d6f 100755
--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@@ -1,15 +1,13 @@
#!/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.
-'''
-'''
HTTP server for our HTML rebaseline viewer.
-'''
+"""
# System-level imports
import argparse
@@ -60,25 +58,31 @@
_SERVER = None # This gets filled in by main()
class Server(object):
- """ HTTP server for our HTML rebaseline viewer.
+ """ HTTP server for our HTML rebaseline viewer. """
- params:
- actuals_dir: directory under which we will check out the latest actual
- GM results
- expectations_dir: directory under which to find GM expectations (they
- must already be in that directory)
- port: which TCP port to listen on for HTTP requests
- export: whether to allow HTTP clients on other hosts to access this server
- """
def __init__(self,
actuals_dir=DEFAULT_ACTUALS_DIR,
expectations_dir=DEFAULT_EXPECTATIONS_DIR,
port=DEFAULT_PORT, export=False):
+ """
+ Args:
+ actuals_dir: directory under which we will check out the latest actual
+ GM results
+ expectations_dir: directory under which to find GM expectations (they
+ must already be in that directory)
+ port: which TCP port to listen on for HTTP requests
+ export: whether to allow HTTP clients on other hosts to access this server
+ """
self._actuals_dir = actuals_dir
self._expectations_dir = expectations_dir
self._port = port
self._export = export
+ def is_exported(self):
+ """ Returns true iff HTTP clients on other hosts are allowed to access
+ this server. """
+ return self._export
+
def fetch_results(self):
""" Create self.results, based on the expectations in
self._expectations_dir and the latest actuals from skia-autogen.
@@ -131,8 +135,8 @@
# All requests must be of this form:
# /dispatcher/remainder
- # where "dispatcher" indicates which do_GET_* dispatcher to run
- # and "remainder" is the remaining path sent to that dispatcher.
+ # where 'dispatcher' indicates which do_GET_* dispatcher to run
+ # and 'remainder' is the remaining path sent to that dispatcher.
normpath = posixpath.normpath(self.path)
(dispatcher_name, remainder) = PATHSPLIT_RE.match(normpath).groups()
dispatchers = {
@@ -147,11 +151,30 @@
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:
+ 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.
+ 'data-hash': str(hash(repr(response_dict['testData']))),
+
+ # Whether the server will accept edits back.
+ # TODO(epoger): Not yet implemented, so hardcoding to False;
+ # once we implement the 'browseonly' mode discussed in
+ # https://codereview.chromium.org/24274003/#msg6 , this value will vary.
+ 'isEditable': False,
+
+ # Whether the service is accessible from other hosts.
+ 'isExported': _SERVER.is_exported(),
+ }
self.send_json_dict(response_dict)
else:
self.send_error(404)
@@ -159,7 +182,11 @@
def do_GET_static(self, path):
""" Handle a GET request for a file under the 'static' directory.
Only allow serving of files within the 'static' directory that is a
- filesystem sibling of this script. """
+ filesystem sibling of this script.
+
+ Args:
+ path: path to file (under static directory) to retrieve
+ """
print '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))
@@ -171,14 +198,22 @@
self.send_error(404)
def redirect_to(self, url):
- """ Redirect the HTTP client to a different url. """
+ """ Redirect the HTTP client to a different url.
+
+ Args:
+ url: URL to redirect the HTTP client to
+ """
self.send_response(301)
self.send_header('Location', url)
self.end_headers()
def send_file(self, path):
""" Send the contents of the file at this path, with a mimetype based
- on the filename extension. """
+ on the filename extension.
+
+ Args:
+ path: path of file whose contents to send to the HTTP client
+ """
# Grab the extension if there is one
extension = os.path.splitext(path)[1]
if len(extension) >= 1:
@@ -199,7 +234,11 @@
def send_json_dict(self, json_dict):
""" Send the contents of this dictionary in JSON format, with a JSON
- mimetype. """
+ mimetype.
+
+ Args:
+ json_dict: dictionary to send
+ """
self.send_response(200)
self.send_header('Content-type', 'application/json')
self.end_headers()
diff --git a/gm/rebaseline_server/static/loader.js b/gm/rebaseline_server/static/loader.js
index 8b5374a..c8606cc 100644
--- a/gm/rebaseline_server/static/loader.js
+++ b/gm/rebaseline_server/static/loader.js
@@ -18,11 +18,11 @@
return function(unfilteredItems, hiddenResultTypes, hiddenConfigs) {
var filteredItems = [];
for (var i = 0; i < unfilteredItems.length; i++) {
- var item = unfilteredItems[i];
- if ((hiddenResultTypes.indexOf(item.resultType) < 0) &&
- (hiddenConfigs.indexOf(item.config) < 0)) {
- filteredItems.push(item);
- }
+ var item = unfilteredItems[i];
+ if (!(true == hiddenResultTypes[item.resultType]) &&
+ !(true == hiddenConfigs[item.config])) {
+ filteredItems.push(item);
+ }
}
return filteredItems;
};
@@ -34,27 +34,44 @@
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;
$scope.sortColumn = 'test';
- $scope.hiddenResultTypes = [
- 'failure-ignored', 'no-comparison', 'succeeded'];
- $scope.hiddenConfigs = [];
+ $scope.hiddenResultTypes = {
+ 'failure-ignored': true,
+ 'no-comparison': true,
+ 'succeeded': true,
+ };
+ $scope.hiddenConfigs = {};
+ $scope.selectedItems = {};
$scope.updateResults();
}
);
+ $scope.isItemSelected = function(index) {
+ return (true == $scope.selectedItems[index]);
+ }
+ $scope.toggleItemSelected = function(index) {
+ if (true == $scope.selectedItems[index]) {
+ delete $scope.selectedItems[index];
+ } else {
+ $scope.selectedItems[index] = true;
+ }
+ // unlike other toggle methods below, does not set
+ // $scope.areUpdatesPending = true;
+ }
+
$scope.isHiddenResultType = function(thisResultType) {
- return ($scope.hiddenResultTypes.indexOf(thisResultType) >= 0);
+ return (true == $scope.hiddenResultTypes[thisResultType]);
}
$scope.toggleHiddenResultType = function(thisResultType) {
- var i = $scope.hiddenResultTypes.indexOf(thisResultType);
- if (i >= 0) {
- $scope.hiddenResultTypes.splice(i, 1);
+ if (true == $scope.hiddenResultTypes[thisResultType]) {
+ delete $scope.hiddenResultTypes[thisResultType];
} else {
- $scope.hiddenResultTypes.push(thisResultType);
+ $scope.hiddenResultTypes[thisResultType] = true;
}
$scope.areUpdatesPending = true;
}
@@ -65,14 +82,13 @@
// category.
// But for now, I wanted to see this working. :-)
$scope.isHiddenConfig = function(thisConfig) {
- return ($scope.hiddenConfigs.indexOf(thisConfig) >= 0);
+ return (true == $scope.hiddenConfigs[thisConfig]);
}
$scope.toggleHiddenConfig = function(thisConfig) {
- var i = $scope.hiddenConfigs.indexOf(thisConfig);
- if (i >= 0) {
- $scope.hiddenConfigs.splice(i, 1);
+ if (true == $scope.hiddenConfigs[thisConfig]) {
+ delete $scope.hiddenConfigs[thisConfig];
} else {
- $scope.hiddenConfigs.push(thisConfig);
+ $scope.hiddenConfigs[thisConfig] = true;
}
$scope.areUpdatesPending = true;
}
@@ -84,15 +100,15 @@
// the items as they are displayed, rather than storing multiple
// array copies? (For better performance.)
$scope.filteredTestData =
- $filter("orderBy")(
- $filter("removeHiddenItems")(
- $scope.testData,
- $scope.hiddenResultTypes,
- $scope.hiddenConfigs
- ),
- $scope.sortColumn);
+ $filter("orderBy")(
+ $filter("removeHiddenItems")(
+ $scope.testData,
+ $scope.hiddenResultTypes,
+ $scope.hiddenConfigs
+ ),
+ $scope.sortColumn);
$scope.limitedTestData = $filter("limitTo")(
- $scope.filteredTestData, $scope.displayLimit);
+ $scope.filteredTestData, $scope.displayLimit);
$scope.imageSize = $scope.imageSizePending;
$scope.areUpdatesPending = false;
}
diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html
index cd3ab15..c317b91 100644
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@@ -11,10 +11,6 @@
<body>
<div ng-controller="Loader.Controller">
- <!-- TODO(epoger): Add a warning banner if the server is running in
- --export mode
- -->
-
<!-- TODO(epoger): Add some indication of how old the
expected/actual data is -->
@@ -23,6 +19,11 @@
</em>
<div ng-hide="!categories">
+ <div ng-hide="!(header.isEditable && header.isExported)"
+ style="background-color:#ffbb00">
+ WARNING! These results are editable and exported, so any user
+ who can connect to this server over the network can modify them.
+ </div>
<table border="1">
<tr>
<th colspan="2">
@@ -131,6 +132,10 @@
ng-click="sortResultsBy('actualHashDigest')">
actual image
</th>
+ <th ng-hide="!header.isEditable">
+ <!-- item-selection checkbox column -->
+ {{selectedItems}} <!-- TODO(epoger): temporary debug output -->
+ </th>
</tr>
<tr ng-repeat="result in limitedTestData">
<td>{{result.resultType}}</td>
@@ -147,6 +152,12 @@
<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">
+ <input type="checkbox"
+ name="rowSelect"
+ value="{{result.index}}"
+ ng-checked="isItemSelected(result.index)"
+ ng-click="toggleItemSelected(result.index)">
</tr>
</table>
</div>