Summary: | Enhance Speedometer automation testing | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | fmeawad | ||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | fmeawad, rniwa | ||||
Priority: | P3 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
fmeawad
2014-06-27 16:39:07 PDT
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
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. Is this bug still relevant? No, not any more. 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). |