From 87d0e938ad8912cd1b17057a3009f2044e46f5c7 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Thu, 5 May 2016 09:09:41 +0200 Subject: [PATCH 1/7] Adding --dialog argument --- certbot/cli.py | 3 +++ certbot/main.py | 3 ++- certbot/tests/cli_test.py | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index e2c57595b..f96bf2656 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -569,6 +569,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): help="Run without ever asking for user input. This may require " "additional command line flags; the client will try to explain " "which ones are required if it finds one missing") + helpful.add( + None, "--dialog", dest="dialog_mode", action="store_true", + help="Run using dialog") helpful.add( None, "--dry-run", action="store_true", dest="dry_run", help="Perform a test run of the client, obtaining test (invalid) certs" diff --git a/certbot/main.py b/certbot/main.py index 309889e8e..161e7d7cd 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -677,8 +677,9 @@ def main(cli_args=sys.argv[1:]): displayer = display_util.NoninteractiveDisplay(sys.stdout) elif config.text_mode: displayer = display_util.FileDisplay(sys.stdout) + elif config.dialog_mode: + displayer = display_util.NcursesDisplay() elif config.verb == "renew": - config.noninteractive_mode = True displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 31056cafe..ca33f4524 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -149,6 +149,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args.extend(['--email', 'io@io.is']) self._cli_missing_flag(args, "--agree-tos") + @mock.patch('certbot.main.renew') + def test_gui(self, renew): + args = ['renew', '--dialog'] + self._call(args) + self.assertFalse(renew.call_args[0][0].noninteractive_mode) + @mock.patch('certbot.main.client.acme_client.Client') @mock.patch('certbot.main._determine_account') @mock.patch('certbot.main.client.Client.obtain_and_enroll_certificate') From 8772a9846f5c7e834dc23863e4905f5676159fc0 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 02:52:47 +0200 Subject: [PATCH 2/7] Raising error on conflicting args --- certbot/main.py | 10 ++++++++++ certbot/tests/cli_test.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/certbot/main.py b/certbot/main.py index 161e7d7cd..d900c65fa 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -669,6 +669,16 @@ def main(cli_args=sys.argv[1:]): sys.excepthook = functools.partial(_handle_exception, config=config) + # Avoid conflicting args + conficting_args = ["quiet", "noninteractive_mode", "text_mode"] + if config.dialog_mode: + for arg in conficting_args: + if getattr(config, arg): + raise errors.Error( + ("Conflicting values for displayer." + " {0} conflicts with dialog_mode").format(arg) + ) + # Displayer if config.quiet: config.noninteractive_mode = True diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index ca33f4524..64cf98c2c 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -49,7 +49,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.logs_dir = os.path.join(self.tmp_dir, 'logs') self.standard_args = ['--config-dir', self.config_dir, '--work-dir', self.work_dir, - '--logs-dir', self.logs_dir, '--text'] + '--logs-dir', self.logs_dir] def tearDown(self): shutil.rmtree(self.tmp_dir) From 1fd44d9302b5fa374dfc0c87416eb6006bccc8d2 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 03:55:52 +0200 Subject: [PATCH 3/7] Fixing tests --- certbot/cli.py | 3 +++ certbot/tests/cli_test.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index f96bf2656..e2878c1e9 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -552,6 +552,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): :rtype: argparse.Namespace """ + + # pylint: disable=too-many-statements + helpful = HelpfulArgumentParser(args, plugins, detect_defaults) # --help is automatically provided by argparse diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 64cf98c2c..7e352febb 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -49,7 +49,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.logs_dir = os.path.join(self.tmp_dir, 'logs') self.standard_args = ['--config-dir', self.config_dir, '--work-dir', self.work_dir, - '--logs-dir', self.logs_dir] + '--logs-dir', self.logs_dir, '--text'] def tearDown(self): shutil.rmtree(self.tmp_dir) @@ -152,6 +152,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods @mock.patch('certbot.main.renew') def test_gui(self, renew): args = ['renew', '--dialog'] + # --text conflicts with --dialog + self.standard_args.remove('--text') self._call(args) self.assertFalse(renew.call_args[0][0].noninteractive_mode) From 353abaa608c74f2a331ee920515aa713d6dd1827 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 04:59:09 +0200 Subject: [PATCH 4/7] Adding args conflict test to main_test.py --- certbot/tests/main_test.py | 39 +++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 66cba64a3..e3f8b860d 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1,15 +1,49 @@ """Tests for certbot.main.""" +import os +import shutil +import tempfile import unittest - import mock - +import six from certbot import cli from certbot import configuration +from certbot import errors +from certbot import main from certbot.plugins import disco as plugins_disco +class MainTest(unittest.TestCase): + """Tests for certbot.main""" + + def setUp(self): + self.tmp_dir = tempfile.mkdtemp() + self.config_dir = os.path.join(self.tmp_dir, 'config') + self.work_dir = os.path.join(self.tmp_dir, 'work') + self.logs_dir = os.path.join(self.tmp_dir, 'logs') + self.standard_args = ['--config-dir', self.config_dir, + '--work-dir', self.work_dir, + '--logs-dir', self.logs_dir, '--text'] + + def tearDown(self): + shutil.rmtree(self.tmp_dir) + + def _call(self, args, stdout=None): + "Run the client with output streams mocked out" + args = self.standard_args + args + + toy_stdout = stdout if stdout else six.StringIO() + with mock.patch('certbot.main.sys.stdout', new=toy_stdout): + with mock.patch('certbot.main.sys.stderr') as stderr: + ret = main.main(args[:]) # NOTE: parser can alter its args! + return ret, toy_stdout, stderr + + def test_args_conflict(self): + args = ['renew', '--dialog', '--text'] + self.assertRaises(errors.Error, self._call, args) + + class ObtainCertTest(unittest.TestCase): """Tests for certbot.main.obtain_cert.""" @@ -26,7 +60,6 @@ class ObtainCertTest(unittest.TestCase): config = configuration.NamespaceConfig( cli.prepare_and_parse_args(plugins, args)) - from certbot import main with mock.patch('certbot.main._init_le_client') as mock_init: main.obtain_cert(config, plugins) From b158d03e9754e8674903fa7bc3ea00642e5c361c Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 16:30:29 +0200 Subject: [PATCH 5/7] Revert removal of a needed line --- certbot/main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/main.py b/certbot/main.py index d900c65fa..0f2aece1e 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -690,6 +690,7 @@ def main(cli_args=sys.argv[1:]): elif config.dialog_mode: displayer = display_util.NcursesDisplay() elif config.verb == "renew": + config.noninteractive_mode = True displayer = display_util.NoninteractiveDisplay(sys.stdout) else: displayer = display_util.NcursesDisplay() From 911b4565bea1cb4dbcfc49a4c4ce4337c377e207 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 23:09:17 +0200 Subject: [PATCH 6/7] Moving check for conflicting args to cli.py --- certbot/cli.py | 10 ++++++++++ certbot/main.py | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 44712ada0..72ec94915 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -343,6 +343,16 @@ class HelpfulArgumentParser(object): if parsed_args.csr: self.handle_csr(parsed_args) + # Avoid conflicting args + conficting_args = ["quiet", "noninteractive_mode", "text_mode"] + if parsed_args.dialog_mode: + for arg in conficting_args: + if getattr(parsed_args, arg): + raise errors.Error( + ("Conflicting values for displayer." + " {0} conflicts with dialog_mode").format(arg) + ) + hooks.validate_hooks(parsed_args) return parsed_args diff --git a/certbot/main.py b/certbot/main.py index 99d06a69e..4ef2e6ac8 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -673,16 +673,6 @@ def main(cli_args=sys.argv[1:]): sys.excepthook = functools.partial(_handle_exception, config=config) - # Avoid conflicting args - conficting_args = ["quiet", "noninteractive_mode", "text_mode"] - if config.dialog_mode: - for arg in conficting_args: - if getattr(config, arg): - raise errors.Error( - ("Conflicting values for displayer." - " {0} conflicts with dialog_mode").format(arg) - ) - # Displayer if config.quiet: config.noninteractive_mode = True From 74f5c851782cf40b938f4e6389596d6c994d2f57 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 25 May 2016 23:13:26 +0200 Subject: [PATCH 7/7] Moving tests for conflicting args to cli_test.py --- certbot/tests/cli_test.py | 4 ++++ certbot/tests/main_test.py | 39 +++----------------------------------- 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index afaa53570..20d891324 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -908,6 +908,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(['-c', test_util.vector_path('cli.ini')]) self.assertTrue(mocked_run.called) + def test_conflicting_args(self): + args = ['renew', '--dialog', '--text'] + self.assertRaises(errors.Error, self._call, args) + class DetermineAccountTest(unittest.TestCase): """Tests for certbot.cli._determine_account.""" diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index e3f8b860d..66cba64a3 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1,49 +1,15 @@ """Tests for certbot.main.""" -import os -import shutil -import tempfile import unittest + import mock -import six + from certbot import cli from certbot import configuration -from certbot import errors -from certbot import main from certbot.plugins import disco as plugins_disco -class MainTest(unittest.TestCase): - """Tests for certbot.main""" - - def setUp(self): - self.tmp_dir = tempfile.mkdtemp() - self.config_dir = os.path.join(self.tmp_dir, 'config') - self.work_dir = os.path.join(self.tmp_dir, 'work') - self.logs_dir = os.path.join(self.tmp_dir, 'logs') - self.standard_args = ['--config-dir', self.config_dir, - '--work-dir', self.work_dir, - '--logs-dir', self.logs_dir, '--text'] - - def tearDown(self): - shutil.rmtree(self.tmp_dir) - - def _call(self, args, stdout=None): - "Run the client with output streams mocked out" - args = self.standard_args + args - - toy_stdout = stdout if stdout else six.StringIO() - with mock.patch('certbot.main.sys.stdout', new=toy_stdout): - with mock.patch('certbot.main.sys.stderr') as stderr: - ret = main.main(args[:]) # NOTE: parser can alter its args! - return ret, toy_stdout, stderr - - def test_args_conflict(self): - args = ['renew', '--dialog', '--text'] - self.assertRaises(errors.Error, self._call, args) - - class ObtainCertTest(unittest.TestCase): """Tests for certbot.main.obtain_cert.""" @@ -60,6 +26,7 @@ class ObtainCertTest(unittest.TestCase): config = configuration.NamespaceConfig( cli.prepare_and_parse_args(plugins, args)) + from certbot import main with mock.patch('certbot.main._init_le_client') as mock_init: main.obtain_cert(config, plugins)