Merge #14683: tests: better combine_logs.py behavior

4aabadbf44 tests: have combine_logs default to most recent test dir (James O'Beirne)

Pull request description:

  Have `combine_logs.py` default to the most recent test directory if no argument is provided. This allows you to avoid an annoying copy-paste when iterating on a failing test, since you can do something like
  ```sh
  alias testlogs='./test/functional/combine_logs.py -c | less'

  ./test/functional/some_test.py  # fails
  testlogs
  ```

Tree-SHA512: 919642ab09c314888a23c9491963b35b9da87e60deb740d1d5e816444aa9bdda5e519dc8ca131669f2d563167ef5f5abb14e22f20f47bf8362915ed578181846

2 files changed, 51 insertions, 9 deletions

diff --git a/test/functional/combine_logs.py b/test/functional/combine_logs.py
index 3230d5cb6b..5bb3b5c094 100755
--- a/test/functional/combine_logs.py
+++ b/test/functional/combine_logs.py
@@ -2,7 +2,9 @@
 """Combine logs from multiple bitcoin nodes as well as the test_framework log.
 
 This streams the combined log output to stdout. Use combine_logs.py > outputfile
-to write to an outputfile."""
+to write to an outputfile.
+
+If no argument is provided, the most recent test directory will be used."""
 
 import argparse
 from collections import defaultdict, namedtuple
@@ -11,6 +13,13 @@ import itertools
 import os
 import re
 import sys
+import tempfile
+
+# N.B.: don't import any local modules here - this script must remain executable
+# without the parent module installed.
+
+# Should match same symbol in `test_framework.test_framework`.
+TMPDIR_PREFIX = "bitcoin_func_test_"
 
 # Matches on the date format at the start of the log event
 TIMESTAMP_PATTERN = re.compile(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d{6})?Z")
@@ -19,22 +28,30 @@ LogEvent = namedtuple('LogEvent', ['timestamp', 'source', 'event'])
 
 def main():
     """Main function. Parses args, reads the log files and renders them as text or html."""
-
-    parser = argparse.ArgumentParser(usage='%(prog)s [options] <test temporary directory>', description=__doc__)
+    parser = argparse.ArgumentParser(
+        description=__doc__, formatter_class=argparse.RawTextHelpFormatter)
+    parser.add_argument(
+        'testdir', nargs='?', default='',
+        help=('temporary test directory to combine logs from. '
+              'Defaults to the most recent'))
     parser.add_argument('-c', '--color', dest='color', action='store_true', help='outputs the combined log with events colored by source (requires posix terminal colors. Use less -r for viewing)')
     parser.add_argument('--html', dest='html', action='store_true', help='outputs the combined log as html. Requires jinja2. pip install jinja2')
-    args, unknown_args = parser.parse_known_args()
+    args = parser.parse_args()
 
     if args.html and args.color:
         print("Only one out of --color or --html should be specified")
         sys.exit(1)
 
-    # There should only be one unknown argument - the path of the temporary test directory
-    if len(unknown_args) != 1:
-        print("Unexpected arguments" + str(unknown_args))
+    testdir = args.testdir or find_latest_test_dir()
+
+    if not testdir:
+        print("No test directories found")
         sys.exit(1)
 
-    log_events = read_logs(unknown_args[0])
+    if not args.testdir:
+        print("Opening latest test directory: {}".format(testdir), file=sys.stderr)
+
+    log_events = read_logs(testdir)
 
     print_logs(log_events, color=args.color, html=args.html)
 
@@ -53,6 +70,29 @@ def read_logs(tmp_dir):
 
     return heapq.merge(*[get_log_events(source, f) for source, f in files])
 
+
+def find_latest_test_dir():
+    """Returns the latest tmpfile test directory prefix."""
+    tmpdir = tempfile.gettempdir()
+
+    def join_tmp(basename):
+        return os.path.join(tmpdir, basename)
+
+    def is_valid_test_tmpdir(basename):
+        fullpath = join_tmp(basename)
+        return (
+            os.path.isdir(fullpath)
+            and basename.startswith(TMPDIR_PREFIX)
+            and os.access(fullpath, os.R_OK)
+        )
+
+    testdir_paths = [
+        join_tmp(name) for name in os.listdir(tmpdir) if is_valid_test_tmpdir(name)
+    ]
+
+    return max(testdir_paths, key=os.path.getmtime) if testdir_paths else None
+
+
 def get_log_events(source, logfile):
     """Generator function that returns individual log events.
 
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 44fc185e6d..0dfa9e0d24 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -43,6 +43,8 @@ TEST_EXIT_PASSED = 0
 TEST_EXIT_FAILED = 1
 TEST_EXIT_SKIPPED = 77
 
+TMPDIR_PREFIX = "bitcoin_func_test_"
+
 
 class SkipTest(Exception):
     """This exception is raised to skip a test"""
@@ -151,7 +153,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             self.options.tmpdir = os.path.abspath(self.options.tmpdir)
             os.makedirs(self.options.tmpdir, exist_ok=False)
         else:
-            self.options.tmpdir = tempfile.mkdtemp(prefix="test")
+            self.options.tmpdir = tempfile.mkdtemp(prefix=TMPDIR_PREFIX)
         self._start_logging()
 
         self.log.debug('Setting up network thread')