Move image decoder unit tests to content_unittests

Move image decoder unit tests to content_unittests

We're deleting the concept of a webcore_unit_test from Chromium. I would prefer
to move these tests into the WebKit repository, but they depend on test data in
src-internal. Instead, this CL moves them into content_unittests.

BUG=184276
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187745

[jochen (gone - plz use gerrit), 2013-03-12 20:18:01]

does this change mean that developers need to have src-internal checked out to
run content_unittests?

[abarth-chromium, 2013-03-12 20:19:03]

On 2013/03/12 20:18:01, jochen wrote:
> does this change mean that developers need to have src-internal checked out to
> run content_unittests?

The tests build fine without src-internal, but the image decoder tests
themselves fail without src-internal.

[jochen (gone - plz use gerrit), 2013-03-12 20:20:37]

hum, in that case we might want to mark them as manual tests. we can make bots
execute manual tests as well

[abarth-chromium, 2013-03-12 20:21:20]

On 2013/03/12 20:20:37, jochen wrote:
> hum, in that case we might want to mark them as manual tests. we can make bots
> execute manual tests as well

Ok.  How do I do that?

[jochen (gone - plz use gerrit), 2013-03-12 20:26:35]

On 2013/03/12 20:21:20, abarth wrote:
> On 2013/03/12 20:20:37, jochen wrote:
> > hum, in that case we might want to mark them as manual tests. we can make
bots
> > execute manual tests as well
> 
> Ok.  How do I do that?

AFAIK, adding a MANUAL_ prefix to the test names should do the trick.

We'll then need to modify the bots to add the --manual flag to content_unittests
if they have a src-internal checkout

[abarth-chromium, 2013-03-12 20:31:19]

Done.

[abarth-chromium, 2013-03-12 20:31:50]

+brettw for content/test OWNERS review

[jam, 2013-03-12 20:34:02]

On 2013/03/12 20:26:35, jochen wrote:
> On 2013/03/12 20:21:20, abarth wrote:
> > On 2013/03/12 20:20:37, jochen wrote:
> > > hum, in that case we might want to mark them as manual tests. we can make
> bots
> > > execute manual tests as well
> > 
> > Ok.  How do I do that?
> 
> AFAIK, adding a MANUAL_ prefix to the test names should do the trick.
> 
> We'll then need to modify the bots to add the --manual flag to
content_unittests
> if they have a src-internal checkout

why make the manual? why not have them early exit if the test data isn't there?
that's what tests in content_browsertests do (i.e. see
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
)

[jochen (gone - plz use gerrit), 2013-03-12 20:36:11]

On 2013/03/12 20:34:02, jam wrote:
> On 2013/03/12 20:26:35, jochen wrote:
> > On 2013/03/12 20:21:20, abarth wrote:
> > > On 2013/03/12 20:20:37, jochen wrote:
> > > > hum, in that case we might want to mark them as manual tests. we can
make
> > bots
> > > > execute manual tests as well
> > > 
> > > Ok.  How do I do that?
> > 
> > AFAIK, adding a MANUAL_ prefix to the test names should do the trick.
> > 
> > We'll then need to modify the bots to add the --manual flag to
> content_unittests
> > if they have a src-internal checkout
> 
> why make the manual? why not have them early exit if the test data isn't
there?
> that's what tests in content_browsertests do (i.e. see
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
> )

ok

my concern was that we end up silently skipping the tests everywhere if there's
a problem with checking out the data, and nobody notices

[jam, 2013-03-12 20:42:02]

