From 52ee2c7519bd261c98232b7aa1e665f7f3af1d08 Mon Sep 17 00:00:00 2001 From: Maarten de Waard Date: Fri, 9 Sep 2016 12:00:47 +0200 Subject: [PATCH 1/3] better tests, 89% coverage --- .gitignore | 2 + certbot_haproxy/tests/__init__.py | 3 - certbot_haproxy/tests/test_authenticator.py | 36 ++++++++++ certbot_haproxy/tests/test_installer.py | 80 ++++++++++++++++++--- 4 files changed, 107 insertions(+), 14 deletions(-) create mode 100644 certbot_haproxy/tests/test_authenticator.py diff --git a/.gitignore b/.gitignore index eaf0f79..39a2eb3 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ pip-log.txt # Unit test / coverage reports .coverage .tox +htmlcov # Translations *.mo @@ -44,3 +45,4 @@ certbot.log # tmp files: *.swp + diff --git a/certbot_haproxy/tests/__init__.py b/certbot_haproxy/tests/__init__.py index c295616..09efe79 100644 --- a/certbot_haproxy/tests/__init__.py +++ b/certbot_haproxy/tests/__init__.py @@ -11,6 +11,3 @@ def load_tests(loader, tests, pattern=None): suite = loader.discover('certbot_haproxy/tests', pattern=pattern) suite.addTests(tests) return suite - -if __name__ == '__main__': - unittest.main() diff --git a/certbot_haproxy/tests/test_authenticator.py b/certbot_haproxy/tests/test_authenticator.py new file mode 100644 index 0000000..02fcbb2 --- /dev/null +++ b/certbot_haproxy/tests/test_authenticator.py @@ -0,0 +1,36 @@ +import unittest +import mock +import os + +from certbot_haproxy.authenticator import HAProxyAuthenticator +from acme import challenges + +class TestAuthenticator(unittest.TestCase): + + test_domain = 'le.wtf' + + """Test the relevant functions of the certbot_haproxy installer""" + + def setUp(self): + mock_le_config = mock.MagicMock( + # TODO: Don't know what we need here + ) + self.authenticator = HAProxyAuthenticator( + config=mock_le_config, name="authenticator") + + def test_more_info(self): + info = self.authenticator.more_info() + self.assertIsInstance(info, str) + + @mock.patch("certbot_haproxy.authenticator.logger") + @mock.patch("certbot.util.logger") + def test_add_parser_arguments(self, util_logger, certbot_logger): + """Weak test taken from apache plugin tests""" + self.authenticator.add_parser_arguments(mock.MagicMock()) + self.assertEqual(certbot_logger.error.call_count, 0) + self.assertEqual(util_logger.error.call_count, 0) + + def test_supported_challenges(self): + chal = self.authenticator.supported_challenges + self.assertIsInstance(chal, list) + self.assertTrue(challenges.HTTP01 in chal) diff --git a/certbot_haproxy/tests/test_installer.py b/certbot_haproxy/tests/test_installer.py index 095de72..7a849c4 100644 --- a/certbot_haproxy/tests/test_installer.py +++ b/certbot_haproxy/tests/test_installer.py @@ -15,6 +15,9 @@ def _conf(self, var): @mock.patch("certbot_haproxy.installer.HAProxyInstaller.conf", new=_conf) class TestInstaller(unittest.TestCase): + + test_domain = 'le.wtf' + """Test the relevant functions of the certbot_haproxy installer""" def setUp(self): @@ -83,7 +86,6 @@ class TestInstaller(unittest.TestCase): def test_deploy_cert_save(self): """Deploy and save a certificate and rollback after that""" # Variables for test: - domain = 'le.wtf' crt_dir = os.path.join(self.temp_dir, self.test_dir, "deploy_test") base = os.path.join(self.temp_dir, self.test_dir, "deploy_cert") key_path = os.path.join(base, "privkey.pem") @@ -99,37 +101,70 @@ class TestInstaller(unittest.TestCase): self.assertRaises( errors.PluginError, self.installer.deploy_cert, - domain, 'no-cert', 'no-key') + self.test_domain, 'no-cert', 'no-key') # Arguments for several tests all_args = [ - (domain, cert_path, key_path), - (domain, cert_path, key_path, chain_path), - (domain, None, key_path, None, fullchain_path), + (self.test_domain, cert_path, key_path), + (self.test_domain, cert_path, key_path, chain_path), + (self.test_domain, None, key_path, None, fullchain_path), ] # Run deploy and save with all types of args for args in all_args: # Deploy with only key and cert self.installer.deploy_cert(*args) + + try: + self.installer.view_config_changes() + except ReverterError: + self.fail("Reverter failed") + except PluginError: + self.fail("Reverter failed with PluginError") + self.installer.save() # Check if le.wtf.pem is created - pem = os.path.join(crt_dir, domain) + self.installer.crt_postfix + pem = os.path.join(crt_dir, self.test_domain) \ + + self.installer.crt_postfix self.assertTrue(os.path.isfile(pem)) # Roll back pem creation self.installer.rollback_checkpoints() # Check if file was removed again self.assertFalse(os.path.isfile(pem)) + # Try to revert: + try: + self.installer.recovery_routine() + except PluginError: + self.fail("Recovery routine didn't work") + + # fail without key + self.assertRaises( + errors.PluginError, + self.installer.deploy_cert, + self.test_domain, cert_path, None) + + # Run twice (should update instead of create) + args = (self.test_domain, cert_path, key_path) + self.installer.deploy_cert(*args) + self.installer.save() + self.installer.deploy_cert(*args) + self.installer.save() + + + def test_enhancement(self): + """ Currently no enhancements are supported, we should see that """ + self.assertRaises( + errors.PluginError, + self.installer.enhance, + self.test_domain, + "non-existent-enhancement") + + @mock.patch("certbot_haproxy.installer.logger") @mock.patch("certbot.util.logger") def test_config_test(self, util_logger, certbot_logger): """Test config_test function with a faulty and a valid cfg file""" - # Check with current config file - self.installer.config_test() - self.assertEqual(certbot_logger.error.call_count, 0) - self.assertEqual(util_logger.error.call_count, 0) - # Check with bad config file self.installer.config.haproxy_config = os.path.join( self.temp_dir, self.test_dir, "haproxy_bad.cfg") @@ -145,3 +180,26 @@ class TestInstaller(unittest.TestCase): errors.MisconfigurationError, self.installer.config_test ) + + def test_more_info(self): + ret = self.installer.more_info() + self.assertIsInstance(ret, basestring) + + @mock.patch('certbot.util.exe_exists', return_value=False) + def test_failed_service_command(self, mock_exe_exists): + """ Fail on service manager command """ + self.assertRaises(errors.NoInstallationError, self.installer.prepare) + mock_exe_exists.assert_called_once() + + @mock.patch('subprocess.check_output', + return_value='not-really-a-version-number') + def test_no_version_number(self, mock_check_output): + """ Fail on version command """ + self.assertRaises(errors.NoInstallationError, self.installer.prepare) + + @mock.patch('subprocess.check_output', + return_value='HA-Proxy version 1.4.8 2014/10/31') + def test_wrong_version_number(self, mock_check_output): + """ Supply a too low version number for HAproxy """ + self.assertRaises(errors.NotSupportedError, self.installer.prepare) + mock_check_output.assert_called_once() From 4a51180f4d44cc9ff2c127a2a44690aa3e8defbd Mon Sep 17 00:00:00 2001 From: Maarten de Waard Date: Fri, 9 Sep 2016 13:29:51 +0200 Subject: [PATCH 2/3] stage1 --- certbot_haproxy/installer.py | 4 ++-- certbot_haproxy/tests/__init__.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/certbot_haproxy/installer.py b/certbot_haproxy/installer.py index bfdf1f9..47eb149 100644 --- a/certbot_haproxy/installer.py +++ b/certbot_haproxy/installer.py @@ -523,7 +523,7 @@ class HAProxyInstaller(common.Plugin): contents = pem.read() if self._cert_key_check(contents, filepath): yield (filepath, filepath, self.conf("haproxy-config")) - except IOError, err: + except IOError as err: logger.error( "Can't access \"%s\", reason:\n %s", filepath, @@ -544,7 +544,7 @@ class HAProxyInstaller(common.Plugin): except TypeError: logger.warn("Could not read certificate, wrong type (not PEM)") # Documentation says it raises "Error" - except Exception, err: # pylint: disable=broad-except + except Exception as err: # pylint: disable=broad-except logger.error("Unexpected error! %s", err) if issuer == self.conf('haproxy-ca-common-name') and key.check(): diff --git a/certbot_haproxy/tests/__init__.py b/certbot_haproxy/tests/__init__.py index 09efe79..39c364b 100644 --- a/certbot_haproxy/tests/__init__.py +++ b/certbot_haproxy/tests/__init__.py @@ -1,4 +1,5 @@ """Certbot HAProxy Tests""" +from __future__ import print_function import unittest @@ -6,7 +7,7 @@ def load_tests(loader, tests, pattern=None): """Find all python files in the tests folder""" if pattern is None: pattern = 'test_*.py' - print "loader: ", loader + print("loader: ", loader) suite = loader.discover('certbot_haproxy/tests', pattern=pattern) suite.addTests(tests) From e17594e997101d46a8926006634f90a3b6d0fdb0 Mon Sep 17 00:00:00 2001 From: Maarten de Waard Date: Fri, 9 Sep 2016 13:32:01 +0200 Subject: [PATCH 3/3] stage2 --- certbot_haproxy/installer.py | 6 ++++-- certbot_haproxy/tests/test_installer.py | 3 ++- certbot_haproxy/util.py | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/certbot_haproxy/installer.py b/certbot_haproxy/installer.py index 47eb149..28c5fa4 100644 --- a/certbot_haproxy/installer.py +++ b/certbot_haproxy/installer.py @@ -26,6 +26,8 @@ Be sure to replace `$USER` with the user that will be running the lehaproxy installer. """ +from builtins import str +from past.builtins import basestring import logging import os import glob @@ -134,7 +136,7 @@ class HAProxyInstaller(common.Plugin): " returns letsencrypt certificates. Defaults to the value" " 'h2ppy h2cker fake CA' that is used by the local boulder." ), - type=unicode, + type=str, default=u'Let\'s Encrypt Authority X3' ) add( @@ -464,7 +466,7 @@ class HAProxyInstaller(common.Plugin): # Write all new files and changes: for filepath, contents in \ - self.new_crt_files.items() + self.crt_files.items(): + list(self.new_crt_files.items()) + list(self.crt_files.items()): # Make sure directory of filepath exists path = os.path.dirname(os.path.abspath(filepath)) diff --git a/certbot_haproxy/tests/test_installer.py b/certbot_haproxy/tests/test_installer.py index 7a849c4..b609686 100644 --- a/certbot_haproxy/tests/test_installer.py +++ b/certbot_haproxy/tests/test_installer.py @@ -1,4 +1,5 @@ """Test installer functions""" +from past.builtins import basestring import unittest import mock import os @@ -75,7 +76,7 @@ class TestInstaller(unittest.TestCase): from OpenSSL import crypto self.installer.new_crt_files = {} self.installer._fall_back_cert() - key = self.installer.new_crt_files.keys()[0] + key = list(self.installer.new_crt_files.keys())[0] cert = self.installer.new_crt_files[key] self.assertIsInstance(key, str) self.assertIsInstance(cert, str) diff --git a/certbot_haproxy/util.py b/certbot_haproxy/util.py index a55483c..2d388ed 100644 --- a/certbot_haproxy/util.py +++ b/certbot_haproxy/util.py @@ -1,6 +1,7 @@ """ Utility functions. """ +from builtins import object from OpenSSL import crypto import socket