rebaseline_server: UI improvements + set reviewed-by-human on commit
- select/clear/toggle all tests
- display "bugs" column
- set reviewed-by-human on commit
- a couple more tiny fixes, TODOs adjusted, etc.
(SkipBuildbotRuns)
R=senorblanco@chromium.org
Review URL: https://codereview.chromium.org/44123004
git-svn-id: http://skia.googlecode.com/svn/trunk@11970 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gm/rebaseline_server/results.py b/gm/rebaseline_server/results.py
index d3c8790..59a8f30 100755
--- a/gm/rebaseline_server/results.py
+++ b/gm/rebaseline_server/results.py
@@ -33,9 +33,17 @@
IMAGE_FILENAME_RE = re.compile(gm_json.IMAGE_FILENAME_PATTERN)
IMAGE_FILENAME_FORMATTER = '%s_%s.png' # pass in (testname, config)
+FIELDS_PASSED_THRU_VERBATIM = [
+ gm_json.JSONKEY_EXPECTEDRESULTS_BUGS,
+ gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE,
+ gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED,
+]
CATEGORIES_TO_SUMMARIZE = [
'builder', 'test', 'config', 'resultType',
+ gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE,
+ gm_json.JSONKEY_EXPECTEDRESULTS_REVIEWED,
]
+
RESULTS_ALL = 'all'
RESULTS_FAILURES = 'failures'
@@ -82,14 +90,13 @@
'config': '8888',
'expectedHashType': 'bitmap-64bitMD5',
'expectedHashDigest': '10894408024079689926',
+ 'bugs': [123, 456],
+ 'ignore-failure': false,
+ 'reviewed-by-human': true,
},
...
]
- 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:
@@ -99,8 +106,11 @@
int(mod['expectedHashDigest'])]]
new_expectations = {
gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS: allowed_digests,
- gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE: False,
}
+ for field in FIELDS_PASSED_THRU_VERBATIM:
+ value = mod.get(field)
+ if value is not None:
+ new_expectations[field] = value
builder_dict = expected_builder_dicts[mod['builder']]
builder_expectations = builder_dict.get(gm_json.JSONKEY_EXPECTEDRESULTS)
if not builder_expectations:
@@ -142,14 +152,17 @@
'testData': # list of test results, with a dictionary for each
[
{
+ 'resultType': 'failed',
'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',
+ 'bugs': [123, 456],
+ 'ignore-failure': false,
+ 'reviewed-by-human': true,
},
...
], # end of 'testData' list
@@ -246,6 +259,7 @@
categories_all = {}
categories_failures = {}
+
Results._ensure_included_in_category_dict(categories_all,
'resultType', [
gm_json.JSONKEY_ACTUALRESULTS_FAILED,
@@ -271,13 +285,18 @@
continue
for image_name in sorted(results_of_this_type.keys()):
actual_image = results_of_this_type[image_name]
+
+ # Default empty expectations; overwrite these if we find any real ones
+ expectations_per_test = None
+ expected_image = [None, None]
try:
+ expectations_per_test = (
+ expected_builder_dicts
+ [builder][gm_json.JSONKEY_EXPECTEDRESULTS][image_name])
# TODO(epoger): assumes a single allowed digest per test
expected_image = (
- expected_builder_dicts
- [builder][gm_json.JSONKEY_EXPECTEDRESULTS]
- [image_name][gm_json.JSONKEY_EXPECTEDRESULTS_ALLOWEDDIGESTS]
- [0])
+ expectations_per_test
+ [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:
@@ -285,12 +304,7 @@
# 1. result_type == NOCOMPARISON
# There are no expectations for this test yet!
#
- # 2. ignore-tests.txt
- # If a test has been listed in ignore-tests.txt, then its status
- # may show as FAILUREIGNORED even if it doesn't have any
- # expectations yet.
- #
- # 3. alternate rendering mode failures (e.g. serialized)
+ # 2. alternate rendering mode failures (e.g. serialized)
# In cases like
# https://code.google.com/p/skia/issues/detail?id=1684
# ('tileimagefilter GM test failing in serialized render mode'),
@@ -299,19 +313,16 @@
# for the test (the implicit expectation is that it must
# render the same in all rendering modes).
#
- # Don't log types 1 or 2, because they are common.
+ # Don't log type 1, because it is common.
# Log other types, because they are rare and we should know about
# them, but don't throw an exception, because we need to keep our
# tools working in the meanwhile!
- if result_type not in [
- gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON,
- gm_json.JSONKEY_ACTUALRESULTS_FAILUREIGNORED] :
+ if result_type != gm_json.JSONKEY_ACTUALRESULTS_NOCOMPARISON:
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
# the 'failed' set of actuals until all the bots have
@@ -333,20 +344,31 @@
(test, config) = IMAGE_FILENAME_RE.match(image_name).groups()
results_for_this_test = {
+ 'resultType': updated_result_type,
'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]),
+
+ # FIELDS_PASSED_THRU_VERBATIM that may be overwritten below...
+ gm_json.JSONKEY_EXPECTEDRESULTS_IGNOREFAILURE: False,
}
+ if expectations_per_test:
+ for field in FIELDS_PASSED_THRU_VERBATIM:
+ results_for_this_test[field] = expectations_per_test.get(field)
Results._add_to_category_dict(categories_all, results_for_this_test)
data_all.append(results_for_this_test)
+
+ # TODO(epoger): In effect, we have a list of resultTypes that we
+ # include in the different result lists (data_all and data_failures).
+ # This same list should be used by the calls to
+ # Results._ensure_included_in_category_dict() earlier on.
if updated_result_type != gm_json.JSONKEY_ACTUALRESULTS_SUCCEEDED:
Results._add_to_category_dict(categories_failures,
- results_for_this_test)
+ results_for_this_test)
data_failures.append(results_for_this_test)
self._results = {
@@ -373,8 +395,6 @@
"""
for category in CATEGORIES_TO_SUMMARIZE:
category_value = test_results.get(category)
- if not category_value:
- continue # test_results did not include this category, keep going
if not category_dict.get(category):
category_dict[category] = {}
if not category_dict[category].get(category_value):
diff --git a/gm/rebaseline_server/static/loader.js b/gm/rebaseline_server/static/loader.js
index 5af0e24..8cfdfec 100644
--- a/gm/rebaseline_server/static/loader.js
+++ b/gm/rebaseline_server/static/loader.js
@@ -21,8 +21,9 @@
var filteredItems = [];
for (var i = 0; i < unfilteredItems.length; i++) {
var item = unfilteredItems[i];
- // For performance, we examine the "set" objects directly rather
- // than calling $scope.isValueInSet().
+ // For performance, we examine the "set" objects directly rather
+ // than calling $scope.isValueInSet().
+ // Besides, I don't think we have access to $scope in here...
if (!(true == hiddenResultTypes[item.resultType]) &&
!(true == hiddenConfigs[item.config]) &&
(viewingTab == item.tab)) {
@@ -58,6 +59,12 @@
$scope.sortColumn = 'test';
$scope.showTodos = false;
+ $scope.showSubmitAdvancedSettings = false;
+ $scope.submitAdvancedSettings = {};
+ $scope.submitAdvancedSettings['reviewed-by-human'] = true;
+ $scope.submitAdvancedSettings['ignore-failures'] = false;
+ $scope.submitAdvancedSettings['bug'] = '';
+
// Create the list of tabs (lists into which the user can file each
// test). This may vary, depending on isEditable.
$scope.tabs = [
@@ -83,10 +90,10 @@
$scope.testData[i].tab = $scope.defaultTab;
}
- // Arrays within which the user can toggle individual elements.
+ // Arrays within which the user can toggle individual elements.
$scope.selectedItems = [];
- // Sets within which the user can toggle individual elements.
+ // Sets within which the user can toggle individual elements.
$scope.hiddenResultTypes = {
'failure-ignored': true,
'no-comparison': true,
@@ -108,6 +115,48 @@
//
+ // Select/Clear/Toggle all tests.
+ //
+
+ /**
+ * Select all currently showing tests.
+ */
+ $scope.selectAllItems = function() {
+ var numItemsShowing = $scope.limitedTestData.length;
+ for (var i = 0; i < numItemsShowing; i++) {
+ var index = $scope.limitedTestData[i].index;
+ if (!$scope.isValueInArray(index, $scope.selectedItems)) {
+ $scope.toggleValueInArray(index, $scope.selectedItems);
+ }
+ }
+ }
+
+ /**
+ * Deselect all currently showing tests.
+ */
+ $scope.clearAllItems = function() {
+ var numItemsShowing = $scope.limitedTestData.length;
+ for (var i = 0; i < numItemsShowing; i++) {
+ var index = $scope.limitedTestData[i].index;
+ if ($scope.isValueInArray(index, $scope.selectedItems)) {
+ $scope.toggleValueInArray(index, $scope.selectedItems);
+ }
+ }
+ }
+
+ /**
+ * Toggle selection of all currently showing tests.
+ */
+ $scope.toggleAllItems = function() {
+ var numItemsShowing = $scope.limitedTestData.length;
+ for (var i = 0; i < numItemsShowing; i++) {
+ var index = $scope.limitedTestData[i].index;
+ $scope.toggleValueInArray(index, $scope.selectedItems);
+ }
+ }
+
+
+ //
// Tab operations.
//
@@ -204,8 +253,7 @@
true
),
$scope.sortColumn);
- $scope.limitedTestData = $filter("limitTo")(
- $scope.filteredTestData, $scope.displayLimit);
+ $scope.limitedTestData = $scope.filteredTestData;
}
$scope.imageSize = $scope.imageSizePending;
$scope.setUpdatesPending(false);
@@ -235,6 +283,21 @@
*/
$scope.submitApprovals = function(testDataSubset) {
$scope.submitPending = true;
+
+ // Convert bug text field to null or 1-item array.
+ var bugs = null;
+ var bugNumber = parseInt($scope.submitAdvancedSettings['bug']);
+ if (!isNaN(bugNumber)) {
+ bugs = [bugNumber];
+ }
+
+ // TODO(epoger): This is a suboptimal way to prevent users from
+ // rebaselining failures in alternative renderModes, but it does work.
+ // For a better solution, see
+ // https://code.google.com/p/skia/issues/detail?id=1748 ('gm: add new
+ // result type, RenderModeMismatch')
+ var encounteredComparisonConfig = false;
+
var newResults = [];
for (var i = 0; i < testDataSubset.length; i++) {
var actualResult = testDataSubset[i];
@@ -245,8 +308,27 @@
expectedHashType: actualResult['actualHashType'],
expectedHashDigest: actualResult['actualHashDigest'],
};
+ if (0 == expectedResult.config.indexOf('comparison-')) {
+ encounteredComparisonConfig = true;
+ }
+
+ // Advanced settings...
+ expectedResult['reviewed-by-human'] =
+ $scope.submitAdvancedSettings['reviewed-by-human'];
+ if (true == $scope.submitAdvancedSettings['ignore-failure']) {
+ // if it's false, don't send it at all (just keep the default)
+ expectedResult['ignoreFailure'] = true;
+ }
+ expectedResult['bugs'] = bugs;
+
newResults.push(expectedResult);
}
+ if (encounteredComparisonConfig) {
+ alert("Approval failed -- you cannot approve results with config " +
+ "type comparison-*");
+ $scope.submitPending = false;
+ return;
+ }
$http({
method: "POST",
url: "/edits",
diff --git a/gm/rebaseline_server/static/view.html b/gm/rebaseline_server/static/view.html
index 292386b..36da9ee 100644
--- a/gm/rebaseline_server/static/view.html
+++ b/gm/rebaseline_server/static/view.html
@@ -25,9 +25,7 @@
<div class="todo-div"><!-- TODOs -->
<p>
TODO(epoger):
- <input type="checkbox" name="showTodosCheckbox" value="true"
- ng-checked="showTodos == true"
- ng-click="showTodos = !showTodos">
+ <input type="checkbox" ng-model="showTodos">
show
<ul ng-hide="!showTodos">
<li>
@@ -38,8 +36,13 @@
Add ability to filter builder and test names
(using a free-form text field, with partial string match)
</li><li>
- Add more columns, such as pixel diffs, notes/bugs,
- ignoreFailure boolean
+ Add pixel diffs, and sorting by percentage of different pixels
+ </li><li>
+ Add ability to sort/filter by reviewed-by-human. Depends on
+ <a href="https://code.google.com/p/skia/issues/detail?id=1758">
+ bug 1758
+ </a>
+ ('rebaseline_server: make the "categories" struct passed from server to client a list instead of a dict')
</li><li>
Improve the column sorting, as per
<a href="http://jsfiddle.net/vojtajina/js64b/14/">
@@ -85,6 +88,7 @@
</th>
</tr>
<tr valign="top">
+ <!-- TODO(epoger): make this an ng-repeat over resultType, config, etc? -->
<td>
resultType<br>
<label ng-repeat="(resultType, count) in categories['resultType']">
@@ -150,6 +154,21 @@
Submitting, please wait...
</div>
</div>
+ <div>
+ Advanced settings...
+ <input type="checkbox" ng-model="showSubmitAdvancedSettings">
+ show
+ <ul ng-hide="!showSubmitAdvancedSettings">
+ <li ng-repeat="setting in ['reviewed-by-human', 'ignore-failures']">
+ {{setting}}
+ <input type="checkbox" ng-model="submitAdvancedSettings[setting]">
+ </li>
+ <li ng-repeat="setting in ['bug']">
+ {{setting}}
+ <input type="text" ng-model="submitAdvancedSettings[setting]">
+ </li>
+ </ul>
+ </div>
</div>
<p>
@@ -167,11 +186,23 @@
(click on the column header radio buttons to re-sort by that column)
</div>
<div style="float:right">
+ <div>
+ all tests shown:
+ <button ng-click="selectAllItems()">
+ select
+ </button>
+ <button ng-click="clearAllItems()">
+ clear
+ </button>
+ <button ng-click="toggleAllItems()">
+ toggle
+ </button>
+ </div>
<div ng-repeat="otherTab in tabs">
<button ng-click="moveSelectedItemsToTab(otherTab)"
ng-disabled="selectedItems.length == 0"
ng-hide="otherTab == viewingTab">
- {{selectedItems.length}} move selected tests to {{otherTab}} tab
+ move {{selectedItems.length}} selected tests to {{otherTab}} tab
</button>
</div>
</div>
@@ -195,6 +226,14 @@
<th>
<input type="radio"
name="sortColumnRadio"
+ value="bugs"
+ ng-checked="(sortColumn == 'bugs')"
+ ng-click="sortResultsBy('bugs')">
+ bugs
+ </th>
+ <th>
+ <input type="radio"
+ name="sortColumnRadio"
value="expectedHashDigest"
ng-checked="(sortColumn == 'expectedHashDigest')"
ng-click="sortResultsBy('expectedHashDigest')">
@@ -213,10 +252,16 @@
</th>
</tr>
<tr ng-repeat="result in limitedTestData">
- <td>{{result.resultType}}</td>
- <td>{{result.builder}}</td>
- <td>{{result.test}}</td>
- <td>{{result.config}}</td>
+ <td ng-repeat="categoryName in ['resultType', 'builder', 'test', 'config']">
+ {{result[categoryName]}}
+ </td>
+ <td>
+ <a ng-repeat="bug in result['bugs']"
+ href="https://code.google.com/p/skia/issues/detail?id={{bug}}"
+ target="_blank">
+ {{bug}}
+ </a>
+ </td>
<td>
<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"/>