Some tweaks to testing framework.

Most interesting is an "api change" - the test methods test.must_exist()
and test.must_exist_one_of() now take an optional 'message' keyword
argument which is passed on to fail_test() if the test fails.

The regex used to test an exception is now working for Python 3.13,
and enabled conditionally - the "enhanced error reporting" changed, in
a way that made it easy to reuse the existing regex (if somebody wants
to take a shot at unifying them, more power!).

Also one unexpected issue was found - one of the check routines does
"output = os.newline.join(output)", but there is no os.newline.  Could use
os.linesep, but just changed it to the Python newline character.

Some annotations added, and some cleanup done on possibly unsafe uses
- mainly that self.stderr() and selt.stdout() *can* return None, but
several places in the code just did string operations on the return
unconditionally. There's already precendent- other places did do a check
before using, so just extended the concept to possibly vulnerable palces.

Signed-off-by: Mats Wichmann <mats@linux.com>

5 files changed, 337 insertions, 206 deletions

diff --git a/testing/framework/TestCmd.py b/testing/framework/TestCmd.py
index 616297a..cada7be 100644
--- a/testing/framework/TestCmd.py
+++ b/testing/framework/TestCmd.py
@@ -1,3 +1,22 @@
+# Copyright 2000-2024 Steven Knight
+#
+# This module is free software, and you may redistribute it and/or modify
+# it under the same terms as Python itself, so long as this copyright message
+# and disclaimer are retained in their original form.
+#
+# IN NO EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT,
+# SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OF
+# THIS CODE, EVEN IF THE AUTHOR HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
+# DAMAGE.
+#
+# THE AUTHOR SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
+# AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
+# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+#
+# Python License: https://docs.python.org/3/license.html#psf-license
+
 """
 A testing framework for commands and scripts.
 
@@ -276,22 +295,6 @@ version.
     TestCmd.where_is('foo', 'PATH1;PATH2', '.suffix3;.suffix4')
 """
 
-# Copyright 2000-2010 Steven Knight
-# This module is free software, and you may redistribute it and/or modify
-# it under the same terms as Python itself, so long as this copyright message
-# and disclaimer are retained in their original form.
-#
-# IN NO EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT,
-# SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OF
-# THIS CODE, EVEN IF THE AUTHOR HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
-# DAMAGE.
-#
-# THE AUTHOR SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
-# PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
-# AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
-# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
-
 __author__ = "Steven Knight <knight at baldmt dot com>"
 __revision__ = "TestCmd.py 1.3.D001 2010/06/03 12:58:27 knight"
 __version__ = "1.3"
@@ -320,7 +323,7 @@ import traceback
 from collections import UserList, UserString
 from pathlib import Path
 from subprocess import PIPE, STDOUT
-from typing import Optional
+from typing import Callable, Dict, Optional, Union
 
 IS_WINDOWS = sys.platform == 'win32'
 IS_MACOS = sys.platform == 'darwin'
@@ -427,7 +430,13 @@ def clean_up_ninja_daemon(self, result_type) -> None:
                 shutil.rmtree(daemon_dir)
 
 
-def fail_test(self=None, condition: bool=True, function=None, skip: int=0, message=None) -> None:
+def fail_test(
+    self=None,
+    condition: bool = True,
+    function: Optional[Callable] = None,
+    skip: int = 0,
+    message: str = "",
+) -> None:
     """Causes a test to exit with a fail.
 
     Reports that the test FAILED and exits with a status of 1, unless
@@ -1047,7 +1056,7 @@ class TestCmd:
         self.verbose_set(verbose)
         self.combine = combine
         self.universal_newlines = universal_newlines
-        self.process = None
+        self.process: Optional[Popen] = None
         # Two layers of timeout: one at the test class instance level,
         # one set on an individual start() call (usually via a run() call)
         self.timeout = timeout
@@ -1055,29 +1064,25 @@ class TestCmd:
         self.set_match_function(match, match_stdout, match_stderr)
         self.set_diff_function(diff, diff_stdout, diff_stderr)
         self._dirlist = []
-        self._preserve = {'pass_test': 0, 'fail_test': 0, 'no_result': 0}
+        self._preserve: Dict[str, Union[str, bool]] = {
+            'pass_test': False,
+            'fail_test': False,
+            'no_result': False,
+        }
         preserve_value = os.environ.get('PRESERVE', False)
         if preserve_value not in [0, '0', 'False']:
             self._preserve['pass_test'] = os.environ['PRESERVE']
             self._preserve['fail_test'] = os.environ['PRESERVE']
             self._preserve['no_result'] = os.environ['PRESERVE']
         else:
-            try:
-                self._preserve['pass_test'] = os.environ['PRESERVE_PASS']
-            except KeyError:
-                pass
-            try:
-                self._preserve['fail_test'] = os.environ['PRESERVE_FAIL']
-            except KeyError:
-                pass
-            try:
-                self._preserve['no_result'] = os.environ['PRESERVE_NO_RESULT']
-            except KeyError:
-                pass
+            self._preserve['pass_test'] = os.environ.get('PRESERVE_PASS', False)
+            self._preserve['fail_test'] = os.environ.get('PRESERVE_FAIL', False)
+            self._preserve['no_result'] = os.environ.get('PRESERVE_NO_RESULT', False)
         self._stdout = []
         self._stderr = []
-        self.status = None
+        self.status: Optional[int] = None
         self.condition = 'no_result'
+        self.workdir: Optional[str]
         self.workdir_set(workdir)
         self.subdir(subdir)
 
@@ -1144,8 +1149,8 @@ class TestCmd:
             list = self._dirlist[:]
             list.reverse()
             for dir in list:
-                self.writable(dir, 1)
-                shutil.rmtree(dir, ignore_errors=1)
+                self.writable(dir, True)
+                shutil.rmtree(dir, ignore_errors=True)
             self._dirlist = []
 
             global _Cleanup
@@ -1242,16 +1247,24 @@ class TestCmd:
 
     unified_diff = staticmethod(difflib.unified_diff)
 
-    def fail_test(self, condition: bool=True, function=None, skip: int=0, message=None) -> None:
+    def fail_test(
+        self,
+        condition: bool = True,
+        function: Optional[Callable] = None,
+        skip: int = 0,
+        message: str = "",
+    )-> None:
         """Cause the test to fail."""
         if not condition:
             return
         self.condition = 'fail_test'
-        fail_test(self=self,
-                  condition=condition,
-                  function=function,
-                  skip=skip,
-                  message=message)
+        fail_test(
+            self=self,
+            condition=condition,
+            function=function,
+            skip=skip,
+            message=message
+        )
 
     def interpreter_set(self, interpreter) -> None:
         """Set the program to be used to interpret the program
