Bug 175908 - Speedometer 2.0: Add the capability to run a specific suite
Summary: Speedometer 2.0: Add the capability to run a specific suite
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks: 172339
  Show dependency treegraph
 
Reported: 2017-08-23 14:56 PDT by Ryosuke Niwa
Modified: 2017-08-23 17:00 PDT (History)
6 users (show)

See Also:


Attachments
Adds the feature (2.99 KB, patch)
2017-08-23 15:12 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Updated change log (3.14 KB, patch)
2017-08-23 15:13 PDT, Ryosuke Niwa
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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