From fef9715d4acb690dff1cc9f08545721d69bf208d Mon Sep 17 00:00:00 2001 From: Ondrej Nosek Date: Wed, 7 Sep 2022 19:53:05 +0200 Subject: [PATCH] `fedpkg local` does not show rpmbuild output subprocess.communicate() method didn't allow a direct pipe output to the shell and therefore wasn't shown to the user. Switched to check_call method. Additionally, the correct exit code is returned when the first part of the command fails. Resolves: rhbz#2124809 JIRA: RHELCMP-9960 Signed-off-by: Ondrej Nosek --- pyrpkg/__init__.py | 12 +++++------- tests/test_cli.py | 26 ++++++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index a672dd2..1b6a0c4 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -2818,14 +2818,12 @@ class Commands(object): stdout=subprocess.PIPE, stderr=subprocess.STDOUT ) - tee = subprocess.Popen( + subprocess.check_call( ("tee", logfile), - stdin=rpmbuild.stdout, - stdout=subprocess.PIPE) - rpmbuild.stdout.close() - tee.communicate() - - except subprocess.SubprocessError: + stdin=rpmbuild.stdout) + rpmbuild.communicate() # without this, 'returncode' is None (=unfinished process) + sys.exit(rpmbuild.returncode) + except subprocess.CalledProcessError: raise rpkgError(debug_cmd) finally: self._cleanup_tmp_dir(tmpdir) diff --git a/tests/test_cli.py b/tests/test_cli.py index 254bfac..97ae0ce 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -846,10 +846,12 @@ class TestLocal(CliTestCase): create_repo_per_test = False + @patch('sys.exit') + @patch('pyrpkg.subprocess.check_call') @patch('subprocess.Popen') @patch('pyrpkg.Commands.rel') @patch('pyrpkg.Commands.ver') - def test_local(self, ver, rel, popen): + def test_local(self, ver, rel, popen, check_call, system_exit): cli_cmd = ['rpkg', '--path', self.cloned_repo_path, '--release', 'rhel-6', 'local'] rel.__str__ = Mock() @@ -868,20 +870,21 @@ class TestLocal(CliTestCase): popen.assert_has_calls([ # at the beginning of this list, there are other calls from load_nameverrel call(rpmbuild, stdout=-1, stderr=-2), - # I can't match this call - stdin=Mock has it's dynamic id. Therefore any_oreder=True - # call(tee, stdin=Mock(), stdout=-1), # check call [-3] separately - call().stdout.close(), call().communicate(), - ], any_order=True) + ], any_order=False) - tee_call_arg = popen.mock_calls[-3] + tee_call_arg = check_call.mock_calls[0] if 'args' in dir(tee_call_arg): # doesn't work in <=py36 self.assertEqual(tee, tee_call_arg.args[0]) + system_exit.assert_called_once() + + @patch('sys.exit') + @patch('pyrpkg.subprocess.check_call') @patch('subprocess.Popen') @patch('pyrpkg.Commands.rel') @patch('pyrpkg.Commands.ver') - def test_local_with_options(self, ver, rel, popen): + def test_local_with_options(self, ver, rel, popen, check_call, system_exit): builddir = os.path.join(self.cloned_repo_path, 'this-builddir') buildrootdir = os.path.join(self.cloned_repo_path, 'this-buildrootdir') @@ -913,16 +916,15 @@ class TestLocal(CliTestCase): popen.assert_has_calls([ # at the beginning of this list, there are other calls from load_nameverrel call(rpmbuild, stdout=-1, stderr=-2), - # I can't match this call - stdin=Mock has it's dynamic id. Therefore any_oreder=True - # call(tee, stdin=Mock(), stdout=-1), # check call [-3] separately - call().stdout.close(), call().communicate(), - ], any_order=True) + ], any_order=False) - tee_call_arg = popen.mock_calls[-3] + tee_call_arg = check_call.mock_calls[0] if 'args' in dir(tee_call_arg): # doesn't work in <=py36 self.assertEqual(tee, tee_call_arg.args[0]) + system_exit.assert_called_once() + class TestVerifyFiles(CliTestCase): -- 2.37.2