@@ -1338,7 +1351,7 @@ class TestCmd:
         if not conditions:
             conditions = ('pass_test', 'fail_test', 'no_result')
         for cond in conditions:
-            self._preserve[cond] = 1
+            self._preserve[cond] = True
 
     def program_set(self, program) -> None:
         """Sets the executable program or script to be tested."""
@@ -1701,12 +1714,12 @@ class TestCmd:
         if self.verbose >= 2:
             write = sys.stdout.write
             write('============ STATUS: %d\n' % self.status)
-            out = self.stdout()
+            out = self.stdout() or ""
             if out or self.verbose >= 3:
                 write(f'============ BEGIN STDOUT (len={len(out)}):\n')
                 write(out)
                 write('============ END STDOUT\n')
-            err = self.stderr()
+            err = self.stderr() or ""
             if err or self.verbose >= 3:
                 write(f'============ BEGIN STDERR (len={len(err)})\n')
                 write(err)
@@ -1985,7 +1998,7 @@ class TestCmd:
             # in the tree bottom-up, lest disabling read permission from
             # the top down get in the way of being able to get at lower
             # parts of the tree.
-            for dirpath, dirnames, filenames in os.walk(top, topdown=0):
+            for dirpath, dirnames, filenames in os.walk(top, topdown=False):
                 for name in dirnames + filenames:
                     do_chmod(os.path.join(dirpath, name))
             do_chmod(top)
@@ -2036,7 +2049,7 @@ class TestCmd:
             do_chmod(top)
         else:
             do_chmod(top)
-            for dirpath, dirnames, filenames in os.walk(top, topdown=0):
+            for dirpath, dirnames, filenames in os.walk(top, topdown=False):
                 for name in dirnames + filenames:
                     do_chmod(os.path.join(dirpath, name))
 
@@ -2090,7 +2103,7 @@ class TestCmd:
             # in the tree bottom-up, lest disabling execute permission from
             # the top down get in the way of being able to get at lower
             # parts of the tree.
-            for dirpath, dirnames, filenames in os.walk(top, topdown=0):
+            for dirpath, dirnames, filenames in os.walk(top, topdown=False):
                 for name in dirnames + filenames:
                     do_chmod(os.path.join(dirpath, name))
             do_chmod(top)
diff --git a/testing/framework/TestCmdTests.py b/testing/framework/TestCmdTests.py
index a2d941d..61c0c5d 100644
--- a/testing/framework/TestCmdTests.py
+++ b/testing/framework/TestCmdTests.py
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 #
 # Copyright 2000-2010 Steven Knight
+#
 # This module is free software, and you may redistribute it and/or modify
 # it under the same terms as Python itself, so long as this copyright message
 # and disclaimer are retained in their original form.
@@ -15,6 +16,8 @@
 # PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
 # AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+#
+# Python License: https://docs.python.org/3/license.html#psf-license
 
 """
 Unit tests for the TestCmd.py module.
diff --git a/testing/framework/TestCommon.py b/testing/framework/TestCommon.py
index 993dc02..f5bb084 100644
--- a/testing/framework/TestCommon.py
+++ b/testing/framework/TestCommon.py
@@ -1,3 +1,22 @@
+# Copyright 2000-2010 Steven Knight
+#
+# This module is free software, and you may redistribute it and/or modify
+# it under the same terms as Python itself, so long as this copyright message
+# and disclaimer are retained in their original form.
+#
+# IN NO EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT,
+# SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OF
+# THIS CODE, EVEN IF THE AUTHOR HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
+# DAMAGE.
+#
+# THE AUTHOR SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
+# AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
+# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+#
+# Python License: https://docs.python.org/3/license.html#psf-license
+
 """
 A testing framework for commands and scripts with commonly useful error handling
 
@@ -39,26 +58,37 @@ provided by the TestCommon class:
 
     test.must_contain_all_lines(output, lines, ['title', find])
 
+    test.must_contain_single_instance_of(output, lines, ['title'])
+
     test.must_contain_any_line(output, lines, ['title', find])
 
     test.must_contain_exactly_lines(output, lines, ['title', find])
 
-    test.must_exist('file1', ['file2', ...])
+    test.must_exist('file1', ['file2', ...], ['message'])
+
+    test.must_exist_one_of(files, ['message'])
 
     test.must_match('file', "expected contents\n")
 
+    test.must_match_file(file, golden_file, ['message', 'newline'])
+
     test.must_not_be_writable('file1', ['file2', ...])
 
     test.must_not_contain('file', 'banned text\n')
 
     test.must_not_contain_any_line(output, lines, ['title', find])
 
+    test.must_not_contain_lines(lines, output, ['title', find]):
+
     test.must_not_exist('file1', ['file2', ...])
 
