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 tree / graph
 
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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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