From 4751b33d0c7eca58b8d801f634ba6d0dd6974664 Mon Sep 17 00:00:00 2001 From: bendikro Date: Sun, 22 May 2016 20:02:02 +0200 Subject: [PATCH] [Console] Fix to console argument parsing When starting console with './deluge-console', providing loggin level '-L info' would fail to parse as it identified 'info' as a subcommand. --- deluge/tests/test_ui_entry.py | 206 +++++++++++++++++++++++++++------- deluge/ui/baseargparser.py | 50 +++++++-- deluge/ui/console/console.py | 8 +- deluge/ui/console/main.py | 23 +++- deluge/ui/ui_entry.py | 10 +- 5 files changed, 235 insertions(+), 62 deletions(-) diff --git a/deluge/tests/test_ui_entry.py b/deluge/tests/test_ui_entry.py index a85f56ac2..30db2043b 100644 --- a/deluge/tests/test_ui_entry.py +++ b/deluge/tests/test_ui_entry.py @@ -1,4 +1,12 @@ # -*- coding: utf-8 -*- +# +# Copyright (C) 2016 bendikro +# +# This file is part of Deluge and is licensed under GNU General Public License 3.0, or later, with +# the additional special exception to link portions of this program with the OpenSSL library. +# See LICENSE for more details. +# + from __future__ import print_function import argparse @@ -20,6 +28,7 @@ from . import common from .basetest import BaseTestCase sys_stdout = sys.stdout +DEBUG = False class TestStdout(object): @@ -31,12 +40,30 @@ class TestStdout(object): setattr(self, a, getattr(sys_stdout, a)) def write(self, *data, **kwargs): - print(data, file=self.out) + print(*data, file=self.out, end="") def flush(self): self.out.flush() +class UIBaseTestCase(object): + + def __init__(self): + self.var = dict() + + def set_up(self): + common.set_tmp_config_dir() + return component.start() + + def tear_down(self): + return component.shutdown() + + def exec_command(self): + if DEBUG: + print("Executing: '%s'\n" % sys.argv, file=sys_stdout) + self.var["start_cmd"]() + + class DelugeEntryTestCase(BaseTestCase): def set_up(self): @@ -91,61 +118,142 @@ class DelugeEntryTestCase(BaseTestCase): self.assertEqual(_level[0], "info") -@pytest.mark.gtkui -class GtkUIEntryTestCase(BaseTestCase): - - def set_up(self): - common.set_tmp_config_dir() - return component.start() - - def tear_down(self): - return component.shutdown() +class GtkUIBaseTestCase(UIBaseTestCase): + """Implement all GtkUI tests here""" def test_start_gtkui(self): - self.patch(sys, "argv", ["./deluge", "gtk"]) + self.patch(sys, "argv", self.var["sys_arg_cmd"]) from deluge.ui.gtkui import gtkui with mock.patch.object(gtkui.GtkUI, "start", autospec=True): - ui_entry.start_ui() + self.exec_command() -class WebUIEntryTestCase(BaseTestCase): +@pytest.mark.gtkui +class GtkUIDelugeScriptEntryTestCase(BaseTestCase, GtkUIBaseTestCase): + + def __init__(self, testname): + BaseTestCase.__init__(self, testname) + GtkUIBaseTestCase.__init__(self) + self.var["cmd_name"] = "deluge gtk" + self.var["start_cmd"] = ui_entry.start_ui + self.var["sys_arg_cmd"] = ["./deluge", "gtk"] def set_up(self): - common.set_tmp_config_dir() - return component.start() + GtkUIBaseTestCase.set_up(self) def tear_down(self): - return component.shutdown() + GtkUIBaseTestCase.tear_down(self) + + +@pytest.mark.gtkui +class GtkUIScriptEntryTestCase(BaseTestCase, GtkUIBaseTestCase): + + def __init__(self, testname): + BaseTestCase.__init__(self, testname) + GtkUIBaseTestCase.__init__(self) + from deluge.ui import gtkui + self.var["cmd_name"] = "deluge-gtk" + self.var["start_cmd"] = gtkui.start + self.var["sys_arg_cmd"] = ["./deluge-gtk"] + + def set_up(self): + GtkUIBaseTestCase.set_up(self) + + def tear_down(self): + GtkUIBaseTestCase.tear_down(self) + + +class DelugeWebMock(DelugeWeb): + def __init__(self, *args, **kwargs): + kwargs["daemon"] = False + DelugeWeb.__init__(self, *args, **kwargs) + + +class WebUIBaseTestCase(UIBaseTestCase): + """Implement all WebUI tests here""" def test_start_webserver(self): - self.patch(sys, "argv", ["./deluge", "web", "--do-not-daemonize"]) + self.patch(sys, "argv", self.var["sys_arg_cmd"]) + self.patch(deluge.ui.web.server, "DelugeWeb", DelugeWebMock) + self.exec_command() - class DelugeWebMock(DelugeWeb): - def __init__(self, *args, **kwargs): - kwargs["daemon"] = False - DelugeWeb.__init__(self, *args, **kwargs) + def test_start_web_with_log_level(self): + _level = [] + + def setup_logger(level="error", filename=None, filemode="w", logrotate=None): + _level.append(level) + + self.patch(deluge.log, "setup_logger", setup_logger) + self.patch(sys, "argv", self.var["sys_arg_cmd"] + ["-L", "info"]) + + config = deluge.configmanager.ConfigManager("ui.conf", ui_entry.DEFAULT_PREFS) + config.config["default_ui"] = "web" + config.save() self.patch(deluge.ui.web.server, "DelugeWeb", DelugeWebMock) - ui_entry.start_ui() + self.exec_command() + self.assertEqual(_level[0], "info") -class ConsoleUIBaseTestCase(object): +class WebUIScriptEntryTestCase(BaseTestCase, WebUIBaseTestCase): - def __init__(self): - self.var = dict() + def __init__(self, testname): + BaseTestCase.__init__(self, testname) + WebUIBaseTestCase.__init__(self) + self.var["cmd_name"] = "deluge-web" + self.var["start_cmd"] = deluge.ui.web.start + self.var["sys_arg_cmd"] = ["./deluge-web", "--do-not-daemonize"] def set_up(self): - common.set_tmp_config_dir() - return component.start() + WebUIBaseTestCase.set_up(self) def tear_down(self): - return component.shutdown() + WebUIBaseTestCase.tear_down(self) + + +class WebUIDelugeScriptEntryTestCase(BaseTestCase, WebUIBaseTestCase): + + def __init__(self, testname): + BaseTestCase.__init__(self, testname) + WebUIBaseTestCase.__init__(self) + self.var["cmd_name"] = "deluge web" + self.var["start_cmd"] = ui_entry.start_ui + self.var["sys_arg_cmd"] = ["./deluge", "web", "--do-not-daemonize"] + + def set_up(self): + WebUIBaseTestCase.set_up(self) + + def tear_down(self): + WebUIBaseTestCase.tear_down(self) + + +class ConsoleUIBaseTestCase(UIBaseTestCase): + """Implement all Console tests here""" def test_start_console(self): self.patch(sys, "argv", self.var["sys_arg_cmd"]) with mock.patch("deluge.ui.console.main.ConsoleUI"): - self.var["start_cmd"]() + self.exec_command() + + def test_start_console_with_log_level(self): + _level = [] + + def setup_logger(level="error", filename=None, filemode="w", logrotate=None): + _level.append(level) + + self.patch(deluge.log, "setup_logger", setup_logger) + self.patch(sys, "argv", self.var["sys_arg_cmd"] + ["-L", "info"]) + + config = deluge.configmanager.ConfigManager("ui.conf", ui_entry.DEFAULT_PREFS) + config.config["default_ui"] = "console" + config.save() + + with mock.patch("deluge.ui.console.main.ConsoleUI"): + # Just test that no exception is raised + self.exec_command() + + self.assertEqual(_level[0], "info") def test_console_help(self): self.patch(sys, "argv", self.var["sys_arg_cmd"] + ["-h"]) @@ -153,12 +261,12 @@ class ConsoleUIBaseTestCase(object): self.patch(argparse._sys, "stdout", fd) with mock.patch("deluge.ui.console.main.ConsoleUI"): - self.assertRaises(exceptions.SystemExit, self.var["start_cmd"]) + self.assertRaises(exceptions.SystemExit, self.exec_command) std_output = fd.out.getvalue() self.assertTrue(("usage: %s" % self.var["cmd_name"]) in std_output) # Check command name self.assertTrue("Common Options:" in std_output) self.assertTrue("Console Options:" in std_output) - self.assertTrue(r"Console commands:\n The following console commands are available:" in std_output) + self.assertTrue("Console commands:\n The following console commands are available:" in std_output) self.assertTrue("The following console commands are available:" in std_output) def test_console_command_info(self): @@ -167,7 +275,7 @@ class ConsoleUIBaseTestCase(object): self.patch(argparse._sys, "stdout", fd) with mock.patch("deluge.ui.console.main.ConsoleUI"): - self.var["start_cmd"]() + self.exec_command() def test_console_command_info_help(self): self.patch(sys, "argv", self.var["sys_arg_cmd"] + ["info", "-h"]) @@ -175,7 +283,7 @@ class ConsoleUIBaseTestCase(object): self.patch(argparse._sys, "stdout", fd) with mock.patch("deluge.ui.console.main.ConsoleUI"): - self.assertRaises(exceptions.SystemExit, self.var["start_cmd"]) + self.assertRaises(exceptions.SystemExit, self.exec_command) std_output = fd.out.getvalue() self.assertTrue("usage: info" in std_output) self.assertTrue("Show information about the torrents" in std_output) @@ -185,11 +293,27 @@ class ConsoleUIBaseTestCase(object): fd = TestStdout(sys.stdout) self.patch(argparse._sys, "stderr", fd) with mock.patch("deluge.ui.console.main.ConsoleUI"): - self.assertRaises(exceptions.SystemExit, self.var["start_cmd"]) + self.assertRaises(exceptions.SystemExit, self.exec_command) self.assertTrue("unrecognized arguments: --ui" in fd.out.getvalue()) -class ConsoleUIEntryTestCase(BaseTestCase, ConsoleUIBaseTestCase): +class ConsoleScriptEntryTestCase(BaseTestCase, ConsoleUIBaseTestCase): + + def __init__(self, testname): + BaseTestCase.__init__(self, testname) + ConsoleUIBaseTestCase.__init__(self) + self.var["cmd_name"] = "deluge-console" + self.var["start_cmd"] = deluge.ui.console.start + self.var["sys_arg_cmd"] = ["./deluge-console"] + + def set_up(self): + ConsoleUIBaseTestCase.set_up(self) + + def tear_down(self): + ConsoleUIBaseTestCase.tear_down(self) + + +class ConsoleDelugeScriptEntryTestCase(BaseTestCase, ConsoleUIBaseTestCase): def __init__(self, testname): BaseTestCase.__init__(self, testname) @@ -198,12 +322,8 @@ class ConsoleUIEntryTestCase(BaseTestCase, ConsoleUIBaseTestCase): self.var["start_cmd"] = ui_entry.start_ui self.var["sys_arg_cmd"] = ["./deluge", "console"] + def set_up(self): + ConsoleUIBaseTestCase.set_up(self) -class ConsoleUIScriptTestCase(BaseTestCase, ConsoleUIBaseTestCase): - - def __init__(self, testname): - BaseTestCase.__init__(self, testname) - ConsoleUIBaseTestCase.__init__(self) - self.var["cmd_name"] = "deluge-console" - self.var["start_cmd"] = deluge.ui.console.start - self.var["sys_arg_cmd"] = ["./deluge-console"] + def tear_down(self): + ConsoleUIBaseTestCase.tear_down(self) diff --git a/deluge/ui/baseargparser.py b/deluge/ui/baseargparser.py index e25a841f5..aad4cfa33 100644 --- a/deluge/ui/baseargparser.py +++ b/deluge/ui/baseargparser.py @@ -19,25 +19,27 @@ from deluge import common from deluge.configmanager import get_config_dir, set_config_dir -def find_subcommand(self, args=None): +def find_subcommand(self, args=None, sys_argv=True): """Find if a subcommand has been supplied. Args: - args (list, optional): The argument list handed to parse_args(). + args (list, optional): The argument list to search through. + sys_argv (bool): Use sys.argv[1:] if args is None. Returns: int: Index of the subcommand or '-1' if none found. """ subcommand_found = -1 - test_args = args if args is not None else sys.argv[1:] + if args is None: + args = sys.argv[1:] if sys_argv is None else [] for x in self._subparsers._actions: if not isinstance(x, argparse._SubParsersAction): continue for sp_name in x._name_parser_map.keys(): - if sp_name in test_args: - subcommand_found = test_args.index(sp_name) + 1 + if sp_name in args: + subcommand_found = args.index(sp_name) return subcommand_found @@ -184,20 +186,47 @@ class BaseArgParser(argparse.ArgumentParser): def parse_args(self, args=None): """Parse UI arguments and handle common and process group options. - Unknown arguments return an error and resulting usage text. + Notes: + Unknown arguments results in usage text printed and system exit. + + Args: + args (list, optional): The arguments to parse. + + Returns: + argparse.Namespace: The parsed arguments. + """ options = super(BaseArgParser, self).parse_args(args=args) - return self._parse_args(options) + return self._handle_ui_options(options) - def parse_known_ui_args(self, args=None): + def parse_known_ui_args(self, args, withhold=None): """Parse UI arguments and handle common and process group options without error. + + Args: + args (list): The arguments to parse. + withhold (list): Values to ignore in the args list. + + Returns: + argparse.Namespace: The parsed arguments. + """ + if withhold: + args = [a for a in args if a not in withhold] options, remaining = super(BaseArgParser, self).parse_known_args(args=args) options.remaining = remaining - return self._parse_args(options) + # Hanlde common and process group options + return self._handle_ui_options(options) - def _parse_args(self, options): + def _handle_ui_options(self, options): + """Handle UI common and process group options. + Args: + options (argparse.Namespace): The parsed options. + + Returns: + argparse.Namespace: The parsed options. + + """ if not self.common_setup: self.common_setup = True # Setup the logger @@ -226,6 +255,7 @@ class BaseArgParser(argparse.ArgumentParser): os.makedirs(common.get_default_config_dir()) if self.process_arg_group: + self.process_arg_group = False # If donotdaemonize is set, skip process forking. if not (common.windows_check() or options.donotdaemonize): if os.fork(): diff --git a/deluge/ui/console/console.py b/deluge/ui/console/console.py index f2a09b904..77170ee97 100644 --- a/deluge/ui/console/console.py +++ b/deluge/ui/console/console.py @@ -7,12 +7,13 @@ # the additional special exception to link portions of this program with the OpenSSL library. # See LICENSE for more details. # +from __future__ import print_function import logging import os import deluge.common -from deluge.ui.baseargparser import DelugeTextHelpFormatter +from deluge.ui.baseargparser import BaseArgParser, DelugeTextHelpFormatter from deluge.ui.console import UI_PATH from deluge.ui.ui import UI @@ -79,6 +80,11 @@ class Console(UI): self.console_cmds[c].add_subparser(subparsers) def start(self): + if self.ui_args is None: + # Started directly by deluge-console script so must find the UI args manually + options, remaining = BaseArgParser(common_help=False).parse_known_args() + self.ui_args = remaining + i = self.console_parser.find_subcommand(args=self.ui_args) self.console_parser.subcommand = False self.parser.subcommand = False if i == -1 else True diff --git a/deluge/ui/console/main.py b/deluge/ui/console/main.py index b10dca818..732372e7e 100644 --- a/deluge/ui/console/main.py +++ b/deluge/ui/console/main.py @@ -33,7 +33,7 @@ from deluge.ui.sessionproxy import SessionProxy log = logging.getLogger(__name__) -class ConsoleCommandParser(argparse.ArgumentParser): +class ConsoleBaseParser(argparse.ArgumentParser): def format_help(self): """ @@ -44,12 +44,15 @@ class ConsoleCommandParser(argparse.ArgumentParser): # Handle epilog manually to keep the text formatting epilog = self.epilog self.epilog = "" - help_str = super(ConsoleCommandParser, self).format_help() + help_str = super(ConsoleBaseParser, self).format_help() if epilog is not None: help_str += epilog self.epilog = epilog return help_str + +class ConsoleCommandParser(ConsoleBaseParser): + def _split_args(self, args): command_options = [] for a in args: @@ -71,6 +74,20 @@ class ConsoleCommandParser(argparse.ArgumentParser): return command_options def parse_args(self, args=None): + """Parse known UI args and handle common and process group options. + + Notes: + If started by deluge entry script this has already been done. + + Args: + args (list, optional): The arguments to parse. + + Returns: + argparse.Namespace: The parsed arguments. + """ + from deluge.ui.ui_entry import AMBIGUOUS_CMD_ARGS + self.base_parser.parse_known_ui_args(args, withhold=AMBIGUOUS_CMD_ARGS) + multi_command = self._split_args(args) # If multiple commands were passed to console if multi_command: @@ -103,7 +120,7 @@ class ConsoleCommandParser(argparse.ArgumentParser): return options -class OptionParser(ConsoleCommandParser): +class OptionParser(ConsoleBaseParser): def __init__(self, **kwargs): super(OptionParser, self).__init__(**kwargs) diff --git a/deluge/ui/ui_entry.py b/deluge/ui/ui_entry.py index b156516bd..f99989662 100644 --- a/deluge/ui/ui_entry.py +++ b/deluge/ui/ui_entry.py @@ -29,6 +29,8 @@ DEFAULT_PREFS = { "default_ui": "gtk" } +AMBIGUOUS_CMD_ARGS = ("-h", "--help", "-v", "-V", "--version") + def start_ui(): """Entry point for ui script""" @@ -49,10 +51,8 @@ def start_ui(): # Setup parser with Common Options and add UI Options group. parser = add_ui_options_group(BaseArgParser()) - ambiguous_args = ["-h", "--help", "-v", "-V", "--version"] - # Parse arguments without help/version as this is handled later. - args = [a for a in sys.argv if a not in ambiguous_args] - options = parser.parse_known_ui_args(args) + # Parse and handle common/process group options + options = parser.parse_known_ui_args(sys.argv, withhold=AMBIGUOUS_CMD_ARGS) config = deluge.configmanager.ConfigManager("ui.conf", DEFAULT_PREFS) log = logging.getLogger(__name__) @@ -86,7 +86,7 @@ def start_ui(): subactions[-1].dest = "%s %s" % (prefix, ui) # Insert a default UI subcommand unless one of the ambiguous_args are specified - parser.set_default_subparser(default_ui, abort_opts=ambiguous_args) + parser.set_default_subparser(default_ui, abort_opts=AMBIGUOUS_CMD_ARGS) # Only parse known arguments to leave the UIs to show a help message if parsing fails. options, remaining = parser.parse_known_args()