+    test.must_not_exist_any_of(files)
+
     test.must_not_be_empty('file')
 
     test.run(
         options="options to be prepended to arguments",
+        arguments="string or list of arguments",
         stdout="expected standard output from the program",
         stderr="expected error output from the program",
         status=expected_status,
@@ -80,22 +110,6 @@ The TestCommon module also provides the following variables
 
 """
 
-# Copyright 2000-2010 Steven Knight
-# This module is free software, and you may redistribute it and/or modify
-# it under the same terms as Python itself, so long as this copyright message
-# and disclaimer are retained in their original form.
-#
-# IN NO EVENT SHALL THE AUTHOR BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT,
-# SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OF
-# THIS CODE, EVEN IF THE AUTHOR HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
-# DAMAGE.
-#
-# THE AUTHOR SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT
-# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
-# PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
-# AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
-# SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
-
 __author__ = "Steven Knight <knight at baldmt dot com>"
 __revision__ = "TestCommon.py 1.3.D001 2010/06/03 12:58:27 knight"
 __version__ = "1.3"
@@ -107,6 +121,7 @@ import sys
 import sysconfig
 
 from collections import UserList
+from typing import Callable, List, Optional, Union
 
 from TestCmd import *
 from TestCmd import __all__
@@ -211,15 +226,14 @@ def separate_files(flist):
             missing.append(f)
     return existing, missing
 
-def contains(seq, subseq, find) -> bool:
-    # Returns True or False.
+def contains(seq, subseq, find: Optional[Callable] = None) -> bool:
     if find is None:
         return subseq in seq
     else:
         f = find(seq, subseq)
         return f not in (None, -1) and f is not False
 
-def find_index(seq, subseq, find):
+def find_index(seq, subseq, find: Optional[Callable] = None) -> Optional[int]:
     # Returns either an index of the subseq within the seq, or None.
     # Accepts a function find(seq, subseq), which returns an integer on success
     # and either: None, False, or -1, on failure.
@@ -264,8 +278,14 @@ class TestCommon(TestCmd):
         super().__init__(**kw)
         os.chdir(self.workdir)
 
-    def options_arguments(self, options, arguments):
+    def options_arguments(
+        self,
+        options: Union[str, List[str]],
+        arguments: Union[str, List[str]],
+    ):
         """Merges the "options" keyword argument with the arguments."""
+        # TODO: this *doesn't* split unless both are non-empty strings.
+        #    Did we want to always split strings?
         if options:
             if arguments is None:
                 return options
@@ -282,14 +302,15 @@ class TestCommon(TestCmd):
         return arguments
 
     def must_be_writable(self, *files) -> None:
-        """Ensures that the specified file(s) exist and are writable.
+        """Ensure that the specified file(s) exist and are writable.
+
         An individual file can be specified as a list of directory names,
         in which case the pathname will be constructed by concatenating
         them.  Exits FAILED if any of the files does not exist or is
         not writable.
         """
-        files = [is_List(x) and os.path.join(*x) or x for x in files]
-        existing, missing = separate_files(files)
+        flist = [is_List(x) and os.path.join(*x) or x for x in files]
+        existing, missing = separate_files(flist)
         unwritable = [x for x in existing if not is_writable(x)]
         if missing:
             print("Missing files: `%s'" % "', `".join(missing))
@@ -297,17 +318,23 @@ class TestCommon(TestCmd):
             print("Unwritable files: `%s'" % "', `".join(unwritable))
         self.fail_test(missing + unwritable)
 
-    def must_contain(self, file, required, mode: str='rb', find=None) -> None:
+    def must_contain(
+        self,
+        file: str,
+        required: str,
+        mode: str = 'rb',
+        find: Optional[Callable] = None,
+    ) -> None:
         """Ensures specified file contains the required text.
 
         Args:
-            file (string): name of file to search in.
+            file: name of file to search in.
             required (string): text to search for. For the default
               find function, type must match the return type from
               reading the file; current implementation will convert.
-            mode (string): file open mode.
-            find (func): optional custom search routine. Must take the
-              form "find(output, line)" non-negative integer on success
+            mode: file open mode.
+            find: optional custom search routine. Must take the
+              form ``find(output, line)``, returning non-negative integer on success
               and None, False, or -1, on failure.
 
         Calling test exits FAILED if search result is false
@@ -326,7 +353,7 @@ class TestCommon(TestCmd):
             print(file_contents)
             self.fail_test()
 
-    def must_contain_all(self, output, input, title=None, find=None) -> None:
+    def must_contain_all(self, output, input, title: str = "", find: Optional[Callable] = None)-> None:
         """Ensures that the specified output string (first argument)
         contains all of the specified input as a block (second argument).
 
@@ -338,10 +365,10 @@ class TestCommon(TestCmd):
         for lines in the output.
         """
         if is_List(output):
-            output = os.newline.join(output)
+            output = '\n'.join(output)
 
         if not contains(output, input, find):
-            if title is None:
+            if not title:
                 title = 'output'
             print(f'Missing expected input from {title}:')
             print(input)
@@ -349,7 +376,7 @@ class TestCommon(TestCmd):
             print(output)
             self.fail_test()
 
-    def must_contain_all_lines(self, output, lines, title=None, find=None) -> None:
+    def must_contain_all_lines(self, output, lines, title: str = "", find: Optional[Callable] = None) -> None:
         """Ensures that the specified output string (first argument)
         contains all of the specified lines (second argument).
 
@@ -365,7 +392,7 @@ class TestCommon(TestCmd):
 
         missing = [line for line in lines if not contains(output, line, find)]
         if missing:
-            if title is None:
+            if not title:
                 title = 'output'
             sys.stdout.write(f"Missing expected lines from {title}:\n")
             for line in missing:
@@ -374,13 +401,12 @@ class TestCommon(TestCmd):
             sys.stdout.write(output)
             self.fail_test()
 
-    def must_contain_single_instance_of(self, output, lines, title=None) -> None:
+    def must_contain_single_instance_of(self, output, lines, title: str = "") -> None:
         """Ensures that the specified output string (first argument)
         contains one instance of the specified lines (second argument).
 
         An optional third argument can be used to describe the type
         of output being searched, and only shows up in failure output.
-
         """
         if is_List(output):
             output = '\n'.join(output)
@@ -392,7 +418,7 @@ class TestCommon(TestCmd):
                 counts[line] = count
 
         if counts:
-            if title is None:
+            if not title:
                 title = 'output'
             sys.stdout.write(f"Unexpected number of lines from {title}:\n")
             for line in counts:
@@ -401,7 +427,7 @@ class TestCommon(TestCmd):
             sys.stdout.write(output)
             self.fail_test()
 
-    def must_contain_any_line(self, output, lines, title=None, find=None) -> None:
+    def must_contain_any_line(self, output, lines, title: str = "", find: Optional[Callable] = None) -> None:
         """Ensures that the specified output string (first argument)
         contains at least one of the specified lines (second argument).
 
@@ -416,7 +442,7 @@ class TestCommon(TestCmd):
             if contains(output, line, find):
                 return
 
-        if title is None:
+        if not title:
             title = 'output'
         sys.stdout.write(f"Missing any expected line from {title}:\n")
         for line in lines:
@@ -425,7 +451,7 @@ class TestCommon(TestCmd):
         sys.stdout.write(output)
         self.fail_test()
 
-    def must_contain_exactly_lines(self, output, expect, title=None, find=None) -> None:
+    def must_contain_exactly_lines(self, output, expect, title: str = "", find: Optional[Callable] = None) -> None:
         """Ensures that the specified output string (first argument)
         contains all of the lines in the expected string (second argument)
         with none left over.
@@ -458,7 +484,7 @@ class TestCommon(TestCmd):
             # all lines were matched
             return
 
-        if title is None:
+        if not title:
             title = 'output'
         if missing:
             sys.stdout.write(f"Missing expected lines from {title}:\n")
@@ -473,23 +499,23 @@ class TestCommon(TestCmd):
         sys.stdout.flush()
         self.fail_test()
 
-    def must_contain_lines(self, lines, output, title=None, find = None):
+    def must_contain_lines(self, lines, output, title: str = "", find: Optional[Callable] = None) -> None:
         # Deprecated; retain for backwards compatibility.
-        return self.must_contain_all_lines(output, lines, title, find)
+        self.must_contain_all_lines(output, lines, title, find)
 
-    def must_exist(self, *files) -> None:
+    def must_exist(self, *files, message: str = "") -> None:
         """Ensures that the specified file(s) must exist.  An individual
         file be specified as a list of directory names, in which case the
         pathname will be constructed by concatenating them.  Exits FAILED
         if any of the files does not exist.
         """
-        files = [is_List(x) and os.path.join(*x) or x for x in files]
-        missing = [x for x in files if not os.path.exists(x) and not os.path.islink(x) ]
+        flist = [is_List(x) and os.path.join(*x) or x for x in files]
+        missing = [x for x in flist if not os.path.exists(x) and not os.path.islink(x)]
         if missing:
             print("Missing files: `%s'" % "', `".join(missing))
-            self.fail_test(missing)
+            self.fail_test(bool(missing), message=message)
 
-    def must_exist_one_of(self, files) -> None:
+    def must_exist_one_of(self, files, message: str = "") -> None:
         """Ensures that at least one of the specified file(s) exists.
         The filenames can be given as a list, where each entry may be
         a single path string, or a tuple of folder names and the final
@@ -507,9 +533,17 @@ class TestCommon(TestCmd):
                 return
             missing.append(xpath)
         print("Missing one of: `%s'" % "', `".join(missing))
-        self.fail_test(missing)
-
-    def must_match(self, file, expect, mode: str = 'rb', match=None, message=None, newline=None):
+        self.fail_test(bool(missing), message=message)
+
+    def must_match(
+        self,
+        file,
+        expect,
+        mode: str = 'rb',
+        match: Optional[Callable] = None,
+        message: str = "",
+        newline=None,
+    ):
         """Matches the contents of the specified file (first argument)
         against the expected contents (second argument).  The expected
         contents are a list of lines or a string which will be split
@@ -519,7 +553,10 @@ class TestCommon(TestCmd):
         if not match:
             match = self.match
         try:
-            self.fail_test(not match(to_str(file_contents), to_str(expect)), message=message)
+            self.fail_test(
+                not match(to_str(file_contents), to_str(expect)),
+                message=message,
+            )
         except KeyboardInterrupt:
             raise
         except:
@@ -527,7 +564,15 @@ class TestCommon(TestCmd):
             self.diff(expect, file_contents, 'contents ')
             raise
 
-    def must_match_file(self, file, golden_file, mode: str='rb', match=None, message=None, newline=None):
+    def must_match_file(
+        self,
+        file,
+        golden_file,
+        mode: str = 'rb',
+        match: Optional[Callable] = None,
+        message: str = "",
+        newline=None,
+    ) -> None:
         """Matches the contents of the specified file (first argument)
         against the expected contents (second argument).  The expected
         contents are a list of lines or a string which will be split
@@ -540,7 +585,10 @@ class TestCommon(TestCmd):
             match = self.match
 
         try:
-            self.fail_test(not match(to_str(file_contents), to_str(golden_file_contents)), message=message)
+            self.fail_test(
+                not match(to_str(file_contents), to_str(golden_file_contents)),
+                message=message,
+            )
         except KeyboardInterrupt:
             raise
         except:
@@ -561,7 +609,7 @@ class TestCommon(TestCmd):
             print(file_contents)
             self.fail_test()
 
-    def must_not_contain_any_line(self, output, lines, title=None, find=None) -> None:
+    def must_not_contain_any_line(self, output, lines, title: str = "", find: Optional[Callable] = None) -> None:
         """Ensures that the specified output string (first argument)
         does not contain any of the specified lines (second argument).
 
@@ -578,7 +626,7 @@ class TestCommon(TestCmd):
                 unexpected.append(line)
 
         if unexpected:
-            if title is None:
+            if not title:
                 title = 'output'
             sys.stdout.write(f"Unexpected lines in {title}:\n")
             for line in unexpected:
@@ -587,8 +635,8 @@ class TestCommon(TestCmd):
             sys.stdout.write(output)
             self.fail_test()
 
-    def must_not_contain_lines(self, lines, output, title=None, find=None):
-        return self.must_not_contain_any_line(output, lines, title, find)
+    def must_not_contain_lines(self, lines, output, title: str = "", find: Optional[Callable] = None) -> None:
+        self.must_not_contain_any_line(output, lines, title, find)
 
     def must_not_exist(self, *files) -> None:
         """Ensures that the specified file(s) must not exist.
@@ -596,11 +644,11 @@ class TestCommon(TestCmd):
         which case the pathname will be constructed by concatenating them.
         Exits FAILED if any of the files exists.
         """
-        files = [is_List(x) and os.path.join(*x) or x for x in files]
-        existing = [x for x in files if os.path.exists(x) or os.path.islink(x)]
+        flist = [is_List(x) and os.path.join(*x) or x for x in files]
+        existing = [x for x in flist if os.path.exists(x) or os.path.islink(x)]
         if existing:
             print("Unexpected files exist: `%s'" % "', `".join(existing))
-            self.fail_test(existing)
+            self.fail_test(bool(existing))
 
     def must_not_exist_any_of(self, files) -> None:
         """Ensures that none of the specified file(s) exists.
@@ -620,7 +668,7 @@ class TestCommon(TestCmd):
                 existing.append(xpath)
         if existing:
             print("Unexpected files exist: `%s'" % "', `".join(existing))
-            self.fail_test(existing)
+            self.fail_test(bool(existing))
 
     def must_not_be_empty(self, file) -> None:
         """Ensures that the specified file exists, and that it is not empty.
@@ -646,8 +694,8 @@ class TestCommon(TestCmd):
         them.  Exits FAILED if any of the files does not exist or is
         writable.
         """
-        files = [is_List(x) and os.path.join(*x) or x for x in files]
-        existing, missing = separate_files(files)
+        flist = [is_List(x) and os.path.join(*x) or x for x in files]
+        existing, missing = separate_files(flist)
         writable = [file for file in existing if is_writable(file)]
         if missing:
             print("Missing files: `%s'" % "', `".join(missing))
@@ -716,50 +764,63 @@ class TestCommon(TestCmd):
             sys.stderr.write(f'Exception trying to execute: {cmd_args}\n')
             raise e
 
-    def finish(self, popen, stdout = None, stderr: str = '', status: int = 0, **kw) -> None:
-        """
-        Finishes and waits for the process being run under control of
-        the specified popen argument.  Additional arguments are similar
-        to those of the run() method:
 
-                stdout  The expected standard output from
-                        the command.  A value of None means
-                        don't test standard output.
+    def finish(
+        self,
+        popen,
+        stdout: Optional[str] = None,
+        stderr: Optional[str] = '',
+        status: Optional[int] = 0,
+        **kw,
+    ) -> None:
+        """Finish and wait for the process being run.
 
-                stderr  The expected error output from
-                        the command.  A value of None means
-                        don't test error output.
+        The *popen* argument describes a ``Popen`` object controlling
+        the process.
 
-                status  The expected exit status from the
-                        command.  A value of None means don't
-                        test exit status.
+        The default behavior is to expect a successful exit, to not test
+        standard output, and to expect error output to be empty.
+
+        Only arguments extending :meth:`TestCmd.finish` are shown.
+
+        Args:
+            stdout:  The expected standard output from the command.
+               A value of ``None`` means don't test standard output.
+            stderr:  The expected error output from the command.
+               A value of ``None`` means don't test error output.
+            status:  The expected exit status from the command.
+               A value of ``None`` means don't test exit status.
         """
         super().finish(popen, **kw)
         match = kw.get('match', self.match)
-        self._complete(self.stdout(), stdout,
-                       self.stderr(), stderr, status, match)
-
-    def run(self, options = None, arguments = None,
-                  stdout = None, stderr: str = '', status: int = 0, **kw) -> None:
+        self._complete(self.stdout(), stdout, self.stderr(), stderr, status, match)
+
+
+    def run(
+        self,
+        options=None,
+        arguments=None,
+        stdout: Optional[str] = None,
+        stderr: Optional[str] = '',
+        status: Optional[int] = 0,
+        **kw,
+    ) -> None:
         """Runs the program under test, checking that the test succeeded.
 
-        The parameters are the same as the base TestCmd.run() method,
-        with the addition of:
+        The default behavior is to expect a successful exit, not test
+        standard output, and expects error output to be empty.
 
-                options Extra options that get prepended to the beginning
-                        of the arguments.
+        Only arguments extending :meth:`TestCmd.run` are shown.
 
-                stdout  The expected standard output from
-                        the command.  A value of None means
-                        don't test standard output.
-
-                stderr  The expected error output from
-                        the command.  A value of None means
-                        don't test error output.
-
-                status  The expected exit status from the
-                        command.  A value of None means don't
-                        test exit status.
+        Args:
+            options: Extra options that get prepended to the beginning
+               of the arguments.
+            stdout:  The expected standard output from the command.
+               A value of ``None`` means don't test standard output.
+            stderr:  The expected error output from the command.
+               A value of ``None`` means don't test error output.
+            status:  The expected exit status from the command.
+               A value of ``None`` means don't test exit status.
 
         By default, this expects a successful exit (status = 0), does
         not test standard output (stdout = None), and expects that error
@@ -767,8 +828,7 @@ class TestCommon(TestCmd):
         """
         kw['arguments'] = self.options_arguments(options, arguments)
         try:
-            match = kw['match']
-            del kw['match']
+            match = kw.pop('match')
         except KeyError:
             match = self.match
         super().run(**kw)
diff --git a/testing/framework/TestCommonTests.py b/testing/framework/TestCommonTests.py
index 014a018..3f7d3fd 100644
--- a/testing/framework/TestCommonTests.py
+++ b/testing/framework/TestCommonTests.py
@@ -1,9 +1,7 @@
 #!/usr/bin/env python
-"""
-Unit tests for the TestCommon.py module.
-"""
-
+#
 # Copyright 2000-2010 Steven Knight
+#
 # This module is free software, and you may redistribute it and/or modify
 # it under the same terms as Python itself, so long as this copyright message
 # and disclaimer are retained in their original form.
@@ -18,6 +16,12 @@ Unit tests for the TestCommon.py module.
 # PARTICULAR PURPOSE.  THE CODE PROVIDED HEREUNDER IS ON AN "AS IS" BASIS,
 # AND THERE IS NO OBLIGATION WHATSOEVER TO PROVIDE MAINTENANCE,
 # SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+#
+# Python License: https://docs.python.org/3/license.html#psf-license
+
+"""
+Unit tests for the TestCommon.py module.
+"""
 
 import os
 import re
@@ -466,6 +470,7 @@ class must_contain_all_lines_TestCase(TestCommonTestCase):
 
         def re_search(output, line):
             return re.compile(line, re.S).search(output)
+
         test.must_contain_all_lines(output, lines, find=re_search)
 
         test.pass_test()
@@ -964,6 +969,22 @@ class must_exist_TestCase(TestCommonTestCase):
         stderr = run_env.stderr()
         assert stderr.find("FAILED") != -1, stderr
 
+    def test_failure_message(self) -> None:
+        """Test must_exist():  failure with extra message"""
+        run_env = self.run_env
+
+        script = lstrip("""\
+        from TestCommon import TestCommon
+        tc = TestCommon(workdir='')
+        tc.must_exist('file1', message="Extra Info")
+        tc.pass_test()
+        """)
+        run_env.run(program=sys.executable, stdin=script)
+        stdout = run_env.stdout()
+        assert stdout == "Missing files: `file1'\n", stdout
+        stderr = run_env.stderr()
+        assert stderr.find("Extra Info") != -1, stderr
+
     def test_file_specified_as_list(self) -> None:
         """Test must_exist():  file specified as list"""
         run_env = self.run_env
@@ -1034,6 +1055,22 @@ class must_exist_one_of_TestCase(TestCommonTestCase):
         stderr = run_env.stderr()
         assert stderr.find("FAILED") != -1, stderr
 
+    def test_failure_message(self) -> None:
+        """Test must_exist_one_of():  failure with extra message"""
+        run_env = self.run_env
+
+        script = lstrip("""\
+        from TestCommon import TestCommon
+        tc = TestCommon(workdir='')
+        tc.must_exist_one_of(['file1'], message="Extra Info")
+        tc.pass_test()
+        """)
+        run_env.run(program=sys.executable, stdin=script)
+        stdout = run_env.stdout()
+        assert stdout == "Missing one of: `file1'\n", stdout
+        stderr = run_env.stderr()
+        assert stderr.find("Extra Info") != -1, stderr
+
     def test_files_specified_as_list(self) -> None:
         """Test must_exist_one_of():  files specified as list"""
         run_env = self.run_env
@@ -1077,8 +1114,7 @@ class must_exist_one_of_TestCase(TestCommonTestCase):
         tc = TestCommon(workdir='')
         tc.subdir('sub')
         tc.write(['sub', 'file1'], "sub/file1\\n")
-        tc.must_exist_one_of(['file2',
-                              ['sub', 'file1']])
+        tc.must_exist_one_of(['file2', ['sub', 'file1']])
         tc.pass_test()
         """)
         run_env.run(program=sys.executable, stdin=script)
@@ -1096,8 +1132,7 @@ class must_exist_one_of_TestCase(TestCommonTestCase):
         tc = TestCommon(workdir='')
         tc.subdir('sub')
         tc.write(['sub', 'file1'], "sub/file1\\n")
-        tc.must_exist_one_of(['file2',
-                              ('sub', 'file1')])
+        tc.must_exist_one_of(['file2', ('sub', 'file1')])
         tc.pass_test()
         """)
         run_env.run(program=sys.executable, stdin=script)
