Reenabling all the SavePageBrowserTest tests.

Reenabling all the SavePageBrowserTest tests.

The tests were previously disabled because of
1. Flakiness (crbug.com/162323)
2. Temporary disabling to aid with serializers merge
   (crrev.com/1218743006 which was not fully reverted by
   crrev.com/1301503003 and crrev.com/1219923010).

I am reenabling tests that were marked as flaky (using MAYBE_foo
pattern).  I cannot repro the flakiness (that according to the code was occuring
only on Windows), so it seems worth to try reenabling the tests to see if
they really are flaky (or if one of change in the past few years has
accidentally fixed the flakiness).

I am also re-enabling tests (i.e. SaveCompleteHTML and
FileNameFromPageTitle) that were meant to be only *temporarily* disabled
to aid with serializers merge.  I could have reenabled into a MAYBE_foo
flakiness pattern, but as explained in the previous paragraph, it seems
better to reenable uncoditionally.

And finally, I am also enabling one test (SaveHTMLOnlyCancel) that
1) wasn't marked as flaky (i.e. was unconditionally disabled) and
2) wasn't covered by the temporary disabling for serializers merge.
This test passed in 100 iterations on my machine, so as with the tests
above it seems worth to try re-enabling this test.

TEST=out\Release\browser_tests.exe --gtest_filter=*SavePage*BrowserTest* --gtest_repeat=100
BUG=162323
Committed: https://crrev.com/04019ba7a7a843a5da52321254c81af2ec44625a
Cr-Commit-Position: refs/heads/master@{#357598}

[Łukasz Anforowicz, 2015-10-20 19:03:22]

lukasza@chromium.org changed reviewers:
+ rdsmith@chromium.org

[Łukasz Anforowicz, 2015-10-20 19:03:22]

Randy, could you please take a look?

WDYT?
1. Seems worth it to try reenabling supposedly flaky tests that don't seem flaky
anymore
2. I am concerned that some tests got temporarily disabled but there was no
follow-up to enable them later on

[Łukasz Anforowicz, 2015-10-20 21:05:43]

Description was changed from

==========
Reenabling all the SavePageBrowserTest tests.

The tests were previously disabled because of
1. Flakiness (crbug.com/162323)
2. Temporary disabling to aid with serializers merge
   (crrev.com/1218743006 which was not fully reverted by
   crrev.com/1301503003 and crrev.com/1219923010).

I am reenabling tests that were marked as flaky (using MAYBE_foo
pattern).  I cannot repro the flakiness (that according to the code was occuring
only on Windows), so it seems worth to try reenabling the tests to see if
they really are flaky (or if one of change in the past few years has
accidentally fixed the flakiness).

I am also re-enabling tests (i.e. SaveCompleteHTML and
FileNameFromPageTitle) that were meant to be only *temporarily* disabled
to aid with serializers merge.  I could have reenabled into a MAYBE_foo
flakiness pattern, but as explained in the previous paragraph, it seems
better to reenable uncoditionally.

And finally, I am also enabling one test (SaveHTMLOnlyCancel) that
1) wasn't marked as flaky (i.e. was unconditionally disabled) and
2) wasn't covered by the temporary disabling for serializers merge.
This test passed in 100 iterations on my machine, so as with the tests
above it seems worth to try re-enabling this test.

TEST=out\Release\browser_tests.exe --gtest_filter=*SavePage*BrowserTest*
--gtest_repeat=100
BUG=162323
==========

to

==========
Reenabling all the SavePageBrowserTest tests.

The tests were previously disabled because of
1. Flakiness (crbug.com/162323)
2. Temporary disabling to aid with serializers merge
   (crrev.com/1218743006 which was not fully reverted by
   crrev.com/1301503003 and crrev.com/1219923010).

I am reenabling tests that were marked as flaky (using MAYBE_foo
pattern).  I cannot repro the flakiness (that according to the code was occuring
only on Windows), so it seems worth to try reenabling the tests to see if
they really are flaky (or if one of change in the past few years has
accidentally fixed the flakiness).

I am also re-enabling tests (i.e. SaveCompleteHTML and
FileNameFromPageTitle) that were meant to be only *temporarily* disabled
to aid with serializers merge.  I could have reenabled into a MAYBE_foo
flakiness pattern, but as explained in the previous paragraph, it seems
better to reenable uncoditionally.

