Add usage message with available page_sets and benchmarks.

Since we updated record_wpr to take actual page_set names instead of filenames, things got a little confusing. This patch clears some of that up:

* Added usage message to record_wpr
* Added additional default command ('run') and additional commands:
  - run_benchmark: only run benchmarks
  - run_pageset: only run pagesets
  - list_pagesets: list all available pagesets
  - list_benchmarks: list all available benchmarks
* Moved some common functionality out of test_runner.py into discover.py
* Call page sets the same as benchmarks. That is use "top_25" rather than "top25_page_set". This will hopefully clear up a lot of the confusion (since it is MUCH more intuitive to what people were doing).

BUG=406429

[chrishenry, 2014-07-31 05:29:36]

https://codereview.chromium.org/432543003/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/record_wpr.py (right):

https://codereview.chromium.org/432543003/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/record_wpr.py:117: for cls in [page_set.PageSet,
benchmark.Benchmark]:
Can we share logic between page_runner.py and this?

https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[ariblue, 2014-08-26 22:42:21]

Description has also been updated, since this patch does a little more, now that
I've gotten back to fixing it.

https://codereview.chromium.org/432543003/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/record_wpr.py (right):

https://codereview.chromium.org/432543003/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/record_wpr.py:117: for cls in [page_set.PageSet,
benchmark.Benchmark]:
On 2014/07/31 05:29:36, chrishenry wrote:
> Can we share logic between page_runner.py and this?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

Done. Logic shared in find_classes.py

[ariblue, 2014-08-26 22:42:33]

ariblue@google.com changed reviewers:
+ nednguyen@google.com

[tonyg, 2014-08-26 22:54:13]

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

[nednguyen, 2014-08-29 15:13:17]

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/record_wpr.py (right):

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:108: return matching_classes[0]()
if len(matching_classes) == 1 else None
Hmhh, would this break if page_set __init__() has parameter? I think record_wpr
not able to handle this maybe fine, but it should output something to say why
this fails.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:126: sys.exit(1)
I think it would be nice if we print s.t like: 

"Recording page set XYZ" or "Recording benchmark XYZ". So people know whether
which is recorded in case of default command.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:175: def _GetBenchmark(self,
base_dir, target):
No need for this extra function, I think the callsite can handle case benchmark
is None.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:228: '  run <PageSet|Benchmark>    
  Run a Page Set or Benchmark (default)\n'
Add "Page Set takes precedence over Benchmark in case of name conflict."

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/util/find_classes.py (right):

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:4: 
I think this can be merged into discover.py

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:55: def
PrintAvailableClasses(base_classes, class_list=None):
This should take an output_stream so we can test it.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:63: # Align the test names to the
longest one.
test names?

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:76:
sys.stderr.write('\n'.join(output))
Why is this sys.stderr?

[ariblue, 2014-08-29 23:07:52]

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/record_wpr.py (right):

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:108: return matching_classes[0]()
if len(matching_classes) == 1 else None
On 2014/08/29 15:13:17, nednguyen wrote:
> Hmhh, would this break if page_set __init__() has parameter? I think
record_wpr
> not able to handle this maybe fine, but it should output something to say why
> this fails.

I've added a logging.warn() with some additional details and then I re-raise the
exception

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:126: sys.exit(1)
On 2014/08/29 15:13:17, nednguyen wrote:
> I think it would be nice if we print s.t like: 
> 
> "Recording page set XYZ" or "Recording benchmark XYZ". So people know whether
> which is recorded in case of default command.

Done.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:175: def _GetBenchmark(self,
base_dir, target):
On 2014/08/29 15:13:17, nednguyen wrote:
> No need for this extra function, I think the callsite can handle case
benchmark
> is None.

Done.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/record_wpr.py:228: '  run <PageSet|Benchmark>    
  Run a Page Set or Benchmark (default)\n'
On 2014/08/29 15:13:17, nednguyen wrote:
> Add "Page Set takes precedence over Benchmark in case of name conflict."

Done.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/util/find_classes.py (right):

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:4: 
On 2014/08/29 15:13:17, nednguyen wrote:
> I think this can be merged into discover.py

Done.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:55: def
PrintAvailableClasses(base_classes, class_list=None):
On 2014/08/29 15:13:17, nednguyen wrote:
> This should take an output_stream so we can test it.

Done.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:63: # Align the test names to the
longest one.
On 2014/08/29 15:13:17, nednguyen wrote:
> test names?

Oh, uh... I shamelessly stole this code from test_runner.py. This probably
refers to benchmarks from back when they were telemetry.test.Test (instead of
telemetry.benchmark.Benchmark). Fixed.

https://codereview.chromium.org/432543003/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/util/find_classes.py:76:
sys.stderr.write('\n'.join(output))
On 2014/08/29 15:13:17, nednguyen wrote:
> Why is this sys.stderr?

Also shamelessly copied over from test_runner.py, which used stderr for pretty
much everything. Since it doesn't really make sense in some locations (like when
you explicitly call 'list'), but does in others. This now uses the output_steam
parameter and defaults to stdout.

[nednguyen, 2014-09-04 03:31:37]

Can you also add unittest for new functions in discover.py?

https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/discover.py (right):

https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/discover.py:18: config =
environment.Environment([util.GetBaseDir()])
Why do we need a global?

https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/discover.py:144: def
GetValidSubclassesOfClasses(base_classes, base_dir=None):
Please add docstring for this function and the ones below.

https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/discover.py:183: output_stream=sys.stdout):
Can we split this function to two functions: one that prints class in
classes_list in sorted order, and one that finds subclasses of base_classes?

And do we really have the use cases on the call sites that need the 2nd
function?

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

https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
tools/telemetry/telemetry/test_runner.py:27: test_class_types =
[benchmark.Benchmark, page_test.PageTest]
May better be explicit about this: _BENCHMARK_AND_PAGE_TESTS_CLASSES

[nednguyen, 2015-04-27 18:27:56]

On 2014/09/04 03:31:37, nednguyen wrote:
> Can you also add unittest for new functions in discover.py?
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/core/discover.py (right):
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/core/discover.py:18: config =
> environment.Environment([util.GetBaseDir()])
> Why do we need a global?
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/core/discover.py:144: def
> GetValidSubclassesOfClasses(base_classes, base_dir=None):
> Please add docstring for this function and the ones below.
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/core/discover.py:183: output_stream=sys.stdout):
> Can we split this function to two functions: one that prints class in
> classes_list in sorted order, and one that finds subclasses of base_classes?
> 
> And do we really have the use cases on the call sites that need the 2nd
> function?
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/test_runner.py (right):
> 
>
https://codereview.chromium.org/432543003/diff/40001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/test_runner.py:27: test_class_types =
> [benchmark.Benchmark, page_test.PageTest]
> May better be explicit about this: _BENCHMARK_AND_PAGE_TESTS_CLASSES

Dave, maybe this patch still has a lot of relevant idea for fixing wpr.

[nednguyen, 2015-04-27 18:28:09]

nednguyen@google.com changed reviewers:
+ aiolos@chromium.org