From 64daefea6b5d1cce9ad1828668f23ceef17591de Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 17 Dec 2016 14:51:34 -0800 Subject: [PATCH] util.exe_exists and hooks._which were almost identical; merge them --- certbot/hooks.py | 24 ++--------------------- certbot/tests/hook_test.py | 13 +------------ certbot/tests/util_test.py | 39 ++++++++++++-------------------------- certbot/util.py | 34 ++++++++++++++++++++------------- 4 files changed, 36 insertions(+), 74 deletions(-) diff --git a/certbot/hooks.py b/certbot/hooks.py index 6aece9bde..4662c4515 100644 --- a/certbot/hooks.py +++ b/certbot/hooks.py @@ -7,6 +7,7 @@ import os from subprocess import Popen, PIPE from certbot import errors +from certbot import util logger = logging.getLogger(__name__) @@ -18,7 +19,7 @@ def validate_hooks(config): def _prog(shell_cmd): """Extract the program run by a shell command""" - cmd = _which(shell_cmd) + cmd = util.which(shell_cmd) return os.path.basename(cmd) if cmd else None @@ -108,24 +109,3 @@ def execute(shell_cmd): logger.error('Error output from %s:\n%s', _prog(shell_cmd), err) return (err, out) - -def _is_exe(fpath): - return os.path.isfile(fpath) and os.access(fpath, os.X_OK) - -def _which(program): - """Test if program is in the path.""" - # Borrowed from: - # https://stackoverflow.com/questions/377017/test-if-executable-exists-in-python - # XXX May need more porting to handle .exe extensions on Windows - - fpath, _fname = os.path.split(program) - if fpath: - if _is_exe(program): - return program - else: - for path in os.environ["PATH"].split(os.pathsep): - exe_file = os.path.join(path, program) - if _is_exe(exe_file): - return exe_file - - return None diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index 3c55b675c..95fede230 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -27,19 +27,8 @@ class HookTest(unittest.TestCase): config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="") self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config) - @mock.patch('certbot.hooks._is_exe') - def test_which(self, mock_is_exe): - mock_is_exe.return_value = True - self.assertEqual(hooks._which("/path/to/something"), "/path/to/something") - with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}): - mock_is_exe.return_value = True - self.assertEqual(hooks._which("pingify"), "/floop/pingify") - mock_is_exe.return_value = False - self.assertEqual(hooks._which("pingify"), None) - self.assertEqual(hooks._which("/path/to/something"), None) - - @mock.patch('certbot.hooks._which') + @mock.patch('certbot.hooks.util.which') def test_prog(self, mockwhich): mockwhich.return_value = "/very/very/funky" self.assertEqual(hooks._prog("funky"), "funky") diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index cd02d835d..866f71e3d 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -45,34 +45,19 @@ class RunScriptTest(unittest.TestCase): self.assertRaises(errors.SubprocessError, self._call, ["test"]) -class ExeExistsTest(unittest.TestCase): - """Tests for certbot.util.exe_exists.""" +class WhichTest(unittest.TestCase): + @mock.patch('certbot.util._is_exe') + def test_which(self, mock_is_exe): + from certbot.util import which + mock_is_exe.return_value = True + self.assertEqual(which("/path/to/something"), "/path/to/something") - @classmethod - def _call(cls, exe): - from certbot.util import exe_exists - return exe_exists(exe) - - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_full_path(self, mock_access, mock_isfile): - mock_access.return_value = True - mock_isfile.return_value = True - self.assertTrue(self._call("/path/to/exe")) - - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_on_path(self, mock_access, mock_isfile): - mock_access.return_value = True - mock_isfile.return_value = True - self.assertTrue(self._call("exe")) - - @mock.patch("certbot.util.os.path.isfile") - @mock.patch("certbot.util.os.access") - def test_not_found(self, mock_access, mock_isfile): - mock_access.return_value = False - mock_isfile.return_value = True - self.assertFalse(self._call("exe")) + with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}): + mock_is_exe.return_value = True + self.assertEqual(which("pingify"), "/floop/pingify") + mock_is_exe.return_value = False + self.assertEqual(which("pingify"), None) + self.assertEqual(which("/path/to/something"), None) class MakeOrVerifyDirTest(unittest.TestCase): diff --git a/certbot/util.py b/certbot/util.py index cbcfa3314..c09d307d2 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -66,28 +66,36 @@ def run_script(params): return stdout, stderr -def exe_exists(exe): +def _is_exe(fpath): + return os.path.isfile(fpath) and os.access(fpath, os.X_OK) + +def which(program): """Determine whether path/name refers to an executable. - :param str exe: Executable path or name + :param str program: Executable path or name - :returns: If exe is a valid executable - :rtype: bool + :returns: Path to executable if it exists, or None + :rtype: str or None """ - def is_exe(path): - """Determine if path is an exe.""" - return os.path.isfile(path) and os.access(path, os.X_OK) + # Borrowed from: + # https://stackoverflow.com/questions/377017/test-if-executable-exists-in-python + # XXX May need more porting to handle .exe extensions on Windows - path, _ = os.path.split(exe) - if path: - return is_exe(exe) + fpath, _fname = os.path.split(program) + if fpath: + if _is_exe(program): + return program else: for path in os.environ["PATH"].split(os.pathsep): - if is_exe(os.path.join(path, exe)): - return True + exe_file = os.path.join(path, program) + if _is_exe(exe_file): + return exe_file - return False + return None + +# Either name makes sense, and we have a lot of call sites! +exe_exists = which def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False):