launchpad-reviewers team mailing list archive

[Colin Watson <mp+411509@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:remove-find-on-path into launchpad:master.

Commit message:
Remove find_on_path in favour of shutil.which

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/411509
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:remove-find-on-path into launchpad:master.

diff --git a/lib/lp/scripts/utilities/withxvfb.py b/lib/lp/scripts/utilities/withxvfb.py
index c466d82..992e830 100755
--- a/lib/lp/scripts/utilities/withxvfb.py
+++ b/lib/lp/scripts/utilities/withxvfb.py
@@ -11,16 +11,15 @@ Follows sinzui's advice to the launchpad-dev list:
 """
 
 import os
+import shutil
 import sys
 
-from lp.services.osutils import find_on_path
-
 
 def main():
     # Look for the command to run in this script's directory if it's not
     # found along the default PATH.
     args = sys.argv[1:]
-    if args and not find_on_path(args[0]):
+    if args and not shutil.which(args[0]):
         nearby = os.path.join(os.path.dirname(sys.argv[0]), args[0])
         if os.access(nearby, os.X_OK):
             args[0] = nearby
diff --git a/lib/lp/services/osutils.py b/lib/lp/services/osutils.py
index 75abdef..1a4e193 100644
--- a/lib/lp/services/osutils.py
+++ b/lib/lp/services/osutils.py
@@ -5,7 +5,6 @@
 
 __all__ = [
     'ensure_directory_exists',
-    'find_on_path',
     'get_pid_from_file',
     'kill_by_pidfile',
     'open_for_writing',
@@ -192,20 +191,6 @@ def write_file(path, content):
         f.write(content)
 
 
-def find_on_path(command):
-    """Is 'command' on the executable search path?"""
-    if "PATH" not in os.environ:
-        return False
-    path = os.environ["PATH"]
-    for element in path.split(os.pathsep):
-        if not element:
-            continue
-        filename = os.path.join(element, command)
-        if os.path.isfile(filename) and os.access(filename, os.X_OK):
-            return True
-    return False
-
-
 def process_exists(pid):
     """Return True if the specified process already exists."""
     try:
diff --git a/lib/lp/services/tests/test_osutils.py b/lib/lp/services/tests/test_osutils.py
index 4fca9f8..2b72da2 100644
--- a/lib/lp/services/tests/test_osutils.py
+++ b/lib/lp/services/tests/test_osutils.py
@@ -7,15 +7,11 @@ import errno
 import os
 import tempfile
 
-from fixtures import (
-    EnvironmentVariable,
-    MockPatch,
-    )
+from fixtures import MockPatch
 from testtools.matchers import FileContains
 
 from lp.services.osutils import (
     ensure_directory_exists,
-    find_on_path,
     open_for_writing,
     process_exists,
     remove_tree,
@@ -117,29 +113,6 @@ class TestWriteFile(TestCase):
         self.assertThat(filename, FileContains(content))
 
 
-class TestFindOnPath(TestCase):
-
-    def test_missing_environment(self):
-        self.useFixture(EnvironmentVariable("PATH"))
-        self.assertFalse(find_on_path("ls"))
-
-    def test_present_executable(self):
-        temp_dir = self.makeTemporaryDirectory()
-        bin_dir = os.path.join(temp_dir, "bin")
-        program = os.path.join(bin_dir, "program")
-        write_file(program, b"")
-        os.chmod(program, 0o755)
-        self.useFixture(EnvironmentVariable("PATH", bin_dir))
-        self.assertTrue(find_on_path("program"))
-
-    def test_present_not_executable(self):
-        temp_dir = self.makeTemporaryDirectory()
-        bin_dir = os.path.join(temp_dir, "bin")
-        write_file(os.path.join(bin_dir, "program"), b"")
-        self.useFixture(EnvironmentVariable("PATH", bin_dir))
-        self.assertFalse(find_on_path("program"))
-
-
 class TestProcessExists(TestCase):
 
     def test_with_process_running(self):