@@ -1943,49 +1978,44 @@ class run_TestCase(TestCommonTestCase):
             fr"""Exception trying to execute: \[{re.escape(repr(sys.executable))}, '[^']*pass'\]
 Traceback \(most recent call last\):
   File "<stdin>", line \d+, in (\?|<module>)
-  File "[^"]+TestCommon.py", line \d+, in run
-    super\(\).run\(\*\*kw\)
-  File "[^"]+TestCmd.py", line \d+, in run
-    p = self.start\(program=program,
-(?:\s*\^*\s)?  File \"[^\"]+TestCommon.py\", line \d+, in start
+  File "[^"]+TestCommon\.py", line \d+, in run
+    super\(\)\.run\(\*\*kw\)
+  File "[^"]+TestCmd\.py", line \d+, in run
+    p = self\.start\(program=program,
+(?:\s*\^*\s)?  File \"[^\"]+TestCommon\.py\", line \d+, in start
     raise e
-  File "[^"]+TestCommon.py", line \d+, in start
-    return super\(\).start\(program, interpreter, arguments,
+  File "[^"]+TestCommon\.py", line \d+, in start
+    return super\(\)\.start\(program, interpreter, arguments,
 (?:\s*\^*\s)?  File \"<stdin>\", line \d+, in raise_exception
 TypeError: forced TypeError
 """)
