Increase timeout in WebappActivityTestBase.waitUntilIdle

Increase timeout in WebappActivityTestBase.waitUntilIdle

This change increases the timeout in
WebappActivityTestBase.waitUntilIdle and re-enables one test
that was failing on M testers due to a too short timeout.

BUG=614338
Committed: https://crrev.com/6db590fb7126cbf0c9a606b2da0aaf1c9968c687
Cr-Commit-Position: refs/heads/master@{#402678}

[kraush, 2016-06-28 20:56:44]

kraush@amazon.com changed reviewers:
+ dfalcantara@chromium.org

[kraush, 2016-06-28 20:56:44]

Hi dfalcantara,

Can you take a look at this test stabilization? (And re-enabling it for M
testers)

Thanks! :)
Holger

[gone, 2016-06-28 21:08:52]

Eh, I'm hesitant to allow this one in with such a large timeout.  Scaling a 10
second timeout on Android One devices, for example, multiplies that out to be a
100 second timeout, which means that the bots will idle for a ridiculously long
time until failing if there's a legitimate failure.  Moreover, this affects all
Webapp*Test URL loading times -- not just the one on startup.

Is 10 seconds absolutely required, or will 5 work?

[kraush, 2016-06-28 21:18:20]

On 2016/06/28 21:08:52, dfalcantara wrote:
> Eh, I'm hesitant to allow this one in with such a large timeout.  Scaling a 10
> second timeout on Android One devices, for example, multiplies that out to be
a
> 100 second timeout, which means that the bots will idle for a ridiculously
long
> time until failing if there's a legitimate failure.  Moreover, this affects
all
> Webapp*Test URL loading times -- not just the one on startup.
> 
> Is 10 seconds absolutely required, or will 5 work?

Good point - I'll run 100 test runs with 5 seconds and see how it goes.
That would only cover our test devices though - I don't know what you use for
Android M testing and what number would be best for them,

I didn't realize Android One uses 10 times the timeout - that would indeed be a
super long timeout!
What's the rules on scaleTimeout in general?
Would it be viable to just take 10 seconds without a scaleTimeout?

[gone, 2016-06-28 21:58:07]

On 2016/06/28 21:18:20, kraush wrote:
> On 2016/06/28 21:08:52, dfalcantara wrote:
> > Eh, I'm hesitant to allow this one in with such a large timeout.  Scaling a
10
> > second timeout on Android One devices, for example, multiplies that out to
be
> a
> > 100 second timeout, which means that the bots will idle for a ridiculously
> long
> > time until failing if there's a legitimate failure.  Moreover, this affects
> all
> > Webapp*Test URL loading times -- not just the one on startup.
> > 
> > Is 10 seconds absolutely required, or will 5 work?
> 
> Good point - I'll run 100 test runs with 5 seconds and see how it goes.
> That would only cover our test devices though - I don't know what you use for
> Android M testing and what number would be best for them,
We seem to use a Nexus 7 (2014) for M tablet testing on the bots, and a Nexus 5X
for M phone tests:
https://build.chromium.org/p/chromium.android/builders/Marshmallow%20Tablet%2...
https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit...

> I didn't realize Android One uses 10 times the timeout - that would indeed be
a
> super long timeout!
> What's the rules on scaleTimeout in general?

The rules are buried somewhere in build scripts that I'm not terribly familiar
with; here's a CL that edits the timeouts:
https://codereview.chromium.org/2073633002

For that particular CL, we'd forgotten to increase the scale when the Svelte
(low memory) devices were moved to the open source repo.  It seems the timeouts
have shrunk to just 2X, though, so down to 20 seconds for a failure on those
devices.  Not as awful as it used to be, so a scaled 10 doesn't sound too bad
now.  If you could still check for a 5 second timeout that'd be great, though.

> Would it be viable to just take 10 seconds without a scaleTimeout?
Eh, I'd prefer to scale when applicable.  The base CriteriaHelper class (which a
lot of tests use) scales a timeout of 3000 milliseconds.

[kraush, 2016-06-28 23:56:21]

On 2016/06/28 21:58:07, dfalcantara wrote:
> On 2016/06/28 21:18:20, kraush wrote:
> > On 2016/06/28 21:08:52, dfalcantara wrote:
> > > Eh, I'm hesitant to allow this one in with such a large timeout.  Scaling
a
> 10
> > > second timeout on Android One devices, for example, multiplies that out to
> be
> > a
> > > 100 second timeout, which means that the bots will idle for a ridiculously
> > long
> > > time until failing if there's a legitimate failure.  Moreover, this
affects
> > all
> > > Webapp*Test URL loading times -- not just the one on startup.
> > > 
> > > Is 10 seconds absolutely required, or will 5 work?
> > 
> > Good point - I'll run 100 test runs with 5 seconds and see how it goes.
> > That would only cover our test devices though - I don't know what you use
for
> > Android M testing and what number would be best for them,
> We seem to use a Nexus 7 (2014) for M tablet testing on the bots, and a Nexus
5X
> for M phone tests:
>
https://build.chromium.org/p/chromium.android/builders/Marshmallow%20Tablet%2...
>
https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit...
> 
> > I didn't realize Android One uses 10 times the timeout - that would indeed
be
> a
> > super long timeout!
> > What's the rules on scaleTimeout in general?
> 
> The rules are buried somewhere in build scripts that I'm not terribly familiar
> with; here's a CL that edits the timeouts:
> https://codereview.chromium.org/2073633002
> 
> For that particular CL, we'd forgotten to increase the scale when the Svelte
> (low memory) devices were moved to the open source repo.  It seems the
timeouts
> have shrunk to just 2X, though, so down to 20 seconds for a failure on those
> devices.  Not as awful as it used to be, so a scaled 10 doesn't sound too bad
> now.  If you could still check for a 5 second timeout that'd be great, though.
> 
> > Would it be viable to just take 10 seconds without a scaleTimeout?
> Eh, I'd prefer to scale when applicable.  The base CriteriaHelper class (which
a
> lot of tests use) scales a timeout of 3000 milliseconds.

With 5 seconds, I'm still getting 23/100 failing :(
While retries would mitigate that somewhat, I'd rather have a higher number for
now to not rely on retries.
Let me know what you think.

[gone, 2016-06-28 23:58:01]

Agreed.  lgtm.  Thanks for looking into this!

[kraush, 2016-06-28 23:58:15]

Forgot the import in diff 1 - fixed.

I've left it at 10 seconds for now (see data in previous post)

Please let me know what you think :)

[kraush, 2016-06-28 23:58:41]

The CQ bit was checked by kraush@amazon.com

[commit-bot: I haz the power, 2016-06-29 00:00:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 02:30:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-29 02:32:29]

Description was changed from

==========
Increase timeout in WebappActivityTestBase.waitUntilIdle

This change increases the timeout in
WebappActivityTestBase.waitUntilIdle and re-enables one test
that was failing on M testers due to a too short timeout.

BUG=614338
==========

to

==========
Increase timeout in WebappActivityTestBase.waitUntilIdle

This change increases the timeout in
WebappActivityTestBase.waitUntilIdle and re-enables one test
that was failing on M testers due to a too short timeout.

BUG=614338

Committed: https://crrev.com/6db590fb7126cbf0c9a606b2da0aaf1c9968c687
Cr-Commit-Position: refs/heads/master@{#402678}
==========

[commit-bot: I haz the power, 2016-06-29 02:32:30]

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