curtin

Merge ~smoser/curtin:cleanup/capture-combined-return-empty-not-none into curtin:master

  1. Git
  2. lp:~smoser/curtin
  3. cleanup/capture-combined-return-empty-not-none
  4. Merge into master
Proposed by Scott Moser
Status: Merged
Approved by: Scott Moser
Approved revision: de680f70c8cd8114d064e06b25f8d020f89bfe91
Merge reported by: Scott Moser
Merged at revision: 3e0c1c2668f259233cfd21be7ed48af335c734da
Proposed branch: ~smoser/curtin:cleanup/capture-combined-return-empty-not-none
Merge into: curtin:master
Diff against target: 42 lines (+8/-5)
2 files modified
curtin/util.py (+7/-4)
tests/unittests/test_util.py (+1/-1)
Reviewer Review Type Date Requested Status
Ryan Harper (community) Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+347067@code.launchpad.net

Commit message

subp: update return value of subp with combine_capture=True.

combine_capture previously would return None in the stderr response.
This was inconsistent with capture=True, combine_capture=False.
It seems more consistent to always return the bytes or string as
documented. Test case and docstring are updated accordingly.

Description of the change

see commit message

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:de680f70c8cd8114d064e06b25f8d020f89bfe91
https://jenkins.ubuntu.com/server/job/curtin-ci/959/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/959
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/959
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/959
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/959

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/959/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

LGTM

review: Approve
Revision history for this message
Scott Moser (smoser) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/curtin/commit/?id=3e0c1c26

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/util.py b/curtin/util.py
2index de0eb88..852c54b 100644
3--- a/curtin/util.py
4+++ b/curtin/util.py
5@@ -103,10 +103,11 @@ def _subp(args, data=None, rcs=None, env=None, capture=False,
6 (out, err) = sp.communicate(data)
7
8 # Just ensure blank instead of none.
9- if not out and capture:
10- out = b''
11- if not err and capture:
12- err = b''
13+ if capture or combine_capture:
14+ if not out:
15+ out = b''
16+ if not err:
17+ err = b''
18 if decode:
19 def ldecode(data, m='utf-8'):
20 if not isinstance(data, bytes):
21@@ -206,6 +207,8 @@ def subp(*args, **kwargs):
22 boolean indicating if stderr should be redirected to stdout. When True,
23 interleaved stderr and stdout will be returned as the first element of
24 a tuple.
25+ if combine_capture is True, then output is captured independent of
26+ the value of capture.
27 :param log_captured:
28 boolean indicating if output should be logged on capture. If
29 True, then stderr and stdout will be logged at DEBUG level. If
30diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
31index 65175c5..483cd5d 100644
32--- a/tests/unittests/test_util.py
33+++ b/tests/unittests/test_util.py
34@@ -260,7 +260,7 @@ class TestSubp(CiTestCase):
35 data = b'hello world'
36 (out, err) = util.subp(self.stdin2err, combine_capture=True,
37 decode=False, data=data)
38- self.assertIsNone(err)
39+ self.assertEqual(err, b'')
40 self.assertEqual(out, data)
41
42 def test_returns_none_if_no_capture(self):

Subscribers

People subscribed via source and target branches