-        expect_stderr = re.compile(expect_stderr, re.M)
 
-        # TODO: Python 3.13+ expanded error msgs again. This doesn't work yet.
+        # Python 3.13+ expanded error msgs again, not in a way we can easily
+        # accomodate with the other regex.
         expect_enhanced_stderr = lstrip(
             fr"""Exception trying to execute: \[{re.escape(repr(sys.executable))}, '[^']*pass'\]
-Traceback (most recent call last):
+Traceback \(most recent call last\):
   File "<stdin>", line \d+, in (\?|<module>)
-  File "[^"]+TestCommon.py", line \d+, in run
-    super\(\).run\(\*\*kw\)
-    ~~~~~~~~~~~^^^^^^
-  File "[^"]+TestCmd.py", line \d+, in run
-    p = self.start\(program=program,
-        ~~~~~~~~~~^^^^^^^^^^^^^^^^^
+  File "[^"]+TestCommon\.py", line \d+, in run
+    super\(\)\.run\(\*\*kw\)
+(?:\s*[~\^]*\s*)?File "[^"]+TestCmd\.py", line \d+, in run
+    p = self\.start\(program=program,
                    interpreter=interpreter,
-                   ^^^^^^^^^^^^^^^^^^^^^^^^
-    ...<2 lines>...
+    \.\.\.<2 lines>\.\.\.
                    timeout=timeout,
-                   ^^^^^^^^^^^^^^^^
                    stdin=stdin\)
-                   ^^^^^^^^^^^^
-(?:\s*\^*\s)?  File \"[^\"]+TestCommon.py\", line \d+, in start
+(?:\s*[~\^]*\s*)?File \"[^\"]+TestCommon\.py\", line \d+, in start
     raise e
-  File "[^"]+TestCommon.py", line \d+, in start
-    return super\(\).start\(program, interpreter, arguments,
-           ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-                         universal_newlines, \*\*kw\)
-                         ^^^^^^^^^^^^^^^^^^^^^^^^^
-(?:\s*\^*\s)?  File \"<stdin>\", line \d+, in raise_exception
+  File "[^"]+TestCommon\.py", line \d+, in start
+    return super\(\)\.start\(program, interpreter, arguments,
+(?:\s*[~\^]*\s*)?universal_newlines, \*\*kw\)
+(?:\s*[~\^]*\s*)?File \"<stdin>\", line \d+, in raise_exception
 TypeError: forced TypeError
 """)
