1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
|
From fef9715d4acb690dff1cc9f08545721d69bf208d Mon Sep 17 00:00:00 2001
From: Ondrej Nosek <onosek@redhat.com>
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 <onosek@redhat.com>
---
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
|
