Bug 134421 – Enhance Speedometer automation testing

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Tools / Tests

Allow the InteractiveRunner to accept a parameter that would select one of the suites.
If the parameter is passed, the passed suite is selected and started, otherwise enabled tests selected (current state).

[Ryosuke Niwa]

That doesn't sound "interactive" to me.  I think it's better to improve Full.html to take configuration parameters and make them report individual measurement instead.

Created attachment 236947 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 236947 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=236947&action=review

> PerformanceTests/Speedometer/resources/main.js:153
> +function startBenchmark(selectedSuite) {
> +    var enabledSuites = Suites.filter(function (suite) { if (selectedSuite) return selectedSuite == suite.name; return !suite.disabled });

The right approach will be for this function to check the query string
since this is the function responsible for filtering suites.

> PerformanceTests/Speedometer/resources/main.js:201
> +function getQueryVariable(variable) {

WebKit's naming convention doesn't use "get" prefix unless we have an out argument.
I'd call this function queryStringValueForKey(key) if I were you.

> PerformanceTests/Speedometer/resources/main.js:204
> +    var vars = queryString.split("&");
> +    for (var i = 0; i < vars.length; i++) {

Each segment split by & are "pairs".
Also, we've been using single quotation marks for strings so please be consistent and use a single quotation here.

> PerformanceTests/Speedometer/resources/main.js:205
> +        var pair = vars[i].split("=");

Ditto.  s/"/'/g.

> PerformanceTests/Speedometer/resources/main.js:209
> +    return false;

It seems strange to return "false" when there's no value.
I think null or undefined will be better.

[Ryosuke Niwa]

Is this bug still relevant?

No, not any more.

[Csaba Osztrogonác]

Comment on attachment 236947 [details]
Patch

Cleared review? from attachment 236947 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).