-        expect_enhanced_stderr = re.compile(expect_enhanced_stderr, re.M)
-
+        if sys.version_info[:2] > (3, 12):
+            expect_stderr = re.compile(expect_enhanced_stderr, re.M)
+        else:
+            expect_stderr = re.compile(expect_stderr, re.M)
         self.run_execution_test(script, expect_stdout, expect_stderr)
 
     def test_ignore_stderr(self) -> None:
diff --git a/testing/framework/TestSCons.py b/testing/framework/TestSCons.py
index 7360466..243be75 100644
--- a/testing/framework/TestSCons.py
+++ b/testing/framework/TestSCons.py
@@ -42,6 +42,7 @@ import time
 import subprocess as sp
 import zipfile
 from collections import namedtuple
+from typing import Optional, Tuple
 
 from TestCommon import *
 from TestCommon import __all__, _python_
@@ -573,8 +574,9 @@ class TestSCons(TestCommon):
                 self.pass_test()
             else:
                 # test failed; have to do this by hand...
+                stdout = self.stdout() or ""
                 print(self.banner('STDOUT '))
-                print(self.stdout())
+                print(stdout)
                 print(self.diff(warning, stderr, 'STDERR '))
                 self.fail_test()
 
@@ -728,8 +730,9 @@ class TestSCons(TestCommon):
             result.extend(sorted(glob.glob(p)))
         return result
 
