Bug 1163801 - Refactor marionette's options mixin system for argparse…

… compatibility, r=AutomatedTester

testing/marionette/client/marionette/__init__.py

@@ -5,17 +5,16 @@
 
 __version__ = '0.18'
 
-from argparse import ArgumentParser
 from .marionette_test import MarionetteTestCase, MarionetteJSTestCase, CommonTestCase, expectedFailure, skip, SkipTest
 from .runner import (
         B2GTestCaseMixin,
         B2GTestResultMixin,
-        BaseMarionetteOptions,
+        BaseMarionetteArguments,
         BaseMarionetteTestRunner,
         BrowserMobProxyTestCaseMixin,
-        EnduranceOptionsMixin,
+        EnduranceArguments,
         EnduranceTestCaseMixin,
-        HTMLReportingOptionsMixin,
+        HTMLReportingArguments,
         HTMLReportingTestResultMixin,
         HTMLReportingTestRunnerMixin,
         Marionette,

testing/marionette/client/marionette/runner/__init__.py

@@ -2,16 +2,29 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from argparse import ArgumentParser
-from base import (
-        B2GTestResultMixin, BaseMarionetteOptions, BaseMarionetteTestRunner,
-        Marionette, MarionetteTest, MarionetteTestResult, MarionetteTextTestRunner,
-         TestManifest, TestResult, TestResultCollection
-        )
-from mixins import (
-        B2GTestCaseMixin, B2GTestResultMixin, EnduranceOptionsMixin,
-        EnduranceTestCaseMixin, HTMLReportingOptionsMixin, HTMLReportingTestResultMixin,
-        HTMLReportingTestRunnerMixin, MemoryEnduranceTestCaseMixin,
-        BrowserMobProxyTestCaseMixin, BrowserMobProxyOptionsMixin,
-        BrowserMobTestCase,
-        )
+from .base import (
+    B2GTestResultMixin,
+    BaseMarionetteArguments,
+    BaseMarionetteTestRunner,
+    Marionette,
+    MarionetteTest,
+    MarionetteTestResult,
+    MarionetteTextTestRunner,
+    TestManifest,
+    TestResult,
+    TestResultCollection,
+)
+
+from .mixins import (
+    B2GTestCaseMixin,
+    B2GTestResultMixin,
+    EnduranceArguments,
+    EnduranceTestCaseMixin,
+    HTMLReportingArguments,
+    HTMLReportingTestResultMixin,
+    HTMLReportingTestRunnerMixin,
+    MemoryEnduranceTestCaseMixin,
+    BrowserMobProxyTestCaseMixin,
+    BrowserMobProxyArguments,
+    BrowserMobTestCase,
+)

testing/marionette/client/marionette/runner/base.py

@@ -244,13 +244,12 @@ def run(self, test):
         return result
 
 
-class BaseMarionetteOptions(ArgumentParser):
+class BaseMarionetteArguments(ArgumentParser):
     socket_timeout_default = 360.0
 
     def __init__(self, **kwargs):
         ArgumentParser.__init__(self, **kwargs)
-        self.parse_args_handlers = [] # Used by mixins
-        self.verify_usage_handlers = [] # Used by mixins
+        self.argument_containers = []
         self.add_argument('tests',
                           nargs='*',
                           default=[],
@@ -384,67 +383,76 @@ def __init__(self, **kwargs):
                              "used multiple times in which case the test must contain "
                              "at least one of the given tags.")
 
+    def register_argument_container(self, container):
+        group = self.add_argument_group(container.name)
+
+        for cli, kwargs in container.args:
+            group.add_argument(*cli, **kwargs)
+
+        self.argument_containers.append(container)
+
     def parse_args(self, args=None, values=None):
         args = ArgumentParser.parse_args(self, args, values)
-        for handler in self.parse_args_handlers:
-            handler(options, tests, args, values)
-
-        return (args, args.tests)
+        for container in self.argument_containers:
+            if hasattr(container, 'parse_args_handler'):
+                container.parse_args_handler(args)
+        return args
 
-    def verify_usage(self, options, tests):
-        if not tests:
+    def verify_usage(self, args):
+        if not args.tests:
             print 'must specify one or more test files, manifests, or directories'
             sys.exit(1)
 
-        if not options.emulator and not options.address and not options.binary:
+        if not args.emulator and not args.address and not args.binary:
             print 'must specify --binary, --emulator or --address'
             sys.exit(1)
 
-        if options.emulator and options.binary:
+        if args.emulator and args.binary:
             print 'can\'t specify both --emulator and --binary'
             sys.exit(1)
 
         # default to storing logcat output for emulator runs
-        if options.emulator and not options.logdir:
-            options.logdir = 'logcat'
+        if args.emulator and not args.logdir:
+            args.logdir = 'logcat'
 
         # check for valid resolution string, strip whitespaces
         try:
-            if options.emulator_res:
-                dims = options.emulator_res.split('x')
+            if args.emulator_res:
+                dims = args.emulator_res.split('x')
                 assert len(dims) == 2
                 width = str(int(dims[0]))
                 height = str(int(dims[1]))
-                options.emulator_res = 'x'.join([width, height])
+                args.emulator_res = 'x'.join([width, height])
         except:
             raise ValueError('Invalid emulator resolution format. '
                              'Should be like "480x800".')
 
-        if options.total_chunks is not None and options.this_chunk is None:
+        if args.total_chunks is not None and args.this_chunk is None:
             self.error('You must specify which chunk to run.')
 
-        if options.this_chunk is not None and options.total_chunks is None:
+        if args.this_chunk is not None and args.total_chunks is None:
             self.error('You must specify how many chunks to split the tests into.')
 
-        if options.total_chunks is not None:
-            if not 1 <= options.total_chunks:
+        if args.total_chunks is not None:
+            if not 1 <= args.total_chunks:
                 self.error('Total chunks must be greater than 1.')
-            if not 1 <= options.this_chunk <= options.total_chunks:
-                self.error('Chunk to run must be between 1 and %s.' % options.total_chunks)
+            if not 1 <= args.this_chunk <= args.total_chunks:
+                self.error('Chunk to run must be between 1 and %s.' % args.total_chunks)
 
-        if options.jsdebugger:
-            options.app_args.append('-jsdebugger')
-            options.socket_timeout = None
+        if args.jsdebugger:
+            args.app_args.append('-jsdebugger')
+            args.socket_timeout = None
 
-        if options.e10s:
-            options.prefs = {
+        if args.e10s:
+            args.prefs = {
                 'browser.tabs.remote.autostart': True
             }
 
-        for handler in self.verify_usage_handlers:
-            handler(options, tests)
+        for container in self.argument_containers:
+            if hasattr(container, 'verify_usage_handler'):
+                container.verify_usage_handler(args)
 
-        return (options, tests)
+        return args
 
 
 class BaseMarionetteTestRunner(object):
@@ -462,7 +470,7 @@ def __init__(self, address=None, emulator=None, emulator_binary=None,
                  sdcard=None, this_chunk=1, total_chunks=1, sources=None,
                  server_root=None, gecko_log=None, result_callbacks=None,
                  adb_host=None, adb_port=None, prefs=None, test_tags=None,
-                 socket_timeout=BaseMarionetteOptions.socket_timeout_default,
+                 socket_timeout=BaseMarionetteArguments.socket_timeout_default,
                  startup_timeout=None, addons=None, **kwargs):
         self.address = address
         self.emulator = emulator

testing/marionette/client/marionette/runner/mixins/__init__.py

@@ -2,17 +2,21 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-from endurance import (
-        EnduranceOptionsMixin, EnduranceTestCaseMixin, MemoryEnduranceTestCaseMixin
-        )
-from reporting import (
-        HTMLReportingOptionsMixin, HTMLReportingTestResultMixin,
-        HTMLReportingTestRunnerMixin
-        )
-from b2g import B2GTestCaseMixin, B2GTestResultMixin
-from browsermob import (
+from .endurance import (
+    EnduranceArguments,
+    EnduranceTestCaseMixin,
+    MemoryEnduranceTestCaseMixin,
+)
+
+from .reporting import (
+    HTMLReportingArguments,
+    HTMLReportingTestResultMixin,
+    HTMLReportingTestRunnerMixin,
+)
+
+from .b2g import B2GTestCaseMixin, B2GTestResultMixin
+from .browsermob import (
     BrowserMobProxyTestCaseMixin,
-    BrowserMobProxyOptionsMixin,
+    BrowserMobProxyArguments,
     BrowserMobTestCase,
-    )
-
+)

testing/marionette/client/marionette/runner/mixins/browsermob.py

@@ -1,29 +1,29 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
 import os
 
 from browsermobproxy import Server
 from marionette import MarionetteTestCase
 
 
-class BrowserMobProxyOptionsMixin(object):
-
-    # verify_usage
-    def browsermob_verify_usage(self, options, tests):
-        if options.browsermob_script is not None:
-            if not os.path.exists(options.browsermob_script):
-                raise ValueError('%s not found' % options.browsermob_script)
-
-    def __init__(self, **kwargs):
-        # Inheriting object must call this __init__ to set up option handling
-        group = self.add_argument_group('Browsermob Proxy')
-        group.add_argument('--browsermob-script',
-                         dest='browsermob_script',
-                         type='string',
-                         help='path to the browsermob-proxy shell script or batch file')
-        group.add_argument('--browsermob-port',
-                         dest='browsermob_port',
-                         type='int',
-                         help='port to run the browsermob proxy on')
-        self.verify_usage_handlers.append(self.browsermob_verify_usage)
+class BrowserMobProxyArguments(object):
+    name = 'Browsermob Proxy'
+    args = [
+        [['--browsermob-script'],
+         {'help': 'path to the browsermob-proxy shell script or batch file',
+          }],
+        [['--browsermob-port'],
+         {'type': int,
+          'help': 'port to run the browsermob proxy on',
+          }],
+    ]
+
+    def verify_usage_handler(self, args):
+        if args.browsermob_script is not None:
+            if not os.path.exists(args.browsermob_script):
+                raise ValueError('%s not found' % args.browsermob_script)
 
 
 class BrowserMobProxyTestCaseMixin(object):

testing/marionette/client/marionette/runner/mixins/endurance.py

@@ -7,41 +7,35 @@
 import time
 
 
-class EnduranceOptionsMixin(object):
-
-    # parse_args
-    def endurance_parse_args(self, options, tests, args=None, values=None):
-        if options.iterations is not None:
-            if options.checkpoint_interval is None or options.checkpoint_interval > options.iterations:
-                options.checkpoint_interval = options.iterations
-
-    # verify_usage
-    def endurance_verify_usage(self, options, tests):
-        if options.iterations is not None and options.iterations < 1:
+class EnduranceArguments(object):
+    name = 'endurance'
+    args = [
+        [['--iterations'],
+         {'type': int,
+          'metavar': 'int',
+          'help': 'iterations for endurance tests',
+          }],
+        [['--checkpoint'],
+         {'dest': 'checkpoint_interval',
+          'type': int,
+          'metavar': 'int',
+          'help': 'checkpoint interval for endurance tests',
+          }],
+    ]
+
+    def parse_args_handler(self, args):
+        if args.iterations is not None:
+            if args.checkpoint_interval is None or args.checkpoint_interval > args.iterations:
+                args.checkpoint_interval = args.iterations
+
+    def verify_usage_handler(self, args):
+        if args.iterations is not None and args.iterations < 1:
             raise ValueError('iterations must be a positive integer')
-        if options.checkpoint_interval is not None and options.checkpoint_interval < 1:
+        if args.checkpoint_interval is not None and args.checkpoint_interval < 1:
             raise ValueError('checkpoint interval must be a positive integer')
-        if options.checkpoint_interval and not options.iterations:
+        if args.checkpoint_interval and not args.iterations:
             raise ValueError('you must specify iterations when using checkpoint intervals')
 
-    def __init__(self, **kwargs):
-        # Inheriting object must call this __init__ to set up option handling
-        group = self.add_option_group('endurance')
-        group.add_option('--iterations',
-                         action='store',
-                         dest='iterations',
-                         type='int',
-                         metavar='int',
-                         help='iterations for endurance tests')
-        group.add_option('--checkpoint',
-                         action='store',
-                         dest='checkpoint_interval',
-                         type='int',
-                         metavar='int',
-                         help='checkpoint interval for endurance tests')
-        self.parse_args_handlers.append(self.endurance_parse_args)
-        self.verify_usage_handlers.append(self.endurance_verify_usage)
-
 
 class EnduranceTestCaseMixin(object):
     def __init__(self, *args, **kwargs):

testing/marionette/client/marionette/runner/mixins/reporting.py

@@ -223,14 +223,14 @@ def _extract_html(result, test_name, test_class='', duration=0,
         return doc.unicode(indent=2)
 
 
-class HTMLReportingOptionsMixin(object):
-
-    def __init__(self, **kwargs):
-        group = self.add_argument_group('htmlreporting')
-        group.add_argument('--html-output',
-                         dest='html_output',
-                         help='html output',
-                         metavar='path')
+class HTMLReportingArguments(object):
+    name = 'htmlreporting'
+    args = [
+        [['--html-output'],
+         {'help': 'html output',
+          'metavar': 'path',
+          }],
+    ]
 
 
 class HTMLReportingTestResultMixin(object):