On 2013/03/12 20:36:11, jochen wrote:
> On 2013/03/12 20:34:02, jam wrote:
> > On 2013/03/12 20:26:35, jochen wrote:
> > > On 2013/03/12 20:21:20, abarth wrote:
> > > > On 2013/03/12 20:20:37, jochen wrote:
> > > > > hum, in that case we might want to mark them as manual tests. we can
> make
> > > bots
> > > > > execute manual tests as well
> > > > 
> > > > Ok.  How do I do that?
> > > 
> > > AFAIK, adding a MANUAL_ prefix to the test names should do the trick.
> > > 
> > > We'll then need to modify the bots to add the --manual flag to
> > content_unittests
> > > if they have a src-internal checkout
> > 
> > why make the manual? why not have them early exit if the test data isn't
> there?
> > that's what tests in content_browsertests do (i.e. see
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
> > )
> 
> ok
> 
> my concern was that we end up silently skipping the tests everywhere if
there's
> a problem with checking out the data, and nobody notices

yeah, that's the tradeoff. My feelings on this is that that would be very rare
in-practice. The alternative of making them MANUAL_ means that server scripts
and a master restart is needed when tests are changed.

note that MANUAL_ is meant to be combined with --gtest_filter. i.e. we wouldn't
want to have the master pass --run-manual without a filter on the buildbots.
Otherwise it would defeat the purpose of adding MANUAL_, which is to have tests
that don't run on bots by default.

https://codereview.chromium.org/12725006/diff/1012/content/test/image_decoder...
File content/test/image_decoder_test.h (right):

https://codereview.chromium.org/12725006/diff/1012/content/test/image_decoder...
content/test/image_decoder_test.h:5: #ifndef
WEBKIT_TOOLS_TEST_SHELL_IMAGE_DECODER_UNITTEST_H_
nit: update

[abarth-chromium, 2013-03-12 20:44:40]

Comments addressed.

[jochen (gone - plz use gerrit), 2013-03-12 20:49:54]

On 2013/03/12 20:42:02, jam wrote:
> On 2013/03/12 20:36:11, jochen wrote:
> > On 2013/03/12 20:34:02, jam wrote:
> > > On 2013/03/12 20:26:35, jochen wrote:
> > > > On 2013/03/12 20:21:20, abarth wrote:
> > > > > On 2013/03/12 20:20:37, jochen wrote:
> > > > > > hum, in that case we might want to mark them as manual tests. we can
> > make
> > > > bots
> > > > > > execute manual tests as well
> > > > > 
> > > > > Ok.  How do I do that?
> > > > 
> > > > AFAIK, adding a MANUAL_ prefix to the test names should do the trick.
> > > > 
> > > > We'll then need to modify the bots to add the --manual flag to
> > > content_unittests
> > > > if they have a src-internal checkout
> > > 
> > > why make the manual? why not have them early exit if the test data isn't
> > there?
> > > that's what tests in content_browsertests do (i.e. see
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/pl...
> > > )
> > 
> > ok
> > 
> > my concern was that we end up silently skipping the tests everywhere if
> there's
> > a problem with checking out the data, and nobody notices
> 
> yeah, that's the tradeoff. My feelings on this is that that would be very rare
> in-practice. The alternative of making them MANUAL_ means that server scripts
> and a master restart is needed when tests are changed.
> 
> note that MANUAL_ is meant to be combined with --gtest_filter. i.e. we
wouldn't
> want to have the master pass --run-manual without a filter on the buildbots.
> Otherwise it would defeat the purpose of adding MANUAL_, which is to have
tests
> that don't run on bots by default.
> 

ok that seems reasonable

>
https://codereview.chromium.org/12725006/diff/1012/content/test/image_decoder...
> File content/test/image_decoder_test.h (right):
> 
>
https://codereview.chromium.org/12725006/diff/1012/content/test/image_decoder...
> content/test/image_decoder_test.h:5: #ifndef
> WEBKIT_TOOLS_TEST_SHELL_IMAGE_DECODER_UNITTEST_H_
> nit: update

[jam, 2013-03-12 20:51:13]

lgtm

[tony, 2013-03-12 20:57:39]

LGTM

[commit-bot: I haz the power, 2013-03-12 21:02:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/12725006/16001

[commit-bot: I haz the power, 2013-03-13 02:01:04]

Change committed as 187745