-    def get_sconsignname(self):
+    def get_sconsignname(self) -> str:
         """Get the scons database name used, and return both the prefix and full filename.
+
         if the user left the options defaulted AND the default algorithm set by
         SCons is md5, then set the database name to be the special default name
 
@@ -739,6 +742,11 @@ class TestSCons(TestCommon):
 
         Returns:
             a pair containing: the current dbname, the dbname.dblite filename
+
+        TODO: docstring is not truthful about returning "both" - but which to fix?
+            Say it returns just basename, or return both?
+        TODO: has no way to account for an ``SConsignFile()`` call which might assign
+            a different dbname. Document that it's only useful for hash testing?
         """
         hash_format = get_hash_format()
         current_hash_algorithm = get_current_hash_algorithm_used()
@@ -749,11 +757,17 @@ class TestSCons(TestCommon):
             return database_prefix
 
 
-    def unlink_sconsignfile(self, name: str='.sconsign.dblite') -> None:
+    def unlink_sconsignfile(self, name: str = '.sconsign.dblite') -> None:
         """Delete the sconsign file.
 
+        Provides a hook to do special things for the sconsign DB,
+        although currently it just calls unlink.
+
         Args:
             name: expected name of sconsign file
+
+        TODO: deal with suffix if :meth:`getsconsignname` does not provide it.
+            How do we know, since multiple formats are allowed?
         """
         return self.unlink(name)
 
@@ -851,7 +865,7 @@ class TestSCons(TestCommon):
             result.append(os.path.join(d, 'linux'))
         return result
 
-    def java_where_java_home(self, version=None) -> str:
+    def java_where_java_home(self, version=None) -> Optional[str]:
         """ Find path to what would be JAVA_HOME.
 
         SCons does not read JAVA_HOME from the environment, so deduce it.
@@ -905,6 +919,7 @@ class TestSCons(TestCommon):
             "Could not run Java: unable to detect valid JAVA_HOME, skipping test.\n",
             from_fw=True,
         )
+        return None
 
     def java_mac_check(self, where_java_bin, java_bin_name) -> None:
         """Extra check for Java on MacOS.
