Run-length encode the JSON results. This makes them considerably...

Run-length encode the JSON results. This makes them considerably
smaller, which should make the dashboard load a ton faster and
allows us to store a lot more runs. Changing the default to 500 runs
for now.

JS Changes:
-Add ability to control the number of results shown per test.
-Add a debug mode for easier local testing
-Consolidate query and hash parameter handling
-Identify tests that need to be marked "SLOW"
-Hide tests that fail for many runs then start passing for many runs.

Tony, can you review the python?
Arv, can you review the JS?

BUG=none
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25820

[tony, 2009-09-09 23:10:44]

Python side LGTM, just a few nits.

http://codereview.chromium.org/201073/diff/2001/2002
File webkit/tools/layout_tests/layout_package/json_results_generator.py (right):

http://codereview.chromium.org/201073/diff/2001/2002#newcode164
Line 164: """Inserts the item into the run-length encoded results."""
Nit: document args?

http://codereview.chromium.org/201073/diff/2001/2002#newcode166
Line 166: encoded_results[0][0] = encoded_results[0][0] + 1
Nit: += 1

http://codereview.chromium.org/201073/diff/2001/2002#newcode168
Line 168: # TODO document why we do as an array instead of a class
TODO?

http://codereview.chromium.org/201073/diff/2001/2002#newcode228
Line 228: num_builds = 0
Nit: Docstring

[arv (Not doing code reviews), 2009-09-09 23:21:04]

nits

http://codereview.chromium.org/201073/diff/2001/2003
File webkit/tools/layout_tests/flakiness_dashboard.html (right):

http://codereview.chromium.org/201073/diff/2001/2003#newcode222
Line 222: function isAlphaNumeric(str) {
Bad name? How about isValidName or something like that?

http://codereview.chromium.org/201073/diff/2001/2003#newcode250
Line 250: function parseParameters(parameterStr, validValueHandler) {
To me, it would make more sense to remove ? outside this function

http://codereview.chromium.org/201073/diff/2001/2003#newcode274
Line 274: var queryState = { 'debug': false, 'testType': 'layout_test_results'
};
Please remove ws inside {}

{'debug': ...}

http://codereview.chromium.org/201073/diff/2001/2003#newcode280
Line 280: function() { return isAlphaNumeric(value); });
line break

http://codereview.chromium.org/201073/diff/2001/2003#newcode353
Line 353: function() { return isAlphaNumeric(value); });
line break

http://codereview.chromium.org/201073/diff/2001/2003#newcode358
Line 358: function() { return value in builders; });
line break

http://codereview.chromium.org/201073/diff/2001/2003#newcode385
Line 385: currentState[key] = value == 'true';
same here

http://codereview.chromium.org/201073/diff/2001/2003#newcode843
Line 843: if (i >= currentState.maxResults)
why not put this in the if conditional?

http://codereview.chromium.org/201073/diff/2001/2003#newcode1166
Line 1166: var testsHTML;
var testsHTML = tableRowsHTML ? ...;

http://codereview.chromium.org/201073/diff/2001/2003#newcode1237
Line 1237: state.push(key + '=' + stateObject[key]);
encodeURIComponent?

[ojan, 2009-09-09 23:37:59]

All done.

http://codereview.chromium.org/201073/diff/2001/2003
File webkit/tools/layout_tests/flakiness_dashboard.html (right):

http://codereview.chromium.org/201073/diff/2001/2003#newcode222
Line 222: function isAlphaNumeric(str) {
On 2009/09/09 23:21:04, arv wrote:
> Bad name? How about isValidName or something like that?

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode250
Line 250: function parseParameters(parameterStr, validValueHandler) {
On 2009/09/09 23:21:04, arv wrote:
> To me, it would make more sense to remove ? outside this function

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode274
Line 274: var queryState = { 'debug': false, 'testType': 'layout_test_results'
};
On 2009/09/09 23:21:04, arv wrote:
> Please remove ws inside {}
> 
> {'debug': ...}

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode280
Line 280: function() { return isAlphaNumeric(value); });
On 2009/09/09 23:21:04, arv wrote:
> line break

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode353
Line 353: function() { return isAlphaNumeric(value); });
On 2009/09/09 23:21:04, arv wrote:
> line break

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode358
Line 358: function() { return value in builders; });
On 2009/09/09 23:21:04, arv wrote:
> line break

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode385
Line 385: currentState[key] = value == 'true';
On 2009/09/09 23:21:04, arv wrote:
> same here

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode843
Line 843: if (i >= currentState.maxResults)
On 2009/09/09 23:21:04, arv wrote:
> why not put this in the if conditional?

Done.

http://codereview.chromium.org/201073/diff/2001/2003#newcode1166
Line 1166: var testsHTML;
On 2009/09/09 23:21:04, arv wrote:
> var testsHTML = tableRowsHTML ? ...;

Done.

[arv (Not doing code reviews), 2009-09-09 23:46:25]

lgtm

Just style nits. Single line functions throughout should use multiline
functions.

http://codereview.chromium.org/201073/diff/1005/1007
File webkit/tools/layout_tests/flakiness_dashboard.html (right):

http://codereview.chromium.org/201073/diff/1005/1007#newcode356
Line 356: function() { return isValidName(value); });
function() {
  return isValidName(value);
}