Telemetry: Allow running only a subset of pages.

Telemetry: Allow running only a subset of pages.

This is needed for the upcoming "the Web Page replay data for a page set can be
distributed across multiple .wpr files" feature.

R=nduca
NOTRY=true

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

[marja, 2013-01-09 14:11:55]

Hi again, nduca@,

I guess this --page-filter feature is needed and useful anyway, independent of
how the metadata layer is done. So here's a small CL for adding just the
--page-filter. Ptal.

[nduca, 2013-01-09 21:47:19]

lgtm with nits

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/bro...
File tools/telemetry/telemetry/browser_options.py (right):

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/bro...
tools/telemetry/telemetry/browser_options.py:107: help='Run only pages whose
URLs match the given filter regexp.')
should we validate that the regex compiles down in the parse_args function lower
down?

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/pag...
File tools/telemetry/telemetry/page_runner.py (right):

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/pag...
tools/telemetry/telemetry/page_runner.py:46: if options.page_filter:
Should we use globbing instead of regex? Lets use whatever behavior
--gtest_filter has for c++ tests...

[marja, 2013-01-10 13:39:59]

Thanks for comments, I didn't dare to commit this despite of the lgtm, because I
didn't do the "use the same wildcard scheme as --gtest_filter"... I can do it if
you really really want, but I wouldn't like to.

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/bro...
File tools/telemetry/telemetry/browser_options.py (right):

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/bro...
tools/telemetry/telemetry/browser_options.py:107: help='Run only pages whose
URLs match the given filter regexp.')
On 2013/01/09 21:47:19, nduca wrote:
> should we validate that the regex compiles down in the parse_args function
lower
> down?

I added checks to _ShuffleAndFilterPageSet, since it already checks for the
--test-shuffle params, and raises if there is a problem. So it seemed to fit to
add this check there too.

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/pag...
File tools/telemetry/telemetry/page_runner.py (right):

https://codereview.chromium.org/11829024/diff/1/tools/telemetry/telemetry/pag...
tools/telemetry/telemetry/page_runner.py:46: if options.page_filter:
On 2013/01/09 21:47:19, nduca wrote:
> Should we use globbing instead of regex? Lets use whatever behavior
> --gtest_filter has for c++ tests...

I think the syntax which --gtest_filter uses is obscure, and more people will
find this regex syntax more intuitive.

E.g., to run for the pages gmail and facebook, I'd be happy to write:
--page-filter="(mail|facebook)"
but who would guess that in the --gtest_filter syntax it is:
--page-filter=*mail*:*facebook*

I checked the gtest source code, and while this matching is not difficult, it..
uh, it doesn't even provide any way to escape the special characters (*, ?, :,
-). I wouldn't copy that format. It's not any more intuitive / wider known than
regexps. Was there a specific feature of that matching scheme which you like and
regexps can't easily provide?

(I got to say that "negative patterns" are semi-appealing, but.. still..)

[nduca, 2013-01-10 20:02:04]

Thats fine! I'm okay with whatever you feel is correct.  Please make sure the
options doc is current and then update
http://dev.chromium.org/developers/telemetry maybe?

[nduca, 2013-01-10 20:02:39]

That is, still LGTM.

[commit-bot: I haz the power, 2013-01-10 20:05:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11829024/5001

[commit-bot: I haz the power, 2013-01-10 20:35:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/11829024/5001

[commit-bot: I haz the power, 2013-01-10 20:37:01]

Change committed as 176146