Bug 175908

Summary: Speedometer 2.0: Add the capability to run a specific suite
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: Tools / TestsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cdumez, joepeck, lforschler, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 172339    
Attachments:
Description Flags
Adds the feature
none
Updated change log saam: review+

Description Ryosuke Niwa 2017-08-23 14:56:20 PDT
Add the capability to run a specific test suite to Speedometer's main test runner.
Comment 1 Ryosuke Niwa 2017-08-23 15:12:20 PDT
Created attachment 318926 [details]
Adds the feature
Comment 2 Ryosuke Niwa 2017-08-23 15:13:19 PDT
Created attachment 318928 [details]
Updated change log
Comment 3 Ryosuke Niwa 2017-08-23 16:04:15 PDT
Committed r221114: <http://trac.webkit.org/changeset/221114>
Comment 4 Radar WebKit Bug Importer 2017-08-23 16:04:49 PDT
<rdar://problem/34046669>
Comment 5 Joseph Pecoraro 2017-08-23 16:12:40 PDT
Comment on attachment 318928 [details]
Updated change log

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

> PerformanceTests/Speedometer/resources/main.js:160
> +        if (currentSuite.name.toLowerCase() == suiteToEnable) {
> +            currentSuite.disabled = false;
> +            found = true;

Can there be duplicate names? If not, then you can `return true` here instead of continuing to loop.

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

This seems to be a duplicate of line 200 with no change between now and then. Seems this line can be removed.

> PerformanceTests/Speedometer/resources/main.js:192
> +                    alert('No tests to run');

This might be more useful to include the name of the one test that no longer exists:

    alert('Suite "' + value + '" does not exist. No tests to run.');
Comment 6 Ryosuke Niwa 2017-08-23 17:00:10 PDT
(In reply to Joseph Pecoraro from comment #5)
> Comment on attachment 318928 [details]
> Updated change log
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318928&action=review
> 
> > PerformanceTests/Speedometer/resources/main.js:160
> > +        if (currentSuite.name.toLowerCase() == suiteToEnable) {
> > +            currentSuite.disabled = false;
> > +            found = true;
> 
> Can there be duplicate names? If not, then you can `return true` here
> instead of continuing to loop.

No, but we still have to set currentSuite.disabled to true in all other suite objects.

> > PerformanceTests/Speedometer/resources/main.js:168
> > +    var enabledSuites = Suites.filter(function (suite) { return !suite.disabled; });
> 
> This seems to be a duplicate of line 200 with no change between now and
> then. Seems this line can be removed.

Oops, removed.

> > PerformanceTests/Speedometer/resources/main.js:192
> > +                    alert('No tests to run');
> 
> This might be more useful to include the name of the one test that no longer
> exists:
> 
>     alert('Suite "' + value + '" does not exist. No tests to run.');

Fixed.
Comment 7 Ryosuke Niwa 2017-08-23 17:00:57 PDT
https://trac.webkit.org/changeset/221118