Merge "Set shell=True in subprocess.check_call" into stable/gambia

2 files changed, 20 insertions, 24 deletions

diff --git a/xtesting/core/feature.py b/xtesting/core/feature.py
index 86215515..2cb36bd6 100644
--- a/xtesting/core/feature.py
+++ b/xtesting/core/feature.py
@@ -99,17 +99,14 @@ class BashFeature(Feature):
             0 if cmd returns 0,
             -1 otherwise.
         """
-        ret = -1
         try:
             cmd = kwargs["cmd"]
             with open(self.result_file, 'w+') as f_stdout:
-                proc = subprocess.Popen(cmd.split(), stdout=f_stdout,
-                                        stderr=subprocess.STDOUT)
-            ret = proc.wait()
-            self.__logger.info(
-                "Test result is stored in '%s'", self.result_file)
-            if ret != 0:
-                self.__logger.error("Execute command: %s failed", cmd)
+                subprocess.check_call(
+                    cmd, shell=True, stdout=f_stdout, stderr=subprocess.STDOUT)
+            return 0
         except KeyError:
             self.__logger.error("Please give cmd as arg. kwargs: %s", kwargs)
-        return ret
+        except subprocess.CalledProcessError:
+            self.__logger.error("Execute command: %s failed", cmd)
+        return -1
diff --git a/xtesting/tests/unit/core/test_feature.py b/xtesting/tests/unit/core/test_feature.py
index a4ac5af7..47766cdd 100644
--- a/xtesting/tests/unit/core/test_feature.py
+++ b/xtesting/tests/unit/core/test_feature.py
@@ -10,6 +10,7 @@
 # pylint: disable=missing-docstring
 
 import logging
+import subprocess
 import unittest
 
 import mock
@@ -84,33 +85,31 @@ class BashFeatureTesting(FeatureTestingBase):
             self.feature = feature.BashFeature(
                 project_name=self._project_name, case_name=self._case_name)
 
-    @mock.patch('subprocess.Popen')
+    @mock.patch('subprocess.check_call')
     def test_run_no_cmd(self, mock_subproc):
+        delattr(FeatureTesting, "_cmd")
         self.assertEqual(
             self.feature.run(), testcase.TestCase.EX_RUN_ERROR)
         mock_subproc.assert_not_called()
 
-    @mock.patch('subprocess.Popen')
+    @mock.patch('subprocess.check_call',
+                side_effect=subprocess.CalledProcessError(0, '', ''))
     def test_run_ko(self, mock_subproc):
+        setattr(FeatureTesting, "_cmd", "run_bar_tests.py")
         with mock.patch('six.moves.builtins.open', mock.mock_open()) as mopen:
-            mock_obj = mock.Mock()
-            attrs = {'wait.return_value': 1}
-            mock_obj.configure_mock(**attrs)
-
-            mock_subproc.return_value = mock_obj
             self._test_run(testcase.TestCase.EX_RUN_ERROR)
-            mopen.assert_called_once_with(self._output_file, "w+")
+        mopen.assert_called_once_with(self._output_file, "w+")
+        mock_subproc.assert_called_once_with(
+            self._cmd, shell=True, stderr=mock.ANY, stdout=mock.ANY)
 
-    @mock.patch('subprocess.Popen')
+    @mock.patch('subprocess.check_call')
     def test_run(self, mock_subproc):
+        setattr(FeatureTesting, "_cmd", "run_bar_tests.py")
         with mock.patch('six.moves.builtins.open', mock.mock_open()) as mopen:
-            mock_obj = mock.Mock()
-            attrs = {'wait.return_value': 0}
-            mock_obj.configure_mock(**attrs)
-
-            mock_subproc.return_value = mock_obj
             self._test_run(testcase.TestCase.EX_OK)
-            mopen.assert_called_once_with(self._output_file, "w+")
+        mopen.assert_called_once_with(self._output_file, "w+")
+        mock_subproc.assert_called_once_with(
+            self._cmd, shell=True, stderr=mock.ANY, stdout=mock.ANY)
 
 
 if __name__ == "__main__":