From 4adc1838e4a540895fabe40e66e01fa18035af1c Mon Sep 17 00:00:00 2001
From: Joe Gordon <jogo@pinterest.com>
Date: Thu, 15 Sep 2022 17:33:59 -0700
Subject: [PATCH] Reuse subunit_trace output logic for load/run

Previously `stestr run` and subunit_trace used slightly different
output and validation logic. run wouldn't catch the case where a test
exited without a status, while code using `last` and `history` would
since they used subunit_trace.

Refactor the subunit_trace output and validation logic and use it for
`load`/`run`.

Converge on the subunit_trace formatted output, in addition to the
summary also show if something goes wrong.

The primary motivation for this change is to better detect when a
subprocess is killed and not all tests are run as expected (segfault,
OOM etc.).
---
 stestr/commands/load.py | 17 ++++++++---------
 stestr/subunit_trace.py | 28 +++++++++++++++-------------
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/stestr/commands/load.py b/stestr/commands/load.py
index 3e26b99..b07fa91 100644
--- a/stestr/commands/load.py
+++ b/stestr/commands/load.py
@@ -278,16 +278,15 @@ def _load_case(inserter, repo, case, subunit_out, pretty_out,
             for test in subunit_trace.RESULTS[worker]:
                 if not test['timestamps'][0] or not test['timestamps'][1]:
                     continue
-                start_times.append(test['timestamps'][0])
-                stop_times.append(test['timestamps'][1])
-        if not start_times or not stop_times:
-            sys.stderr.write("\nNo tests were successful during the run")
+                start_times.append(test["timestamps"][0])
+                stop_times.append(test["timestamps"][1])
+
+        # This is not ideal, as it means if you use don't enter this if statement
+        # some errors aren't caught
+        if subunit_trace.print_full_output(
+            stdout, start_times, stop_times, post_fails=True, no_summary=False
+        ):
             return 1
-        start_time = min(start_times)
-        stop_time = max(stop_times)
-        elapsed_time = stop_time - start_time
-        subunit_trace.print_fails(stdout)
-        subunit_trace.print_summary(stdout, elapsed_time)
     if not results.wasSuccessful(summary_result):
         return 1
     else:
diff --git a/stestr/subunit_trace.py b/stestr/subunit_trace.py
index a89ea11..8856285 100644
--- a/stestr/subunit_trace.py
+++ b/stestr/subunit_trace.py
@@ -242,11 +242,7 @@ def show_outcome(stream, test, print_failures=False, failonly=False,
 
 
 def print_fails(stream):
-    """Print summary failure report.
-
-    Currently unused, however there remains debate on inline vs. at end
-    reporting, so leave the utility function for later use.
-    """
+    """Print summary failure report."""
     if not FAILS:
         return
     stream.write("\n==============================\n")
@@ -403,18 +399,25 @@ def trace(stdin, stdout, print_failures=False, failonly=False,
             x['timestamps'][0] for x in RESULTS[worker] if
             x['timestamps'][0] is not None]
         stop_times += [
-            x['timestamps'][1] for x in RESULTS[worker] if
-            x['timestamps'][1] is not None]
-    if not start_times:
+            x["timestamps"][1]
+            for x in RESULTS[worker]
+            if x["timestamps"][1] is not None
+        ]
+    if print_full_output(stdout, start_times, stop_times, post_fails, no_summary):
+        return 1
+    return 0 if results.wasSuccessful(summary) else 1
+
+
+def print_full_output(stdout, start_times, stop_times, post_fails, no_summary):
+    """Print output plus edge case validation"""
+    if not start_times or count_tests("status", ".*") == 0:
         print("The test run didn't actually run any tests", file=sys.stderr)
         return 1
+
     start_time = min(start_times)
     stop_time = max(stop_times)
     elapsed_time = stop_time - start_time
 
-    if count_tests('status', '.*') == 0:
-        print("The test run didn't actually run any tests", file=sys.stderr)
-        return 1
     if post_fails:
         print_fails(stdout)
     if not no_summary:
@@ -422,7 +425,7 @@ def trace(stdin, stdout, print_failures=False, failonly=False,
 
     # NOTE(mtreinish): Ideally this should live in testtools streamSummary
     # this is just in place until the behavior lands there (if it ever does)
-    if count_tests('status', '^success$') == 0:
+    if count_tests("status", "^xfail$") + count_tests("status", "^success$") == 0:
         print("\nNo tests were successful during the run", file=sys.stderr)
         return 1
     in_progress = get_stuck_in_progress()
@@ -432,7 +435,6 @@ def trace(stdin, stdout, print_failures=False, failonly=False,
         for test in in_progress:
             print("\n\t* %s" % test, file=sys.stderr)
         return 1
-    return 0 if results.wasSuccessful(summary) else 1
 
 
 def main():