Remove the net-internals dependency from chrome proxy telemetry tests.

Remove the net-internals dependency from chrome proxy telemetry tests.

This change fully removes any dependency on chrome://net-internals from
the data reduction proxy telemetry tests. Also, The ExplicitBypass test
has now been replaced with the ReenableAfterBypass test, which verifies
that the data reduction proxy will be re-enabled after a bypass expires.

The tests can now all run on android webview, although several of the
tests are for features that webview does not support: namely
safebrowsing and the HTTP fallback proxy tests.

BUG=440522, 447317, 447054
Committed: https://crrev.com/6d76812fe3713cfcbb55818c40f75cee092b9de8
Cr-Commit-Position: refs/heads/master@{#311609}

[sclittle, 2015-01-14 01:31:54]

sclittle@chromium.org changed reviewers:
+ bengr@chromium.org, bolian@chromium.org

[sclittle, 2015-01-14 01:31:55]

bengr, bolian: all

Thanks in advance!

[bolian, 2015-01-14 07:01:04]

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:320: # The very
first response should be through the HTTP fallback proxy.
Is this comment true?  I am not sure the order is guaranteed.

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics_unittest.py
(right):

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics_unittest.py:208: 
Also add test for the new funcs?  At least add a TODO?

[sclittle, 2015-01-14 18:55:24]

Patchset #2 (id:20001) has been deleted

[sclittle, 2015-01-14 19:20:51]

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:320: # The very
first response should be through the HTTP fallback proxy.
On 2015/01/14 07:01:04, bolian wrote:
> Is this comment true?  I am not sure the order is guaranteed.

We're iterating through a list of timeline events, which implies that they would
be iterated in chronological order according to when they occurred. They're
certainly currently implemented to be returned in chronological order.

The test waits for the page load through the fallback proxy to finish before
continuing on with the rest of the test, so the fallback proxy response will
occur before the bypassed ones.

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics_unittest.py
(right):

https://codereview.chromium.org/792303006/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics_unittest.py:208: 
On 2015/01/14 07:01:04, bolian wrote:
> Also add test for the new funcs?  At least add a TODO?

Sure, I've added new unit tests, except for ReenableAfterBypass since it's not
really testable.

[bolian, 2015-01-14 23:33:03]

lgtm

[bengr, 2015-01-15 00:06:09]

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py (right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310: for 1 to
5 minutes, then waits 5 minutes and verifies that the proxy is no
Yikes. I don't think it is a good idea to have a test pause for 5 minutes.

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait until
30 seconds after the bypass should expire, and fetch a page. It
Is there no way to get around such delays?

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
(right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
bypass_seconds_low: The minimum number of seconds that the bypass
Why aren't these bypass_seconds_min/max?

[sclittle, 2015-01-15 00:44:35]

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py (right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310: for 1 to
5 minutes, then waits 5 minutes and verifies that the proxy is no
On 2015/01/15 00:06:09, bengr wrote:
> Yikes. I don't think it is a good idea to have a test pause for 5 minutes.

In order to test that the proxy is re-enabled after the bypass expires, we need
to wait for the bypass to expire, then see if the proxy gets used. Even using
net-internals#proxy like we were before wouldn't catch bugs in the net stack
proxy retry code.

I suppose we could do block=60 which would only pause for 90 sec, but we'd have
to add a new test URL that's supported by the server.

Also, I think there's value in testing block=0 specifically, so that it isn't
accidentally changed without our knowledge; e.g. if someone accidentally changes
block=0 to mean 0 seconds or 1-5 hours or something, that would have significant
consequences to Data Saver.

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait until
30 seconds after the bypass should expire, and fetch a page. It
On 2015/01/15 00:06:09, bengr wrote:
> Is there no way to get around such delays?

See my comment in chrome_proxy_measurements.py.

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
File
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
(right):

https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
bypass_seconds_low: The minimum number of seconds that the bypass
On 2015/01/15 00:06:09, bengr wrote:
> Why aren't these bypass_seconds_min/max?

Sure, changed.

[bengr, 2015-01-15 00:50:03]

On 2015/01/15 00:44:35, sclittle wrote:
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py
(right):
> 
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310: for 1
to
> 5 minutes, then waits 5 minutes and verifies that the proxy is no
> On 2015/01/15 00:06:09, bengr wrote:
> > Yikes. I don't think it is a good idea to have a test pause for 5 minutes.
> 
> In order to test that the proxy is re-enabled after the bypass expires, we
need
> to wait for the bypass to expire, then see if the proxy gets used. Even using
> net-internals#proxy like we were before wouldn't catch bugs in the net stack
> proxy retry code.
> 
> I suppose we could do block=60 which would only pause for 90 sec, but we'd
have
> to add a new test URL that's supported by the server.
> 
> Also, I think there's value in testing block=0 specifically, so that it isn't
> accidentally changed without our knowledge; e.g. if someone accidentally
changes
> block=0 to mean 0 seconds or 1-5 hours or something, that would have
significant
> consequences to Data Saver.
> 
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):
> 
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait until
> 30 seconds after the bypass should expire, and fetch a page. It
> On 2015/01/15 00:06:09, bengr wrote:
> > Is there no way to get around such delays?
> 
> See my comment in chrome_proxy_measurements.py.
> 
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> File
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
> (right):
> 
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
> bypass_seconds_low: The minimum number of seconds that the bypass
> On 2015/01/15 00:06:09, bengr wrote:
> > Why aren't these bypass_seconds_min/max?
> 
> Sure, changed.

lgtm. We could add a flag that changes the default bypass period. I'd like that.

[sclittle, 2015-01-15 01:00:53]

On 2015/01/15 00:50:03, bengr wrote:
> On 2015/01/15 00:44:35, sclittle wrote:
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py
> (right):
> > 
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310: for 1
> to
> > 5 minutes, then waits 5 minutes and verifies that the proxy is no
> > On 2015/01/15 00:06:09, bengr wrote:
> > > Yikes. I don't think it is a good idea to have a test pause for 5 minutes.
> > 
> > In order to test that the proxy is re-enabled after the bypass expires, we
> need
> > to wait for the bypass to expire, then see if the proxy gets used. Even
using
> > net-internals#proxy like we were before wouldn't catch bugs in the net stack
> > proxy retry code.
> > 
> > I suppose we could do block=60 which would only pause for 90 sec, but we'd
> have
> > to add a new test URL that's supported by the server.
> > 
> > Also, I think there's value in testing block=0 specifically, so that it
isn't
> > accidentally changed without our knowledge; e.g. if someone accidentally
> changes
> > block=0 to mean 0 seconds or 1-5 hours or something, that would have
> significant
> > consequences to Data Saver.
> > 
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):
> > 
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait
until
> > 30 seconds after the bypass should expire, and fetch a page. It
> > On 2015/01/15 00:06:09, bengr wrote:
> > > Is there no way to get around such delays?
> > 
> > See my comment in chrome_proxy_measurements.py.
> > 
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > File
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
> > (right):
> > 
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
> > bypass_seconds_low: The minimum number of seconds that the bypass
> > On 2015/01/15 00:06:09, bengr wrote:
> > > Why aren't these bypass_seconds_min/max?
> > 
> > Sure, changed.
> 
> lgtm. We could add a flag that changes the default bypass period. I'd like
that.

You mean a flag that changes block=0 to mean whatever we want? I guess we could,
but I don't see the usefulness of it outside running that test a bit faster.
Besides, we'd probably have to wait ~1 minute regardless to account for
potential timing discrepancies, and it would be easier to just exclude running
that test if in a hurry.

[sclittle, 2015-01-15 01:01:15]

The CQ bit was checked by sclittle@chromium.org

[commit-bot: I haz the power, 2015-01-15 01:01:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/792303006/60001

[bengr, 2015-01-15 01:03:31]

On 2015/01/15 01:00:53, sclittle wrote:
> On 2015/01/15 00:50:03, bengr wrote:
> > On 2015/01/15 00:44:35, sclittle wrote:
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py
> > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310: for
1
> > to
> > > 5 minutes, then waits 5 minutes and verifies that the proxy is no
> > > On 2015/01/15 00:06:09, bengr wrote:
> > > > Yikes. I don't think it is a good idea to have a test pause for 5
minutes.
> > > 
> > > In order to test that the proxy is re-enabled after the bypass expires, we
> > need
> > > to wait for the bypass to expire, then see if the proxy gets used. Even
> using
> > > net-internals#proxy like we were before wouldn't catch bugs in the net
stack
> > > proxy retry code.
> > > 
> > > I suppose we could do block=60 which would only pause for 90 sec, but we'd
> > have
> > > to add a new test URL that's supported by the server.
> > > 
> > > Also, I think there's value in testing block=0 specifically, so that it
> isn't
> > > accidentally changed without our knowledge; e.g. if someone accidentally
> > changes
> > > block=0 to mean 0 seconds or 1-5 hours or something, that would have
> > significant
> > > consequences to Data Saver.
> > > 
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait
> until
> > > 30 seconds after the bypass should expire, and fetch a page. It
> > > On 2015/01/15 00:06:09, bengr wrote:
> > > > Is there no way to get around such delays?
> > > 
> > > See my comment in chrome_proxy_measurements.py.
> > > 
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > File
> > >
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > >
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
> > > bypass_seconds_low: The minimum number of seconds that the bypass
> > > On 2015/01/15 00:06:09, bengr wrote:
> > > > Why aren't these bypass_seconds_min/max?
> > > 
> > > Sure, changed.
> > 
> > lgtm. We could add a flag that changes the default bypass period. I'd like
> that.
> 
> You mean a flag that changes block=0 to mean whatever we want? I guess we
could,
> but I don't see the usefulness of it outside running that test a bit faster.
> Besides, we'd probably have to wait ~1 minute regardless to account for
> potential timing discrepancies, and it would be easier to just exclude running
> that test if in a hurry.

A test should not take >5 minutes to run. Why a full ~1 minute to account for
timing discrepancies?

[sclittle, 2015-01-15 01:33:56]

On 2015/01/15 01:03:31, bengr wrote:
> On 2015/01/15 01:00:53, sclittle wrote:
> > On 2015/01/15 00:50:03, bengr wrote:
> > > On 2015/01/15 00:44:35, sclittle wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > > File tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py
> > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > > tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:310:
for
> 1
> > > to
> > > > 5 minutes, then waits 5 minutes and verifies that the proxy is no
> > > > On 2015/01/15 00:06:09, bengr wrote:
> > > > > Yikes. I don't think it is a good idea to have a test pause for 5
> minutes.
> > > > 
> > > > In order to test that the proxy is re-enabled after the bypass expires,
we
> > > need
> > > > to wait for the bypass to expire, then see if the proxy gets used. Even
> > using
> > > > net-internals#proxy like we were before wouldn't catch bugs in the net
> stack
> > > > proxy retry code.
> > > > 
> > > > I suppose we could do block=60 which would only pause for 90 sec, but
we'd
> > > have
> > > > to add a new test URL that's supported by the server.
> > > > 
> > > > Also, I think there's value in testing block=0 specifically, so that it
> > isn't
> > > > accidentally changed without our knowledge; e.g. if someone accidentally
> > > changes
> > > > block=0 to mean 0 seconds or 1-5 hours or something, that would have
> > > significant
> > > > consequences to Data Saver.
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > > File tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py
(right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > > tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:397: # Wait
> > until
> > > > 30 seconds after the bypass should expire, and fetch a page. It
> > > > On 2015/01/15 00:06:09, bengr wrote:
> > > > > Is there no way to get around such delays?
> > > > 
> > > > See my comment in chrome_proxy_measurements.py.
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > > File
> > > >
> > >
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/792303006/diff/40001/tools/chrome_proxy/integ...
> > > >
> > >
> >
>
tools/chrome_proxy/integration_tests/chrome_proxy_pagesets/reenable_after_bypass.py:13:
> > > > bypass_seconds_low: The minimum number of seconds that the bypass
> > > > On 2015/01/15 00:06:09, bengr wrote:
> > > > > Why aren't these bypass_seconds_min/max?
> > > > 
> > > > Sure, changed.
> > > 
> > > lgtm. We could add a flag that changes the default bypass period. I'd like
> > that.
> > 
> > You mean a flag that changes block=0 to mean whatever we want? I guess we
> could,
> > but I don't see the usefulness of it outside running that test a bit faster.
> > Besides, we'd probably have to wait ~1 minute regardless to account for
> > potential timing discrepancies, and it would be easier to just exclude
running
> > that test if in a hurry.
> 
> A test should not take >5 minutes to run. Why a full ~1 minute to account for
> timing discrepancies?

It's an attempt to reduce flakiness due to slow networks and slow javascript and
slow adb, although the 30 sec on each end could definitely be tuned, especially
the 30 sec on the end.

There are ways to reduce this ~1 minute down by quite a bit, but I don't think
we should really worry about that at the moment since the test waits for 5
minutes anyways. We should definitely revisit this if/whenever we add any more
really slow tests.

[commit-bot: I haz the power, 2015-01-15 02:01:26]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-01-15 02:02:18]

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