Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: helper: Print stdout on timeout #163

Merged
merged 1 commit into from
Sep 29, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions test/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,24 @@ def run(command, *, timeout=30):
logger = logger_from_command(command)
logger.info('running: %s', command)

proc = subprocess.run(shlex.split(command), capture_output=True, text=True, check=False,
timeout=timeout)

for line in proc.stdout.splitlines():
if line:
logger.info('stdout: %s', line)
for line in proc.stderr.splitlines():
if line:
logger.warning('stderr: %s', line)
def stdout_print_helper(logger, prefix, stdout):
if stdout is None:
return

for line in stdout.splitlines():
if line:
logger.info(f'{prefix}: %s', line)

try:
proc = subprocess.run(shlex.split(command), capture_output=True, text=True, check=False,
timeout=timeout)
except subprocess.TimeoutExpired as e:
stdout_print_helper(logger, "stdout", e.stdout)
stdout_print_helper(logger, "stderr", e.stderr)
raise
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't stderr then be printed as well?

Might it be an option to assign stoud and stderr to variables and handle printout in a finally clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, printing stderr makes sense. I'm not sure I'm getting the point with finally block. You mean to handle printout in finally instead of except block?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ment combining the output for the good case and the error case, yes.

Something like

diff --git a/test/helper.py b/test/helper.py
index b067d6e..232aa5c 100644
--- a/test/helper.py
+++ b/test/helper.py
@@ -66,15 +66,21 @@ def run(command, *, timeout=30):
     logger = logger_from_command(command)
     logger.info('running: %s', command)
 
-    proc = subprocess.run(shlex.split(command), capture_output=True, text=True, check=False,
-                          timeout=timeout)
-
-    for line in proc.stdout.splitlines():
-        if line:
-            logger.info('stdout: %s', line)
-    for line in proc.stderr.splitlines():
-        if line:
-            logger.warning('stderr: %s', line)
+    try:
+        proc = subprocess.run(shlex.split(command), capture_output=True, text=True, check=False,
+                              timeout=timeout)
+        stdout = proc.stdout
+        stderr = proc.stderr
+    except subprocess.TimeoutExpired as e:
+        stdout = e.stdout
+        stderr = e.stderr
+    finally:
+        for line in stdout.splitlines():
+            if line:
+                logger.info('stdout: %s', line)
+        for line in stderr.splitlines():
+            if line:
+                logger.warning('stderr: %s', line)
 
     logger.info('exitcode: %d', proc.returncode)

But I have no clue how this actually differs from just printing the error itself and this solution also wouldn't allow to easily re-raise the error.

@Bastian-Krause do you have any better idea maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Published slightly modified version of your proposal


stdout_print_helper(logger, "stdout", proc.stdout)
stdout_print_helper(logger, "stderr", proc.stderr)

logger.info('exitcode: %d', proc.returncode)

Expand Down