Disable all test cases in history_browsertest.cc

Revive some test cases in history_browsertest.cc

This CL adds the (required) call to InProcessBrowserTest::SetUp().
Previously, none of these test cases were executing (but reporting
success). Some test cases do not pass after this, so those have been
disabled.

This CL is being committed to unblock work on http://crbug.com/510649

BUG=511442
Committed: https://crrev.com/6df850272d1ca1414a1895a54d5ba98d3b8ff5a8
Cr-Commit-Position: refs/heads/master@{#340115}

[pval...(no longer on Chromium), 2015-07-22 00:19:22]

pvalenzuela@chromium.org changed reviewers:
+ sdefresne@chromium.org

[pval...(no longer on Chromium), 2015-07-22 17:48:24]

pvalenzuela@chromium.org changed reviewers:
+ sky@chromium.org
- sdefresne@chromium.org

[pval...(no longer on Chromium), 2015-07-22 17:48:25]

R=sky so I can help unblock the parent bug :-)

[sky, 2015-07-22 22:21:01]

Is there a reason you don't want to add the call to the
InProcessBrowserTest::Setup?

[pval...(no longer on Chromium), 2015-07-22 22:59:23]

On 2015/07/22 22:21:01, sky wrote:
> Is there a reason you don't want to add the call to the
> InProcessBrowserTest::Setup?

To be honest, I have not tried adding the InProcessBrowserTest::Setup call here.
I would prefer to commit this CL so that I can get the top-level fix committed
and then some person can re-enable the tests later. I prefer that person to be
someone with more context re: history, but I could try and do it myself later if
no one volunteers.

My goal here is committing https://codereview.chromium.org/1232973002/ (fix the
Chrome-wide problem as described in crbug.com/510649). This is to prevent all
future tests from encountering this problem.

[sky, 2015-07-22 23:58:10]

My concern is you would be disabling a bunch of tests with the hope someone will
look at it later. That leaves in a bad spot with respect to coverage.

[sky, 2015-07-23 00:05:58]

A better approach is to add the check to InProcessBrowserTest::SetUp now with a
way to disable it. The tests that aren't calling super would disable it. This
way no new tests get added that don't call SetUp() and we still have test
coverage.

[pval...(no longer on Chromium), 2015-07-23 00:30:32]

On 2015/07/23 00:05:58, sky wrote:
> A better approach is to add the check to InProcessBrowserTest::SetUp now with
a
> way to disable it. The tests that aren't calling super would disable it. This
> way no new tests get added that don't call SetUp() and we still have test
> coverage.

To be clear: history_browsertest.cc provides no test coverage right now (the
test method contents literally do not run; see crbug.com/510649). The dangerous
part here is that they report success even though no test cases executed.

Disabling this test class + a bug (crbug.com/511442) is much more preferable to
the current situation.

That said, I will try to add the SetUp call and hope some of the test cases are
salvageable. I'll ping back once I've tried this.

[pval...(no longer on Chromium), 2015-07-23 00:51:00]

On 2015/07/23 00:30:32, pvalenzuela wrote:
> On 2015/07/23 00:05:58, sky wrote:
> > A better approach is to add the check to InProcessBrowserTest::SetUp now
with
> a
> > way to disable it. The tests that aren't calling super would disable it.
This
> > way no new tests get added that don't call SetUp() and we still have test
> > coverage.
> 
> To be clear: history_browsertest.cc provides no test coverage right now (the
> test method contents literally do not run; see crbug.com/510649). The
dangerous
> part here is that they report success even though no test cases executed.
> 
> Disabling this test class + a bug (crbug.com/511442) is much more preferable
to
> the current situation.
> 
> That said, I will try to add the SetUp call and hope some of the test cases
are
> salvageable. I'll ping back once I've tried this.

Ok, good news: 18 of the 23 cases work after adding the call. I've disabled the
other 5 and updated the description.

I'm going to do a CQ dry-run to see if these cases pass on all platforms.

[pval...(no longer on Chromium), 2015-07-23 00:51:05]

The CQ bit was checked by pvalenzuela@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-23 00:51:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1247223002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1247223002/20001

[commit-bot: I haz the power, 2015-07-23 01:39:28]

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

[commit-bot: I haz the power, 2015-07-23 01:39:29]

Dry run: This issue passed the CQ dry run.

[sky, 2015-07-23 15:15:52]

Ouch, ok, LGTM

[pval...(no longer on Chromium), 2015-07-23 18:01:20]

The CQ bit was checked by pvalenzuela@chromium.org

[commit-bot: I haz the power, 2015-07-23 18:02:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1247223002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1247223002/20001

[commit-bot: I haz the power, 2015-07-23 18:10:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-23 18:11:25]

Patchset 2 (id:??) landed as
https://crrev.com/6df850272d1ca1414a1895a54d5ba98d3b8ff5a8
Cr-Commit-Position: refs/heads/master@{#340115}