Fix local command execution.

- Setting non-blocking on process IO streams produced process execution
  failures (exit code 1).
- Use local_process function to test tobiko-keystone-credentials
  command
- Create fixture to add prefix bin directory to PATH environment
  variable

Change-Id: Idd0f2001923bbf7f2108688a6331e6fe359f6690

tobiko/shell/sh/__init__.py

@@ -39,6 +39,7 @@ ShellExecuteResult = _execute.ShellExecuteResult
 local_execute = _local.local_execute
 local_process = _local.local_process
 LocalShellProcessFixture = _local.LocalShellProcessFixture
+LocalExecutePathFixture = _local.LocalExecutePathFixture
 
 process = _process.process
 ShellProcessFixture = _process.ShellProcessFixture

tobiko/shell/sh/_local.py

@@ -18,11 +18,14 @@ from __future__ import absolute_import
 import fcntl
 import os
 import subprocess
+import sys
 
 from oslo_log import log
 
+import tobiko
 from tobiko.shell.sh import _io
 from tobiko.shell.sh import _execute
+from tobiko.shell.sh import _path
 from tobiko.shell.sh import _process
 
 
@@ -54,33 +57,40 @@ def local_process(command, environment=None, current_dir=None, timeout=None,
         stderr=stderr)
 
 
+class LocalExecutePathFixture(_path.ExecutePathFixture):
+
+    executable_dirs = [os.path.realpath(os.path.join(d, 'bin'))
+                       for d in [sys.prefix, sys.exec_prefix]]
+    environ = os.environ
+
+
 class LocalShellProcessFixture(_process.ShellProcessFixture):
 
+    path_execute = tobiko.required_fixture(LocalExecutePathFixture)
+
     def create_process(self):
+        tobiko.setup_fixture(self.path_execute)
         parameters = self.parameters
-        popen_params = {}
-        if parameters.stdin:
-            popen_params.update(stdin=subprocess.PIPE)
-        if parameters.stdout:
-            popen_params.update(stdout=subprocess.PIPE)
-        if parameters.stderr:
-            popen_params.update(stderr=subprocess.PIPE)
-        return subprocess.Popen(
-            args=self.command,
-            bufsize=parameters.buffer_size,
-            shell=False,
-            cwd=parameters.current_dir,
-            env=merge_dictionaries(os.environ, parameters.environment),
-            universal_newlines=False,
-            **popen_params)
+        args = self.command
+        stdin = parameters.stdin and subprocess.PIPE or None
+        stdout = parameters.stdout and subprocess.PIPE or None
+        stderr = parameters.stderr and subprocess.PIPE or None
+        env = merge_dictionaries(os.environ, parameters.environment)
+        return subprocess.Popen(args=args,
+                                bufsize=parameters.buffer_size,
+                                shell=False,
+                                universal_newlines=False,
+                                env=env,
+                                cwd=parameters.current_dir,
+                                stdin=stdin,
+                                stdout=stdout,
+                                stderr=stderr)
 
     def setup_stdin(self):
-        set_non_blocking(self.process.stdin.fileno())
         self.stdin = _io.ShellStdin(delegate=self.process.stdin,
                               buffer_size=self.parameters.buffer_size)
 
     def setup_stdout(self):
-        set_non_blocking(self.process.stdout.fileno())
         self.stdout = _io.ShellStdout(delegate=self.process.stdout,
                                       buffer_size=self.parameters.buffer_size)
 

tobiko/shell/sh/_path.py

@@ -0,0 +1,68 @@
+# Copyright (c) 2019 Red Hat, Inc.
+#
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+from __future__ import absolute_import
+
+import collections
+
+from oslo_log import log
+
+import tobiko
+
+
+LOG = log.getLogger(__name__)
+
+
+class ExecutePathFixture(tobiko.SharedFixture):
+
+    executable_dirs = None
+    environ = None
+
+    def __init__(self, executable_dirs=None, environ=None):
+        super(ExecutePathFixture, self).__init__()
+        if executable_dirs:
+            self.executable_dirs = tuple(executable_dirs)
+        tobiko.check_valid_type(self.executable_dirs, collections.Iterable)
+
+        if environ is not None:
+            self.environ = environ
+        tobiko.check_valid_type(self.environ, collections.Mapping)
+
+    def setup_fixture(self):
+        missing_dirs = []
+        for executable_dir in self.executable_dirs:
+            if (executable_dir not in self.path_dirs and
+                    executable_dir not in missing_dirs):
+                missing_dirs.append(executable_dir)
+        if missing_dirs:
+            new_path_dirs = missing_dirs + self.path_dirs
+            self.addCleanup(self.remove_executable_dirs, missing_dirs)
+            self.environ['PATH'] = ':'.join(new_path_dirs)
+            LOG.debug('Directories added to executable path: %s',
+                      ', '.join(sorted(new_path_dirs)))
+
+    @property
+    def path_dirs(self):
+        return list(self.environ.get('PATH', '').split(':'))
+
+    def remove_executable_dirs(self, removal_dirs):
+        removal_dirs = set(removal_dirs) & set(self.path_dirs)
+        if removal_dirs:
+            new_path_dirs = [p
+                             for p in self.path_dirs
+                             if p not in removal_dirs]
+            self.environ['PATH'] = ':'.join(new_path_dirs)
+            LOG.debug('Directories removed from executable path: %s',
+                      ', '.join(sorted(removal_dirs)))

tobiko/tests/functional/openstack/test_keystone.py

@@ -15,46 +15,22 @@
 #    under the License.
 from __future__ import absolute_import
 
-import contextlib
-import os
-import subprocess
-
+from oslo_log import log
 import testtools
 import yaml
 
 from tobiko.openstack import keystone
+from tobiko.shell import sh
+
+
+LOG = log.getLogger(__name__)
 
 
 class TobikoKeystoneCredentialsCommandTest(testtools.TestCase):
 
     def test_execute(self):
-        with execute('tobiko-keystone-credentials') as process:
+        with sh.local_process('tobiko-keystone-credentials') as process:
             actual = yaml.full_load(process.stdout)
-
+        process.check_exit_status()
         expected = keystone.default_keystone_credentials().to_dict()
         self.assertEqual(expected, actual)
-
-
-@contextlib.contextmanager
-def execute(command, check_exit_status=0):
-    process = subprocess.Popen(command,
-                               shell=True,
-                               env=os.environ,
-                               stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE,
-                               universal_newlines=True)
-
-    try:
-        yield process
-
-        process.wait()
-        if (check_exit_status is not None and
-                check_exit_status != process.returncode):
-            error = process.stderr.read()
-            message = "Unexpected exit status ({!s}):\n{!s}".format(
-                process.returncode, error)
-            raise RuntimeError(message)
-
-    finally:
-        if process.returncode is None:
-            process.kill()