From a39732f5cd23e1303337e7e9f8d0485801ee865d Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Tue, 18 Aug 2015 16:27:04 -0400 Subject: [PATCH] Bug 1163801 - Refactor marionette's options mixin system for argparse compatibility, r=AutomatedTester --- .../marionette/client/marionette/__init__.py | 7 +- .../client/marionette/runner/__init__.py | 39 +++++++---- .../client/marionette/runner/base.py | 70 +++++++++++-------- .../marionette/runner/mixins/__init__.py | 28 ++++---- .../marionette/runner/mixins/browsermob.py | 40 +++++------ .../marionette/runner/mixins/endurance.py | 56 +++++++-------- .../marionette/runner/mixins/reporting.py | 16 ++--- .../marionette/client/marionette/runtests.py | 33 ++++----- testing/marionette/mach_commands.py | 29 ++++---- 9 files changed, 169 insertions(+), 149 deletions(-) diff --git a/testing/marionette/client/marionette/__init__.py b/testing/marionette/client/marionette/__init__.py index 83b333ddcfa4..3495a60a00f3 100644 --- a/testing/marionette/client/marionette/__init__.py +++ b/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, diff --git a/testing/marionette/client/marionette/runner/__init__.py b/testing/marionette/client/marionette/runner/__init__.py index 0dadfae093c0..3835e2b82039 100644 --- a/testing/marionette/client/marionette/runner/__init__.py +++ b/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, +) diff --git a/testing/marionette/client/marionette/runner/base.py b/testing/marionette/client/marionette/runner/base.py index 022b90d18661..a0180f0617c7 100644 --- a/testing/marionette/client/marionette/runner/base.py +++ b/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 diff --git a/testing/marionette/client/marionette/runner/mixins/__init__.py b/testing/marionette/client/marionette/runner/mixins/__init__.py index 530ba465e368..8e686b6dd8b0 100644 --- a/testing/marionette/client/marionette/runner/mixins/__init__.py +++ b/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, - ) - +) diff --git a/testing/marionette/client/marionette/runner/mixins/browsermob.py b/testing/marionette/client/marionette/runner/mixins/browsermob.py index 0b05c474bd20..ea10ab4403a0 100644 --- a/testing/marionette/client/marionette/runner/mixins/browsermob.py +++ b/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): diff --git a/testing/marionette/client/marionette/runner/mixins/endurance.py b/testing/marionette/client/marionette/runner/mixins/endurance.py index 3d5ce1f9b090..f03e44e13567 100644 --- a/testing/marionette/client/marionette/runner/mixins/endurance.py +++ b/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): diff --git a/testing/marionette/client/marionette/runner/mixins/reporting.py b/testing/marionette/client/marionette/runner/mixins/reporting.py index 05143071569d..547e9093d82e 100644 --- a/testing/marionette/client/marionette/runner/mixins/reporting.py +++ b/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): diff --git a/testing/marionette/client/marionette/runtests.py b/testing/marionette/client/marionette/runtests.py index f719e6b244a2..0f0e067a434f 100644 --- a/testing/marionette/client/marionette/runtests.py +++ b/testing/marionette/client/marionette/runtests.py @@ -10,8 +10,8 @@ from marionette.marionette_test import MarionetteTestCase, MarionetteJSTestCase from marionette.runner import ( BaseMarionetteTestRunner, - BaseMarionetteOptions, - BrowserMobProxyOptionsMixin + BaseMarionetteArguments, + BrowserMobProxyArguments, ) import mozlog @@ -22,22 +22,23 @@ def __init__(self, **kwargs): self.test_handlers = [MarionetteTestCase, MarionetteJSTestCase] -class MarionetteOptions(BaseMarionetteOptions, - BrowserMobProxyOptionsMixin): +class MarionetteArguments(BaseMarionetteArguments): def __init__(self, **kwargs): - BaseMarionetteOptions.__init__(self, **kwargs) - BrowserMobProxyOptionsMixin.__init__(self, **kwargs) + BaseMarionetteArguments.__init__(self, **kwargs) + self.register_argument_container(BrowserMobProxyArguments()) -def startTestRunner(runner_class, options, tests): - if options.pydebugger: - MarionetteTestCase.pydebugger = __import__(options.pydebugger) +def startTestRunner(runner_class, args): + if args.pydebugger: + MarionetteTestCase.pydebugger = __import__(args.pydebugger) - runner = runner_class(**vars(options)) + args = vars(args) + tests = args.pop('tests') + runner = runner_class(**args) runner.run_tests(tests) return runner -def cli(runner_class=MarionetteTestRunner, parser_class=MarionetteOptions): +def cli(runner_class=MarionetteTestRunner, parser_class=MarionetteArguments): parser = parser_class( usage='%prog [options] test_file_or_dir ...', version="%prog {version} (using marionette-driver: {driver_version}" @@ -47,15 +48,15 @@ def cli(runner_class=MarionetteTestRunner, parser_class=MarionetteOptions): transport_version=transport_version) ) mozlog.commandline.add_logging_group(parser) - options, tests = parser.parse_args() - parser.verify_usage(options, tests) + args = parser.parse_args() + parser.verify_usage(args) logger = mozlog.commandline.setup_logging( - options.logger_name, options, {"tbpl": sys.stdout}) + args.logger_name, args, {"tbpl": sys.stdout}) - options.logger = logger + args.logger = logger - runner = startTestRunner(runner_class, options, tests) + runner = startTestRunner(runner_class, args) if runner.failed > 0: sys.exit(10) diff --git a/testing/marionette/mach_commands.py b/testing/marionette/mach_commands.py index 05c752df9f24..474cfe888272 100644 --- a/testing/marionette/mach_commands.py +++ b/testing/marionette/mach_commands.py @@ -39,36 +39,37 @@ def run_marionette(tests, b2g_path=None, emulator=None, testtype=None, from marionette.runtests import ( MarionetteTestRunner, - BaseMarionetteOptions, + BaseMarionetteArguments, startTestRunner ) - parser = BaseMarionetteOptions() + parser = BaseMarionetteArguments() commandline.add_logging_group(parser) - options, args = parser.parse_known_args() + args = parser.parse_args() if not tests: tests = [os.path.join(topsrcdir, - 'testing/marionette/client/marionette/tests/unit-tests.ini')] + 'testing/marionette/client/marionette/tests/unit-tests.ini')] + args.tests = tests if b2g_path: - options.homedir = b2g_path + args.homedir = b2g_path if emulator: - options.emulator = emulator + args.emulator = emulator else: - options.binary = binary - path, exe = os.path.split(options.binary) + args.binary = binary + path, exe = os.path.split(args.binary) for k, v in kwargs.iteritems(): - setattr(options, k, v) + setattr(args, k, v) - parser.verify_usage(options, tests) + parser.verify_usage(args) - options.logger = commandline.setup_logging("Marionette Unit Tests", - options, - {"mach": sys.stdout}) + args.logger = commandline.setup_logging("Marionette Unit Tests", + args, + {"mach": sys.stdout}) - runner = startTestRunner(MarionetteTestRunner, options, tests) + runner = startTestRunner(MarionetteTestRunner, args) if runner.failed > 0: return 1