qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/5] python/utils: add VerboseProcessError


From: Hanna Reitz
Subject: Re: [PATCH v3 2/5] python/utils: add VerboseProcessError
Date: Thu, 17 Mar 2022 17:34:13 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

On 17.03.22 17:31, John Snow wrote:
On Thu, Mar 17, 2022 at 11:56 AM Hanna Reitz <hreitz@redhat.com> wrote:
On 17.03.22 16:13, John Snow wrote:
On Thu, Mar 17, 2022 at 5:23 AM Hanna Reitz <hreitz@redhat.com> wrote:
On 08.03.22 02:57, John Snow wrote:
This adds an Exception that extends the Python stdlib
subprocess.CalledProcessError.

The difference is that the str() method of this exception also adds the
stdout/stderr logs. In effect, if this exception goes unhandled, Python
will print the output in a visually distinct wrapper to the terminal so
that it's easy to spot in a sea of traceback information.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
    python/qemu/utils/__init__.py | 36 +++++++++++++++++++++++++++++++++++
    1 file changed, 36 insertions(+)

diff --git a/python/qemu/utils/__init__.py b/python/qemu/utils/__init__.py
index 5babf40df2..355ac550bc 100644
--- a/python/qemu/utils/__init__.py
+++ b/python/qemu/utils/__init__.py
@@ -18,6 +18,7 @@
    import os
    import re
    import shutil
+from subprocess import CalledProcessError
    import textwrap
    from typing import Optional

@@ -26,6 +27,7 @@


    __all__ = (
+    'VerboseProcessError',
        'add_visual_margin',
        'get_info_usernet_hostfwd_port',
        'kvm_available',
@@ -121,3 +123,37 @@ def _wrap(line: str) -> str:
            os.linesep.join(_wrap(line) for line in content.splitlines()),
            _bar(None, top=False),
        ))
+
+
+class VerboseProcessError(CalledProcessError):
+    """
+    The same as CalledProcessError, but more verbose.
+
+    This is useful for debugging failed calls during test executions.
+    The return code, signal (if any), and terminal output will be displayed
+    on unhandled exceptions.
+    """
+    def summary(self) -> str:
+        """Return the normal CalledProcessError str() output."""
+        return super().__str__()
+
+    def __str__(self) -> str:
+        lmargin = '  '
+        width = -len(lmargin)
+        sections = []
+
+        name = 'output' if self.stderr is None else 'stdout'
+        if self.stdout:
+            sections.append(add_visual_margin(self.stdout, width, name))
+        else:
+            sections.append(f"{name}: N/A")
+
+        if self.stderr:
+            sections.append(add_visual_margin(self.stderr, width, 'stderr'))
+        elif self.stderr is not None:
What exactly is this condition for?  I would’ve understood if it was
`self.stdout` (because the stdout section then is called just “output”,
so it would make sense to omit the stderr block), but this way it looks
like we’ll only go here if `self.stderr` is an empty string (which
doesn’t make much sense to me, and I don’t understand why we wouldn’t
have the same in the `self.stdout` part above).

(tl;dr: should this be `self.stdout`?)

Hanna

if self.stderr is None, it means that the IO streams were combined. If
it is merely empty, it means there wasn't any stderr output.
Might warrant a comment? :)
How 'bout:

has_combined_output = self.stderr is None

That would be better, although I’m not quite sure I’d immediately know what this means.  Something like “Does self.stdout contain both stdout and stderr?” above it would clear my potential and/or assumed confusion, I believe.

Hanna




reply via email to

[Prev in Thread] Current Thread [Next in Thread]