From 10579ee6f574ac32608913ffbc8a8141c63485ce Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Fri, 28 Aug 2020 15:39:12 +0200
Subject: [PATCH] buildroot: return a new CompletedBuild with output

Create a new CompletedBuild object that wraps and is very similar
to the subprocess.CompletedProcess, i.e. it has a process member
but also has shortcuts for returncode. Additionally, the output
of the process is not only forwarded to the monitor, but also
captured and then handed to CompletedBuild, so its output member
will actually contain the full build output. To be compatible
with the previously returned CompletedProcess, `stderr`, `stdout`
members exist on CompletedBuild that also return `output`.
---
 osbuild/buildroot.py       | 36 +++++++++++++++++++++++++++++++++++-
 test/mod/test_buildroot.py |  2 ++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/osbuild/buildroot.py b/osbuild/buildroot.py
index faf593bc..05859038 100644
--- a/osbuild/buildroot.py
+++ b/osbuild/buildroot.py
@@ -8,6 +8,7 @@ little access to the outside as possible.
 import contextlib
 import importlib
 import importlib.util
+import io
 import os
 import stat
 import subprocess
@@ -19,6 +20,31 @@ __all__ = [
 ]
 
 
+class CompletedBuild:
+    """The result of a `BuildRoot.run`
+
+    Contains the actual `process` that was executed but also has
+    convenience properties to quickly access the `returncode` and
+    `output`. The latter is also provided via `stderr`, `stdout`
+    properties, making it a drop-in replacement for `CompletedProcess`.
+    """
+    def __init__(self, proc: subprocess.CompletedProcess, output: str):
+        self.process = proc
+        self.output = output
+
+    @property
+    def returncode(self):
+        return self.process.returncode
+
+    @property
+    def stdout(self):
+        return self.output
+
+    @property
+    def stderr(self):
+        return self.output
+
+
 # pylint: disable=too-many-instance-attributes
 class BuildRoot(contextlib.AbstractContextManager):
     """Build Root
@@ -118,6 +144,8 @@ class BuildRoot(contextlib.AbstractContextManager):
 
         This must be called from within an active context of this buildroot
         context-manager.
+
+        Returns a `CompletedBuild` object.
         """
 
         if not self._exitstack:
@@ -202,14 +230,20 @@ class BuildRoot(contextlib.AbstractContextManager):
                                 encoding="utf-8",
                                 close_fds=True)
 
+        data = io.StringIO()
+
         while True:
             txt = proc.stdout.read(4096)
             if not txt:
                 break
 
+            data.write(txt)
             monitor.log(txt)
 
         txt, _ = proc.communicate()
         monitor.log(txt)
+        data.write(txt)
+        output = data.getvalue()
+        data.close()
 
-        return proc
+        return CompletedBuild(proc, output)
diff --git a/test/mod/test_buildroot.py b/test/mod/test_buildroot.py
index bfba9170..451bb677 100644
--- a/test/mod/test_buildroot.py
+++ b/test/mod/test_buildroot.py
@@ -68,6 +68,8 @@ class TestBuildRoot(test.TestBase):
         with open(logfile) as f:
             log = f.read()
         assert log
+        assert r.output
+        self.assertEqual(log, r.output)
 
     @unittest.skipUnless(test.TestBase.have_test_data(), "no test-data access")
     def test_bind_mounts(self):