Re-enable content_browsertests on main waterfall.

Re-enable content_browsertests on main waterfall.

Disabled the two tests that are crashing on android.

TBR=jam
BUG=341142
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249358

[Paweł Hajdan Jr., 2014-02-05 22:42:41]

LGTM with comments

Please make sure to Publish+Mail Comments after uploading the change.

https://codereview.chromium.org/153503005/diff/1/build/android/pylib/gtest/gt...
File build/android/pylib/gtest/gtest_config.py (right):

https://codereview.chromium.org/153503005/diff/1/build/android/pylib/gtest/gt...
build/android/pylib/gtest/gtest_config.py:34: 'content_browsertests',
nit: Let's keep this list sorted.

https://codereview.chromium.org/153503005/diff/1/content/renderer/resource_fe...
File content/renderer/resource_fetcher_browsertest.cc (right):

https://codereview.chromium.org/153503005/diff/1/content/renderer/resource_fe...
content/renderer/resource_fetcher_browsertest.cc:336: // disable these two tests
on android to enable content_browsertest
Please reference the bug (http://crbug.com/...).

Also, start the comment with a capital letter and end with a dot.

It's actually fine to just say // Disabled (http://crbug.com/...).

[zty, 2014-02-06 00:19:12]

The CQ bit was checked by zty@chromium.org

[zty, 2014-02-06 00:19:32]

The CQ bit was unchecked by zty@chromium.org

[zty, 2014-02-06 00:19:33]

The CQ bit was checked by zty@chromium.org

[zty, 2014-02-06 00:19:33]

The CQ bit was unchecked by zty@chromium.org

[zty, 2014-02-06 00:20:03]

The CQ bit was checked by zty@chromium.org

[commit-bot: I haz the power, 2014-02-06 00:22:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zty@chromium.org/153503005/100001

[zty, 2014-02-06 00:29:05]

Added component owner to review

[commit-bot: I haz the power, 2014-02-06 01:01:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-06 01:01:33]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Paweł Hajdan Jr., 2014-02-06 01:05:36]

The CQ bit was checked by phajdan.jr@chromium.org

[commit-bot: I haz the power, 2014-02-06 01:10:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zty@chromium.org/153503005/100001

[navabi, 2014-02-06 01:38:31]

lgtm

Adding mmenke, who along with jam may know how to fix this test so we can
re-enable it.

[mmenke, 2014-02-06 02:04:53]

The CQ bit was unchecked by mmenke@chromium.org

[mmenke, 2014-02-06 02:05:47]

On 2014/02/06 01:38:31, navabi wrote:
> lgtm
> 
> Adding mmenke, who along with jam may know how to fix this test so we can
> re-enable it.

Sorry, unchecking commit flag.  The other tests are disabled by a different
mechanism.  Should disable them all the same way.

[Paweł Hajdan Jr., 2014-02-06 02:07:28]

Please see issue 339980 and the chromium-dev link there. Using DISABLED_ is the
new way.

[mmenke, 2014-02-06 02:10:57]

On 2014/02/06 02:07:28, Paweł Hajdan Jr. wrote:
> Please see issue 339980 and the chromium-dev link there. Using DISABLED_ is
the
> new way.

I'm fine with the new way, but if using the new way, please update the old ones
as well.  Having half disabled on way and half the other seems *very* confusing
to me.

[zty, 2014-02-06 02:46:09]

On 2014/02/06 02:10:57, mmenke wrote:
> On 2014/02/06 02:07:28, Paweł Hajdan Jr. wrote:
> > Please see issue 339980 and the chromium-dev link there. Using DISABLED_ is
> the
> > new way.
> 
> I'm fine with the new way, but if using the new way, please update the old
ones
> as well.  Having half disabled on way and half the other seems *very*
confusing
> to me.

I agree with you. I've updated the CL to make ResourceFetcherTests consistent.
However our main objective this time is to bring back the content_browsertest on
our main waterfall. I don't think it's practical to change all tests to use the
new proper suppression.

[mmenke, 2014-02-06 03:17:27]

LGTM, sorry for delaying the CL, definitely agree that we want to get those
tests running as soon as we can.

https://codereview.chromium.org/153503005/diff/470001/content/renderer/resour...
File content/renderer/resource_fetcher_browsertest.cc (right):

https://codereview.chromium.org/153503005/diff/470001/content/renderer/resour...
content/renderer/resource_fetcher_browsertest.cc:275: // Disable
(http://crbug.com/341142).
nit:  I duped this against the other bug, so you can remove it.

[zty, 2014-02-06 03:18:01]

The CQ bit was checked by zty@chromium.org

[zty, 2014-02-06 03:18:10]

Thanks!

[commit-bot: I haz the power, 2014-02-06 03:18:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zty@chromium.org/153503005/470001

[commit-bot: I haz the power, 2014-02-06 11:22:27]

Change committed as 249358