Bug 16142 - Add --range option to run-sunspider
Summary: Add --range option to run-sunspider
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-26 03:47 PST by Eric Seidel (no email)
Modified: 2011-07-02 12:41 PDT (History)
3 users (show)

See Also:


Attachments
partial fix (8.45 KB, patch)
2007-11-26 03:50 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
partial fix (which actually runs) (8.46 KB, patch)
2007-11-26 04:00 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-11-26 03:47:58 PST
Add --range option to run-sunspider

I'd like to have information on exactly how the sunspider results changed, when.  to do that, I've added a --range option to sunspider which takes git/svn ranges and runs sunspider against all revisions in the provided range.

My current patch is incomplete, but I'll attach it anyway.  I'm not sure when I'll next have time to hack, hopefully tomorrow.
Comment 1 Eric Seidel (no email) 2007-11-26 03:50:13 PST
Created attachment 17527 [details]
partial fix

Next step is to add an HTML which actually knows how to use the output.  That will require sunspiderCompareResults() not *always* call printOutput, and instead return an object which can be told to print if needed.
Comment 2 Eric Seidel (no email) 2007-11-26 04:00:11 PST
Created attachment 17528 [details]
partial fix (which actually runs)

The previous patch was missing a semicolon.
Comment 3 Eric Seidel (no email) 2007-11-26 18:13:25 PST
Comment on attachment 17528 [details]
partial fix (which actually runs)

I think this is real enough for review.  I'll eventually come up with some sort of UI, but for now, just having a quick way to generate sunspider numbers per-build is incredibly useful.  Of course this half of the functionality (running something once for each build) really could be abstracted into a separate script.  But the later portion of building a custom UI to display all these will need to be in run-sunspider regardless.
Comment 4 Adam Roben (:aroben) 2007-11-28 00:38:51 PST
Comment on attachment 17528 [details]
partial fix (which actually runs)

 130 $resultsFile = $outputFilePath  if $outputFilePath;

Looks like you've got an extra space in there.

 128     return gitBranch()  if isGit();

Ditto.

 151         return split('\n', $revisionsString);
 152     } elsif (isSVN()) {

 154             return [$1];
 155         } elsif ($range =~ /^r?([0-9]+):?r?([0-9]+|HEAD)$/i) {

No need to say "else" after a return.

 156             return [$1..$2];

What does the range operator do if $1 or $2 is "HEAD"?

 61   --range           Get samples across the provided git range of builds

You shouldn't say "git" here.

 122 # Takes 0 or 1 args
 123 sub runUsingCurrentCheckout

You can still use a prototype:

sub runUsingCurrentCheckout(;$)

 179         last  unless ($result == 0);

Extra space here as well.

It seems silly to do two chdirs for every test run (plus an extra one at the start of the revision range). Can't we do the chdirs ahead of time and just do them once?

"--range" makes me think of a range of times or ranges of error -- maybe there's some more specific name we can give to this option? Maybe --revision-range?

r=me
Comment 5 Eric Seidel (no email) 2007-12-15 01:40:04 PST
Comment on attachment 17528 [details]
partial fix (which actually runs)

Going to clear the review flag given mjs's concerns about the format.  The next person who needs this functionality, and fix the patch.