Suspend redness on touch build.

These tests fail in the touch environment, and we would like to queue fixing them behind keeping that build working as well as it does today.

BUG=84854, 84855
TEST=none

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

[Peter Mayo, 2011-06-06 19:46:06]

Please let me know if I should add anyone else to the review, and what the
timeframe of feedback is if it isn't today.


Also note - there were two simultaneous linux try jobs for this CL, only one
failed.  See
http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/30624 for
the one that worked  :-)

[sadrul, 2011-06-06 19:58:30]

http://codereview.chromium.org/7112013/diff/2001/chrome/browser/search_engine...
File chrome/browser/search_engines/template_url_parser_unittest.cc (right):

http://codereview.chromium.org/7112013/diff/2001/chrome/browser/search_engine...
chrome/browser/search_engines/template_url_parser_unittest.cc:18: #define
MAYBE_TestWikipedia FAILS_TestWikipedia
FYI: http://codereview.chromium.org/7065052/ should fix these.

[Peter Mayo, 2011-06-06 20:25:08]

> FYI: http://codereview.chromium.org/7065052/ should fix these.

Good to know, but I can't wait on this.  I will ping that owner to make sure he
adds re-enabling the test to his CL.

[sky, 2011-06-06 21:22:50]

I think the right thing to do for the /template_url_parser_unittest is to have
the test explicitly use 16x16, instead of kFaviconSize.

Your description should have:
BUG=84854
TEST=none

Also, please update bug 84854 to include how it fails. Are any asserts thrown?
Does the test hang?

[Peter Mayo, 2011-06-06 23:04:28]

On 2011/06/06 21:22:50, sky wrote:
> I think the right thing to do for the /template_url_parser_unittest is to have
> the test explicitly use 16x16, instead of kFaviconSize.

Misplaced comment?

> 
> Your description should have:
> BUG=84854
> TEST=none

Done, referencing both bugs.  84854 & 84855

> 
> Also, please update bug 84854 to include how it fails. Are any asserts thrown?
> Does the test hang?

Done.
FYI:
 84854 Yes, No.
 84855 No, No.

[sky, 2011-06-06 23:18:07]

On Mon, Jun 6, 2011 at 4:04 PM,  <petermayo@chromium.org> wrote:
> On 2011/06/06 21:22:50, sky wrote:
>>
>> I think the right thing to do for the /template_url_parser_unittest is to
>> have
>> the test explicitly use 16x16, instead of kFaviconSize.
>
> Misplaced comment?

Not really, but I go back and forth on this. I was saying that if you
force TemplateURL to use 16x16 explicitly rather than kFaviconSize,
then the tests will pass on the touch build. I think a better idea is
to have create different xml test files for the touch build that have
the right size in them. For example, to fix TestDictionary create a
copy of dictionary.xml, modify the width/height to 32x32 and change
the test to load the copy for touch builds.

  -Scott

>
>
>> Your description should have:
>> BUG=84854
>> TEST=none
>
> Done, referencing both bugs.  84854 & 84855
>
>
>> Also, please update bug 84854 to include how it fails. Are any asserts
>> thrown?
>> Does the test hang?
>
> Done.
> FYI:
>  84854 Yes, No.
>  84855 No, No.
>
>
>
> http://codereview.chromium.org/7112013/
>

[Peter Mayo, 2011-06-07 22:31:08]

On 2011/06/06 23:18:07, sky wrote:
> On Mon, Jun 6, 2011 at 4:04 PM,  <mailto:petermayo@chromium.org> wrote:
> > On 2011/06/06 21:22:50, sky wrote:
> >>
> >> I think the right thing to do for the /template_url_parser_unittest is to
> >> have
> >> the test explicitly use 16x16, instead of kFaviconSize.
> >
> > Misplaced comment?
> 
> Not really, but I go back and forth on this. I was saying that if you
> force TemplateURL to use 16x16 explicitly rather than kFaviconSize,
> then the tests will pass on the touch build. I think a better idea is
> to have create different xml test files for the touch build that have
> the right size in them. For example, to fix TestDictionary create a
> copy of dictionary.xml, modify the width/height to 32x32 and change
> the test to load the copy for touch builds.
> 
>   -Scott
> 

I'd like to separate that into a separate CL, on a slightly delayed timeframe.

Peter.

[sky, 2011-06-07 23:15:37]

LGTM

[commit-bot: I haz the power, 2011-06-08 01:44:41]

Change committed as 88265