Move chrome_proxy tests from under tools/perf to tools/chrome_proxy/integration_tests.

Move chrome_proxy tests from under tools/perf to tools/chrome_proxy/integration_tests.

BUG=393360
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284310

[bolian, 2014-07-15 00:55:57]

Hello Ben and Tony,

This is moving the chrome_proxy tests to tools/chrome_proxy/integration_tests.

Thanks,
Bolian

[bengr, 2014-07-15 16:48:55]

lgtm with nits and __init__.py looks empty.

https://codereview.chromium.org/397483002/diff/40001/tools/chrome_proxy/OWNERS
File tools/chrome_proxy/OWNERS (right):

https://codereview.chromium.org/397483002/diff/40001/tools/chrome_proxy/OWNER...
tools/chrome_proxy/OWNERS:3: marq@chromium.org
Please add megjablon and kundaji

[bolian, 2014-07-15 17:00:28]

__init__.py is intended to be empty.

https://codereview.chromium.org/397483002/diff/40001/tools/chrome_proxy/OWNERS
File tools/chrome_proxy/OWNERS (right):

https://codereview.chromium.org/397483002/diff/40001/tools/chrome_proxy/OWNER...
tools/chrome_proxy/OWNERS:3: marq@chromium.org
On 2014/07/15 16:48:55, bengr1 wrote:
> Please add megjablon and kundaji

Done.

[bolian, 2014-07-16 07:27:21]

Tony,

Can you take a look?

Thanks,
Bolian

[bolian, 2014-07-16 18:55:45]

Added Dave to the reviewers. Dave, do you have time to take a look?

[tonyg, 2014-07-17 17:20:18]

lgtm, thanks!

Only question is whether you want a run_tests in there too for running any
unittests associated with the benchmarks.

[tonyg, 2014-07-17 17:22:00]

On 2014/07/17 17:20:18, tonyg wrote:
> lgtm, thanks!
> 
> Only question is whether you want a run_tests in there too for running any
> unittests associated with the benchmarks.

Or perhaps it would be fine for tools/perf/run_tests to discover tests in the
new directory as well so that you don't have to mess with adding a new target to
the buildbots.

Unittests will be key to making sure that people making telemetry changes don't
forget to update chrome_proxy.

[bolian, 2014-07-17 18:24:07]

On 2014/07/17 17:22:00, tonyg wrote:
> On 2014/07/17 17:20:18, tonyg wrote:
> > lgtm, thanks!
> > 
> > Only question is whether you want a run_tests in there too for running any
> > unittests associated with the benchmarks.
> 
> Or perhaps it would be fine for tools/perf/run_tests to discover tests in the
> new directory as well so that you don't have to mess with adding a new target
to
> the buildbots.
> 
> Unittests will be key to making sure that people making telemetry changes
don't
> forget to update chrome_proxy.

Thanks!
I updated tools/perf/run_tests to run chrome_proxy tests. There seems to be the
assumption that all tests should be under the same top_level_dir and modules are
under top_level_dir. So, I have to run chrome_proxy tests after the perf tests
are done.

Please take a look whether it is appropriate.

[tonyg, 2014-07-17 18:27:53]

https://codereview.chromium.org/397483002/diff/100001/tools/perf/run_tests
File tools/perf/run_tests (right):

https://codereview.chromium.org/397483002/diff/100001/tools/perf/run_tests#ne...
tools/perf/run_tests:32: sys.exit(test_result)
If the benchmark is happy, then I'm happy with this.

But I know Dave has a lot of plans here, so Dave would you mind double checking
this?

[dtu, 2014-07-17 22:17:39]

https://codereview.chromium.org/397483002/diff/100001/tools/perf/run_tests
File tools/perf/run_tests (right):

https://codereview.chromium.org/397483002/diff/100001/tools/perf/run_tests#ne...
tools/perf/run_tests:32: sys.exit(test_result)
On 2014/07/17 18:27:52, tonyg wrote:
> If the benchmark is happy, then I'm happy with this.
> 
> But I know Dave has a lot of plans here, so Dave would you mind double
checking
> this?

I think I'd prefer another run_tests or run_unittests in tools/chrome_proxy/.
There're a few points of concern here.
- Not sure if the buildbot log parser will work with multiple separate gtest
outputs in the same stream.
- Telemetry zip archive doesn't contain chrome_proxy/, run_tests will fail when
running from the archive.
- By design we disallow multiple top_level_dirs. nduca will know more. Also we
disallow this cross-directory dependency.

[bolian, 2014-07-17 22:42:33]

> I think I'd prefer another run_tests or run_unittests in tools/chrome_proxy/.
> There're a few points of concern here.
> - Not sure if the buildbot log parser will work with multiple separate gtest
> outputs in the same stream.
> - Telemetry zip archive doesn't contain chrome_proxy/, run_tests will fail
when
> running from the archive.
> - By design we disallow multiple top_level_dirs. nduca will know more. Also we
> disallow this cross-directory dependency.

Ok. I added a separate chrome_proxy/run_tests. Then, can someone give me a
pointer to setup buildbot to run this new test target? I will probably do it in
a separate CL. Anything else related to adding a new test target?

[dtu, 2014-07-18 21:56:51]

lgtm % nits

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
File tools/chrome_proxy/run_benchmark (right):

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
tools/chrome_proxy/run_benchmark:2: # Copyright (c) 2014 The Chromium Authors.
All rights reserved.
nit: no (c)

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
File tools/chrome_proxy/run_measurement (right):

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
tools/chrome_proxy/run_measurement:1: #!/usr/bin/env python
nit: Don't need this file.

[bolian, 2014-07-18 23:07:24]

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
File tools/chrome_proxy/run_benchmark (right):

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
tools/chrome_proxy/run_benchmark:2: # Copyright (c) 2014 The Chromium Authors.
All rights reserved.
nit: no (c)

Done.

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
File tools/chrome_proxy/run_measurement (right):

https://chromiumcodereview.appspot.com/397483002/diff/120001/tools/chrome_pro...
tools/chrome_proxy/run_measurement:1: #!/usr/bin/env python
nit: Don't need this file.

Done.

[bolian, 2014-07-18 23:12:53]

The CQ bit was checked by bolian@chromium.org

[commit-bot: I haz the power, 2014-07-18 23:13:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bolian@chromium.org/397483002/190001

[commit-bot: I haz the power, 2014-07-19 04:50:50]

Change committed as 284310