Have repeats understand "time"

tools/telemetry/telemetry/core/browser_options.py

Index: tools/telemetry/telemetry/core/browser_options.py
diff --git a/tools/telemetry/telemetry/core/browser_options.py b/tools/telemetry/telemetry/core/browser_options.py
index f27f93499ca313a7c5da46c687db82af71ce5d06..93b456099b38a1a72780a874eaf1bef174cce0c7 100644
--- a/tools/telemetry/telemetry/core/browser_options.py
+++ b/tools/telemetry/telemetry/core/browser_options.py
@@ -49,6 +49,11 @@ class BrowserOptions(optparse.Values):
 
     self.no_proxy_server = False
 
+    self.page_repeat = 1
+    self.pageset_repeat = 1
+    self.page_repeat_time = 0.0
+    self.pageset_repeat_time = 0.0

[dennis_jeffrey, 2013/07/10 20:11:20]

maybe initialize the above 2 variables to "None" instead of 0.0, since they're
checked in a condition below at line 234:

self.page_repeat_time or self.pageset_repeat_time

(I'm a little more comfortable using either "None" or "0" when checking in a
condition, rather than a float).

+
   def Copy(self):
     return copy.deepcopy(self)
 
@@ -114,11 +119,13 @@ class BrowserOptions(optparse.Values):
     # Page set options
     group = optparse.OptionGroup(parser, 'Page set options')
     group.add_option('--page-repeat', dest='page_repeat', default=1,
-        help='Number of times to repeat each individual ' +
-        'page in the pageset before proceeding.')
+        help='Number of iterations or length of time to repeat each ' +
+             'individual page in the pageset before proceeding.  Append ' +
+             'an \'s\' to specify length of time in seconds')

[dennis_jeffrey, 2013/07/10 20:11:20]

Maybe also give an example in the help string, something like this:

e.g., '10' to repeat for 10 iterations, or '30s' to repeat for 30 seconds.

[edmundyan, 2013/07/11 01:18:55]

Done.

     group.add_option('--pageset-repeat', dest='pageset_repeat', default=1,
-        help='Number of times to repeat the entire pageset ' +
-        'before finishing.')
+        help='Number of iterations or length of time to repeat the entire ' +
+        'pageset before finishing.  Append an \'s\' to specify length of ' +

[dtu, 2013/07/10 23:15:41]

Don't need the + signs here.

[edmundyan, 2013/07/11 01:18:55]

Done.

+        'time in seconds')
     group.add_option('--pageset-shuffle', action='store_true',
         dest='pageset_shuffle',
         help='Shuffle the order of pages within a pageset.')
@@ -215,6 +222,19 @@ class BrowserOptions(optparse.Values):
         delattr(self, 'extra_wpr_args_as_string')
       if self.profile_type == 'default':
         self.dont_override_profile = True
+      if self.page_repeat != 1: # pylint: disable=E1101

[dennis_jeffrey, 2013/07/10 20:11:20]

is this check necessary?

The condition at line 226 below being true would already imply that
self.page_repeat != 1.

[edmundyan, 2013/07/10 21:13:09]

The default value is int(1), and would cause an error on the next line when we
try to access it like a string.  I could combine the nested ifs into one or use
isinstance() to ensure it's a string?

Also wondering if I need to add more checks here. "10secs" would pass this
function without errors.

[dtu, 2013/07/10 23:15:41]

I think you should keep 'page_repeat' as a string with a default value of '1';
and as you parse it, set 'page_repeat_seconds' or 'page_repeat_iterations' based
on which one it is, and do the type conversion too, which will cause it to fail
fast if it's the wrong type. Similar to how the 'extra_wpr_args_as_string'
conversion to 'extra_wpr_args' is done, above.

[edmundyan, 2013/07/11 01:18:55]

Done.  Also using regex matching instead of character matching to add robustness
(as per 1:1 w/ Dennis)

+        if self.page_repeat[-1] == 's':

[dennis_jeffrey, 2013/07/10 20:11:20]

maybe accept both upper and lowercase 's'?

if self.page_repeat[-1] in ['s', 'S']:

[edmundyan, 2013/07/11 01:18:55]

Done.

+          self.page_repeat_time = self.page_repeat[:-1]
+          self.page_repeat = 1
+      if self.pageset_repeat != 1: # pylint: disable=E1101

[dennis_jeffrey, 2013/07/10 20:11:20]

similar comment as line 225 above

+        if self.pageset_repeat[-1] == 's':
+          self.pageset_repeat_time = self.pageset_repeat[:-1]
+          self.pageset_repeat = 1
+      if ((self.page_repeat != 1 or self.pageset_repeat != 1) and
+          (self.page_repeat_time or self.pageset_repeat_time)):
+        sys.stderr.write('Cannot interchange repeat options.  Use only ' +
+                         'iterations or time.\n')
+        sys.exit(1)
       self.profile_dir = profile_types.GetProfileDir(self.profile_type)
       return ret
     parser.parse_args = ParseArgs