@@ -965,7 +980,7 @@ class TestSCons(TestCommon):
 
         return where_java
 
-    def java_where_javac(self, version=None) -> str:
+    def java_where_javac(self, version=None) -> Tuple[str, str]:
         """ Find java compiler.
 
         Args:
@@ -989,21 +1004,25 @@ class TestSCons(TestCommon):
                  stderr=None,
                  status=None)
         # Note recent versions output version info to stdout instead of stderr
+        stdout = self.stdout() or ""
+        stderr = self.stderr() or ""
         if version:
             verf = f'javac {version}'
-            if self.stderr().find(verf) == -1 and self.stdout().find(verf) == -1:
+            if stderr.find(verf) == -1 and stdout.find(verf) == -1:
                 fmt = "Could not find javac for Java version %s, skipping test(s).\n"
                 self.skip_test(fmt % version, from_fw=True)
         else:
             version_re = r'javac (\d*\.*\d)'
-            m = re.search(version_re, self.stderr())
+            m = re.search(version_re, stderr)
             if not m:
-                m = re.search(version_re, self.stdout())
+                m = re.search(version_re, stdout)
 
             if m:
                 version = m.group(1)
                 self.javac_is_gcj = False
-            elif self.stderr().find('gcj') != -1:
+                return where_javac, version
+
+            if stderr.find('gcj') != -1:
                 version = '1.2'
                 self.javac_is_gcj = True
             else:
@@ -1167,7 +1186,7 @@ else:
 
     def Qt_create_SConstruct(self, place, qt_tool: str='qt3') -> None:
         if isinstance(place, list):
-            place = test.workpath(*place)
+            place = self.workpath(*place)
 
         var_prefix=qt_tool.upper()
         self.write(place, f"""\
@@ -1366,10 +1385,11 @@ SConscript(sconscript)
 
         if doCheckStdout:
             exp_stdout = self.wrap_stdout(".*", rdstr)
-            if not self.match_re_dotall(self.stdout(), exp_stdout):
+            stdout = self.stdout() or ""
+            if not self.match_re_dotall(stdout, exp_stdout):
                 print("Unexpected stdout: ")
                 print("-----------------------------------------------------")
-                print(repr(self.stdout()))
+                print(repr(stdout))
                 print("-----------------------------------------------------")
                 print(repr(exp_stdout))
                 print("-----------------------------------------------------")
@@ -1522,10 +1542,11 @@ SConscript(sconscript)
 
         if doCheckStdout:
             exp_stdout = self.wrap_stdout(".*", rdstr)
-            if not self.match_re_dotall(self.stdout(), exp_stdout):
+            stdout = self.stdout() or ""
+            if not self.match_re_dotall(stdout, exp_stdout):
                 print("Unexpected stdout: ")
                 print("----Actual-------------------------------------------")
-                print(repr(self.stdout()))
+                print(repr(stdout))
                 print("----Expected-----------------------------------------")
                 print(repr(exp_stdout))
                 print("-----------------------------------------------------")
@@ -1610,7 +1631,8 @@ if os.path.exists(Python_h):
 else:
     print("False")
 """)
-        incpath, libpath, libname, python_h = self.stdout().strip().split('\n')
+        stdout = self.stdout() or ""
+        incpath, libpath, libname, python_h = stdout.strip().split('\n')
         if python_h == "False" and python_h_required:
             self.skip_test('Can not find required "Python.h", skipping test.\n', from_fw=True)
 
@@ -1737,7 +1759,7 @@ class TimeSCons(TestSCons):
         directory containing the executing script to the temporary
         working directory.
         """
-        self.variables = kw.get('variables')
+        self.variables: dict = kw.get('variables')
         default_calibrate_variables = []
         if self.variables is not None:
             for variable, value in self.variables.items():
@@ -1872,8 +1894,9 @@ class TimeSCons(TestSCons):
         # the full and null builds.
         kw['status'] = None
         self.run(*args, **kw)
-        sys.stdout.write(self.stdout())
-        stats = self.collect_stats(self.stdout())
+        stdout = self.stdout() or ""
+        sys.stdout.write(stdout)
+        stats = self.collect_stats(stdout)
         # Delete the time-commands, since no commands are ever
         # executed on the help run and it is (or should be) always 0.0.
         del stats['time-commands']
@@ -1885,8 +1908,9 @@ class TimeSCons(TestSCons):
         """
         self.add_timing_options(kw)
         self.run(*args, **kw)
-        sys.stdout.write(self.stdout())
-        stats = self.collect_stats(self.stdout())
+        stdout = self.stdout() or ""
+        sys.stdout.write(stdout)
+        stats = self.collect_stats(stdout)
         self.report_traces('full', stats)
         self.trace('full-memory', 'initial', **stats['memory-initial'])
         self.trace('full-memory', 'prebuild', **stats['memory-prebuild'])
@@ -1917,8 +1941,9 @@ class TimeSCons(TestSCons):
         # SConscript:/private/var/folders/ng/48pttrpj239fw5rmm3x65pxr0000gn/T/testcmd.12081.pk1bv5i5/SConstruct  took 533.646 ms
         read_str = 'SConscript:.*\n'
         self.up_to_date(arguments='.', read_str=read_str, **kw)
-        sys.stdout.write(self.stdout())
-        stats = self.collect_stats(self.stdout())
+        stdout = self.stdout() or ""
+        sys.stdout.write(stdout)
+        stats = self.collect_stats(stdout)
         # time-commands should always be 0.0 on a null build, because
         # no commands should be executed.  Remove it from the stats
         # so we don't trace it, but only if it *is* 0 so that we'll