And finally, I am also enabling one test (SaveHTMLOnlyCancel) that
1) wasn't marked as flaky (i.e. was unconditionally disabled) and
2) wasn't covered by the temporary disabling for serializers merge.
This test passed in 100 iterations on my machine, so as with the tests
above it seems worth to try re-enabling this test.

TEST=out\Release\browser_tests.exe --gtest_filter=*SavePage*BrowserTest*
--gtest_repeat=100
BUG=162323
==========

[Randy Smith (Not in Mondays), 2015-10-21 17:57:42]

Thanks for doing this (and, I presume, tracking for flakiness going forward
:-}); I agree with your judgement that, given that you can't repro the flake,
it's worth just re-enabling everything and seeing what happens.

LGTM.

[Łukasz Anforowicz, 2015-10-21 22:56:28]

On 2015/10/21 17:57:42, rdsmith wrote:
> Thanks for doing this
> (and, I presume, tracking for flakiness going forward :-})

Any hints for this?  I guess the baseline option is to land and do nothing until
I get an angry email from an engineer that gets affected (if the flakiness is
still there, which we're hopeing it's not the case).  Is there some piece of
infrastructure where I can say: "If test such-and-such fails in the next X days,
then let me know, because I am most likely responsible for this failure"? 
Hmmm... I guess I can try asking on chromium-dev...

> I agree with your judgement that, given that you can't repro the flake,
> it's worth just re-enabling everything and seeing what happens.

Ack.

> LGTM.

Thanks.  I'll probably hold off with landing this CL until the dust settles down
from the other changes in this area (so if any flakiness reappears, we will know
if it is from this CL vs from the other CLs).

[Randy Smith (Not in Mondays), 2015-10-22 16:57:19]

On 2015/10/21 22:56:28, Łukasz Anforowicz wrote:
> On 2015/10/21 17:57:42, rdsmith wrote:
> > Thanks for doing this
> > (and, I presume, tracking for flakiness going forward :-})
> 
> Any hints for this?  I guess the baseline option is to land and do nothing
until
> I get an angry email from an engineer that gets affected (if the flakiness is
> still there, which we're hopeing it's not the case).  Is there some piece of
> infrastructure where I can say: "If test such-and-such fails in the next X
days,
> then let me know, because I am most likely responsible for this failure"? 
> Hmmm... I guess I can try asking on chromium-dev...

Other than the flakiness dashboard
(http://dev.chromium.org/developers/testing/flakiness-dashboard) I don't know of
anything.  I've also occasionally just emailed the next 2-3 shifts of sheriffs.

> 
> > I agree with your judgement that, given that you can't repro the flake,
> > it's worth just re-enabling everything and seeing what happens.
> 
> Ack.
> 
> > LGTM.
> 
> Thanks.  I'll probably hold off with landing this CL until the dust settles
down
> from the other changes in this area (so if any flakiness reappears, we will
know
> if it is from this CL vs from the other CLs).

Makes sense.

[Łukasz Anforowicz, 2015-10-26 16:31:36]

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

[commit-bot: I haz the power, 2015-10-26 16:32:43]

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

[commit-bot: I haz the power, 2015-10-26 17:04:40]

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

[commit-bot: I haz the power, 2015-10-26 17:04:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Łukasz Anforowicz, 2015-11-02 23:24:15]

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

[commit-bot: I haz the power, 2015-11-02 23:25:16]

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

[commit-bot: I haz the power, 2015-11-03 00:32:32]

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

[commit-bot: I haz the power, 2015-11-03 00:32:32]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2015-11-03 19:49:42]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2015-11-03 19:49:43]

The patchset sent to the CQ was uploaded after l-g-t-m from rdsmith@chromium.org
Link to the patchset: https://codereview.chromium.org/1413763008/#ps20001
(title: "Rebasing + adjusting reference files (b.saved1.htm and b.saved2.htm).")

[commit-bot: I haz the power, 2015-11-03 19:52:01]

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

[commit-bot: I haz the power, 2015-11-03 20:10:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-03 20:11:01]

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