[telemetry] Pass test_runner environment in local args instead of a global variable.

[telemetry] Pass test_runner environment in local args instead of a global variable.

Also adds more fields to environment to narrow the scope of test and page set discovery. This should avoid problems with adding Python files to unrelated directories, and hides the PageTests from run_gpu_tests.py.

BUG=None
TEST=tools/perf/run_benchmark; tools/perf/run_measurement; content/test/gpu/run_gpu_tests.py  # All return a full and correct test list.

[dtu, 2014-05-06 23:45:18]

Followup, as requested. (Actually the other patch [1] got mired in unit test
failures and hasn't landed yet, so it's an advance refactor.)

[1] https://chromiumcodereview.appspot.com/228073002/

[dtu, 2014-05-06 23:53:18]

https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
File tools/telemetry/telemetry/test_runner.py (right):

https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
tools/telemetry/telemetry/test_runner.py:26: class Environment(object):
Moved this into here from its own file, because run_tests and record_wpr have
different fields from run_benchmark, so they'll have different classes.

[Satyanarayana N H Manyam, 2014-05-07 02:38:04]

https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
File tools/telemetry/telemetry/test_runner.py (right):

https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
tools/telemetry/telemetry/test_runner.py:215: tests +=
discover.DiscoverClasses(search_dir, environment.top_level_dir,
we should take top_level_dir for each search_dir, else relpath computation will
fail right?

[dtu, 2014-05-07 18:00:09]

On 2014/05/07 02:38:04, Satyanarayana N H Manyam wrote:
>
https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/test_runner.py (right):
> 
>
https://codereview.chromium.org/267253002/diff/10006/tools/telemetry/telemetr...
> tools/telemetry/telemetry/test_runner.py:215: tests +=
> discover.DiscoverClasses(search_dir, environment.top_level_dir,
> we should take top_level_dir for each search_dir, else relpath computation
will
> fail right?

We wanted to restrict it to one top_level_dir, but I'm not as familiar with the
organization structure going forward. nduca, thoughts?

[nduca, 2014-05-08 02:20:20]

i think we need one top level dir, which is ~= cwd, which is where telemety ran
from, which resolves any ambiguities with relative imports inside python
modules.

We shouldn't have any relpath computation anywhere in telemetry because its
inherently ambiguous.

[nduca, 2014-05-08 02:20:29]

er, which is to say, lgtm from my pov

[Satyanarayana N H Manyam, 2014-05-08 03:24:05]

On 2014/05/08 02:20:20, nduca wrote:
> i think we need one top level dir, which is ~= cwd, which is where telemety
ran
> from, which resolves any ambiguities with relative imports inside python
> modules.
> 
> We shouldn't have any relpath computation anywhere in telemetry because its
> inherently ambiguous.

discover.DiscoverClasses(search_dir, environment.top_level_dir,..) has relpath
computation for adding the modules paths

One of the use case of this is to use pagesets/measurements from 2 src trees, in
which case it might not work in this new flow right?

[dtu, 2014-05-08 18:36:33]

On 2014/05/08 03:24:05, Satyanarayana N H Manyam wrote:
> On 2014/05/08 02:20:20, nduca wrote:
> > i think we need one top level dir, which is ~= cwd, which is where telemety
> ran
> > from, which resolves any ambiguities with relative imports inside python
> > modules.
> > 
> > We shouldn't have any relpath computation anywhere in telemetry because its
> > inherently ambiguous.
> 
> discover.DiscoverClasses(search_dir, environment.top_level_dir,..) has relpath
> computation for adding the modules paths
> 
> One of the use case of this is to use pagesets/measurements from 2 src trees,
in
> which case it might not work in this new flow right?

Yes, you should have a separate run_benchmark script for each source tree.

[dtu, 2014-05-08 18:42:20]

On 2014/05/08 18:36:33, dtu wrote:
> On 2014/05/08 03:24:05, Satyanarayana N H Manyam wrote:
> > On 2014/05/08 02:20:20, nduca wrote:
> > > i think we need one top level dir, which is ~= cwd, which is where
telemety
> > ran
> > > from, which resolves any ambiguities with relative imports inside python
> > > modules.
> > > 
> > > We shouldn't have any relpath computation anywhere in telemetry because
its
> > > inherently ambiguous.
> > 
> > discover.DiscoverClasses(search_dir, environment.top_level_dir,..) has
relpath
> > computation for adding the modules paths
> > 
> > One of the use case of this is to use pagesets/measurements from 2 src
trees,
> in
> > which case it might not work in this new flow right?
> 
> Yes, you should have a separate run_benchmark script for each source tree.

Similar to how we have run_gpu test.py in content/test/gpu/ and run_benchmark in
tools/perf/

[dtu, 2014-05-08 18:43:42]

On 2014/05/08 18:42:20, dtu wrote:
> On 2014/05/08 18:36:33, dtu wrote:
> > On 2014/05/08 03:24:05, Satyanarayana N H Manyam wrote:
> > > On 2014/05/08 02:20:20, nduca wrote:
> > > > i think we need one top level dir, which is ~= cwd, which is where
> telemety
> > > ran
> > > > from, which resolves any ambiguities with relative imports inside python
> > > > modules.
> > > > 
> > > > We shouldn't have any relpath computation anywhere in telemetry because
> its
> > > > inherently ambiguous.
> > > 
> > > discover.DiscoverClasses(search_dir, environment.top_level_dir,..) has
> relpath
> > > computation for adding the modules paths
> > > 
> > > One of the use case of this is to use pagesets/measurements from 2 src
> trees,
> > in
> > > which case it might not work in this new flow right?
> > 
> > Yes, you should have a separate run_benchmark script for each source tree.
> 
> Similar to how we have run_gpu test.py in content/test/gpu/ and run_benchmark
in
> tools/perf/

Speaking of which, +kbr for content/test/gpu/ OWNERS

[Ken Russell (switch to Gerrit), 2014-05-08 21:25:21]

LGTM once this passes the GPU bots. (mac_gpu_retina_triggered_tests has known
failures; see Issue 371194.)

https://chromiumcodereview.appspot.com/267253002/diff/10006/tools/telemetry/t...
File tools/telemetry/telemetry/test_runner.py (right):

https://chromiumcodereview.appspot.com/267253002/diff/10006/tools/telemetry/t...
tools/telemetry/telemetry/test_runner.py:82: def AddCommandLineArgs(cls, parser,
_):  # pylint: disable=W0221
Since you're ignoring the unused argument anyway why not name it 'environment'?

[nednguyen, 2014-08-05 14:53:27]

On 2014/05/08 21:25:21, Ken Russell wrote:
> LGTM once this passes the GPU bots. (mac_gpu_retina_triggered_tests has known
> failures; see Issue 371194.)
> 
>
https://chromiumcodereview.appspot.com/267253002/diff/10006/tools/telemetry/t...
> File tools/telemetry/telemetry/test_runner.py (right):
> 
>
https://chromiumcodereview.appspot.com/267253002/diff/10006/tools/telemetry/t...
> tools/telemetry/telemetry/test_runner.py:82: def AddCommandLineArgs(cls,
parser,
> _):  # pylint: disable=W0221
> Since you're ignoring the unused argument anyway why not name it
'environment'?

Hi Dave, is this patch ready? Can you land it since I have a patch that depends
on this one.

[tonyg, 2015-01-14 03:13:44]

tonyg@chromium.org changed reviewers:
- tonyg@chromium.org

[eakuefner, 2015-02-11 19:25:41]

eakuefner@chromium.org changed reviewers:
+ nduca@chromium.org

[eakuefner, 2015-02-11 19:25:41]

CC me, probably going to pick this up and land it.