- Add UI test for SunSpider....

- Add UI test for SunSpider.
- Modify SunSpider to work with the UI test framework.

BUG=8785
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12465

[Pam (message me for reviews), 2009-03-24 23:37:53]

Nice. I'd been thinking we'd just turn sunspider back on as it was before, at
least for now, but this is much nicer.

http://codereview.chromium.org/42581/diff/12/1019
File chrome/test/ui/sunspider_uitest.cc (right):

http://codereview.chromium.org/42581/diff/12/1019#newcode74
Line 74: return false;
Why these, here and below, rather than ASSERT_TRUE()?

http://codereview.chromium.org/42581/diff/12/1019#newcode139
Line 139: std::string trace = reference_ ? "reference" : "";
I'm not sure how well it works to have a trace with an empty name.

Also, if the reference trace name matches the main name plus "_ref", the plotter
framework will be able to handle it specially (put its value in parentheses on
the waterfall, make sure it's plotted immediately after its main trace on the
graph, etc.).

So I think you want "score" and "score_ref", or whatever it is that's being
measured.

[Patrick Johnson, 2009-03-25 00:50:09]

http://codereview.chromium.org/42581/diff/12/1019
File chrome/test/ui/sunspider_uitest.cc (right):

http://codereview.chromium.org/42581/diff/12/1019#newcode74
Line 74: return false;
On 2009/03/24 23:37:53, Pam wrote:
> Why these, here and below, rather than ASSERT_TRUE()?

I tried using ASSERT_TRUE(), but that causes a compiler error:

error C2440: 'return' : cannot convert from 'void' to 'bool'

I suspect, but haven't verified, that the ASSERT* functions will just insert a
'return;', assuming the return type of the function is void.

http://codereview.chromium.org/42581/diff/12/1019#newcode139
Line 139: std::string trace = reference_ ? "reference" : "";
On 2009/03/24 23:37:53, Pam wrote:
> I'm not sure how well it works to have a trace with an empty name.

Right, my mistake.  Good catch.

> Also, if the reference trace name matches the main name plus "_ref", the
plotter
> framework will be able to handle it specially (put its value in parentheses on
> the waterfall, make sure it's plotted immediately after its main trace on the
> graph, etc.).
> 
> So I think you want "score" and "score_ref", or whatever it is that's being
> measured.

Neat, thanks for the tip.  Done.

[Pam (message me for reviews), 2009-03-25 16:34:58]

LGTM; one optional comment below.

- Pam

http://codereview.chromium.org/42581/diff/12/1019
File chrome/test/ui/sunspider_uitest.cc (right):

http://codereview.chromium.org/42581/diff/12/1019#newcode74
Line 74: return false;
On 2009/03/25 00:50:09, Patrick Johnson wrote:
> On 2009/03/24 23:37:53, Pam wrote:
> > Why these, here and below, rather than ASSERT_TRUE()?
> 
> I tried using ASSERT_TRUE(), but that causes a compiler error:
> 
> error C2440: 'return' : cannot convert from 'void' to 'bool'
> 
> I suspect, but haven't verified, that the ASSERT* functions will just insert a
> 'return;', assuming the return type of the function is void.

Makes sense. Perhaps add a comment to that effect, since neither you nor I
expected it?

[Patrick Johnson, 2009-03-25 17:52:12]

Thanks for the review!

http://codereview.chromium.org/42581/diff/12/1019
File chrome/test/ui/sunspider_uitest.cc (right):

http://codereview.chromium.org/42581/diff/12/1019#newcode74
Line 74: return false;
On 2009/03/25 16:34:58, Pam wrote:
> On 2009/03/25 00:50:09, Patrick Johnson wrote:
> > On 2009/03/24 23:37:53, Pam wrote:
> > > Why these, here and below, rather than ASSERT_TRUE()?
> > 
> > I tried using ASSERT_TRUE(), but that causes a compiler error:
> > 
> > error C2440: 'return' : cannot convert from 'void' to 'bool'
> > 
> > I suspect, but haven't verified, that the ASSERT* functions will just insert
a
> > 'return;', assuming the return type of the function is void.
> 
> Makes sense. Perhaps add a comment to that effect, since neither you nor I
> expected it?
> 

Yeah, a comment could save someone time/confusion.  Done.