Integration test for data reduction proxy version header

Integration test for data reduction proxy version header

This test verifies that the build and patch directives in the
Chrome-Proxy header are used as expected by the data reduction
proxy.

BUG=409936
Committed: https://crrev.com/57fa3c717e29259bb8137667b4daca55a92414c8
Cr-Commit-Position: refs/heads/master@{#295074}

[bengr, 2014-09-04 01:38:22]

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

[bengr, 2014-09-04 01:38:22]

bolian: all

[bolian, 2014-09-04 02:09:02]

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

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:74: class
ChromeProxyVersionTest(benchmark.Benchmark):
FYI: In a separate CL, if you want have this test run in trybot, you need to add
an entry in clank/build/chrome_proxy_tests.json.

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:74: class
ChromeProxyVersionTest(benchmark.Benchmark):
The name "Test" is implicit. Can you rename this and all other occurrences  
(including the tag below) to, for example, "ChromeProxyClientVersion"?

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

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:217: # Depends
on the fix of http://crbug.com/330342.
wrong comment?

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:221: In this
test, the configured proxy is the chromeproxy-test server. The test
rm the first sentence.

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:233:
options.AppendExtraBrowserArgs('--user-agent="No Version"')
This is not the final UA string, right?  You want to set an older version when
the server change rolled out?

[bengr, 2014-09-04 18:42:23]

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

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:74: class
ChromeProxyVersionTest(benchmark.Benchmark):
On 2014/09/04 02:09:02, bolian wrote:
> The name "Test" is implicit. Can you rename this and all other occurrences  
> (including the tag below) to, for example, "ChromeProxyClientVersion"?

Done.

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_benchmark.py:74: class
ChromeProxyVersionTest(benchmark.Benchmark):
On 2014/09/04 02:09:02, bolian wrote:
> FYI: In a separate CL, if you want have this test run in trybot, you need to
add
> an entry in clank/build/chrome_proxy_tests.json.

Acknowledged.

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

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:217: # Depends
on the fix of http://crbug.com/330342.
On 2014/09/04 02:09:02, bolian wrote:
> wrong comment?

Done.

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:221: In this
test, the configured proxy is the chromeproxy-test server. The test
On 2014/09/04 02:09:02, bolian wrote:
> rm the first sentence.

Done.

https://codereview.chromium.org/544463002/diff/1/tools/chrome_proxy/integrati...
tools/chrome_proxy/integration_tests/chrome_proxy_measurements.py:233:
options.AppendExtraBrowserArgs('--user-agent="No Version"')
On 2014/09/04 02:09:02, bolian wrote:
> This is not the final UA string, right?  You want to set an older version when
> the server change rolled out?

Done.

[bolian, 2014-09-04 20:49:48]

lgtm

[megjablon, 2014-09-10 19:02:30]

megjablon@chromium.org changed reviewers:
+ megjablon@chromium.org

[megjablon, 2014-09-10 19:02:30]

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

https://codereview.chromium.org/544463002/diff/20001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:163: raise
ChromeProxyMetricException, ('%s: Response missing via header')
This doesn't print anything to %s
('%s: Response missing via header' % (r.url))

[bengr, 2014-09-12 20:36:41]

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

https://codereview.chromium.org/544463002/diff/20001/tools/chrome_proxy/integ...
tools/chrome_proxy/integration_tests/chrome_proxy_metrics.py:163: raise
ChromeProxyMetricException, ('%s: Response missing via header')
On 2014/09/10 19:02:30, megjablon wrote:
> This doesn't print anything to %s
> ('%s: Response missing via header' % (r.url))

Done.

[bengr, 2014-09-12 20:36:49]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-09-12 20:38:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/544463002/40001

[commit-bot: I haz the power, 2014-09-12 20:38:57]

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

[commit-bot: I haz the power, 2014-09-12 20:38:57]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[bengr, 2014-09-12 21:10:43]

bengr@chromium.org changed reviewers:
+ marq@google.com

[bengr, 2014-09-12 21:11:06]

marq: PTAL. megjablon and bolian are not committers.

[marq (ping after 24h), 2014-09-16 07:41:48]

marq@chromium.org changed reviewers:
+ marq@chromium.org

[marq (ping after 24h), 2014-09-16 07:41:49]

lgtm, largely a rubber-stamp as this is literally the first time I've looked at
the integration test scripts.

[bengr, 2014-09-16 14:43:22]

The CQ bit was checked by bengr@chromium.org

[commit-bot: I haz the power, 2014-09-16 14:44:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/544463002/40001

[commit-bot: I haz the power, 2014-09-16 15:41:00]

Committed patchset #3 (id:40001) as 39f2b05abf9817fbe1c5503bff7e0654353aee02

[commit-bot: I haz the power, 2014-09-16 15:42:16]

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

[bengr, 2014-09-17 00:50:17]

Patchset #4 (id:60001) has been deleted