tests: Modernize test scripts

by using argparse and pathlib and removing Python 2 specific code.

9 files changed, 161 insertions, 196 deletions

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 301959db..559c757a 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -31,7 +31,7 @@ macro(add_ledger_harness_tests _class)
       if ((TestFile_IsPythonTest EQUAL -1) OR HAVE_BOOST_PYTHON)
         add_test(NAME ${_class}Test_${TestFile_Name}
           COMMAND ${Python_EXECUTABLE} ${PROJECT_SOURCE_DIR}/test/RegressTests.py
-          $<TARGET_FILE:ledger> ${PROJECT_SOURCE_DIR}
+          --ledger $<TARGET_FILE:ledger> --sourcepath ${PROJECT_SOURCE_DIR}
           ${TestFile} ${TEST_PYTHON_FLAGS})
         set_tests_properties(${_class}Test_${TestFile_Name}
           PROPERTIES ENVIRONMENT "TZ=${Ledger_TEST_TIMEZONE}")
diff --git a/test/CheckBaselineTests.py b/test/CheckBaselineTests.py
index 9f52dd10..1a983b00 100755
--- a/test/CheckBaselineTests.py
+++ b/test/CheckBaselineTests.py
@@ -1,9 +1,9 @@
 #!/usr/bin/env python3
 
+import argparse
 import sys
 import re
 import os
-import argparse
 
 from os.path import *
 from subprocess import Popen, PIPE
@@ -51,24 +51,9 @@ class CheckBaselineTests (CheckOptions):
     return errors
 
 if __name__ == "__main__":
-  def getargs():
-    parser = argparse.ArgumentParser(prog='CheckBaselineTests',
-            description='Check that ledger options are tested')
-    parser.add_argument('-l', '--ledger',
-        dest='ledger',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the ledger executable to test with')
-    parser.add_argument('-s', '--source',
-        dest='source',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the top level ledger source directory')
-    return parser.parse_args()
-
-  args = getargs()
+  args = argparse.ArgumentParser(prog='CheckBaselineTests',
+                                 description='Check that ledger options are tested',
+                                 parents=[CheckOptions.parser()]).parse_args()
   script = CheckBaselineTests(args)
   status = script.main()
   sys.exit(status)
diff --git a/test/CheckManpage.py b/test/CheckManpage.py
index e519a00a..1795b77e 100755
--- a/test/CheckManpage.py
+++ b/test/CheckManpage.py
@@ -19,24 +19,9 @@ class CheckManpage (CheckOptions):
     self.source_type = 'manpage'
 
 if __name__ == "__main__":
-  def getargs():
-    parser = argparse.ArgumentParser(prog='CheckManpage',
-            description='Check that ledger options are documented in the manpage')
-    parser.add_argument('-l', '--ledger',
-        dest='ledger',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the ledger executable to test with')
-    parser.add_argument('-s', '--source',
-        dest='source',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the top level ledger source directory')
-    return parser.parse_args()
-
-  args = getargs()
+  args = argparse.ArgumentParser(prog='CheckManpage',
+                                 description='Check that ledger options are documented in the manpage'
+                                 parents=[CheckOptions.parser()]).parse_args()
   script = CheckManpage(args)
   status = script.main()
   sys.exit(status)
diff --git a/test/CheckOptions.py b/test/CheckOptions.py
index faf1630e..cdd9b244 100755
--- a/test/CheckOptions.py
+++ b/test/CheckOptions.py
@@ -4,6 +4,7 @@ import re
 import os
 import sys
 import shlex
+import pathlib
 import argparse
 import subprocess
 
@@ -11,6 +12,15 @@ from os.path import *
 from subprocess import Popen, PIPE
 
 class CheckOptions (object):
+  @staticmethod
+  def parser():
+    parser = argparse.ArgumentParser(add_help=False)
+    parser.add_argument('-l', '--ledger', type=pathlib.Path, required=True,
+        help='the path to the ledger executable to test with')
+    parser.add_argument('-s', '--source', type=pathlib.Path, required=True,
+        help='the path to the top level ledger source directory')
+    return parser
+
   def __init__(self, args):
     self.option_pattern = None
     self.source_file = None
diff --git a/test/CheckTexinfo.py b/test/CheckTexinfo.py
index 82c7a286..7b13c50e 100755
--- a/test/CheckTexinfo.py
+++ b/test/CheckTexinfo.py
@@ -90,24 +90,9 @@ class CheckTexinfo (CheckOptions):
     return options
 
 if __name__ == "__main__":
-  def getargs():
-    parser = argparse.ArgumentParser(prog='CheckTexinfo',
-            description='Check that ledger options are documented in the texinfo manual')
-    parser.add_argument('-l', '--ledger',
-        dest='ledger',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the ledger executable to test with')
-    parser.add_argument('-s', '--source',
-        dest='source',
-        type=str,
-        action='store',
-        required=True,
-        help='the path to the top level ledger source directory')
-    return parser.parse_args()
-
-  args = getargs()
+  args = argparse.ArgumentParser(prog='CheckTexinfo',
+                                 description='Check that ledger options are documented in the texinfo manual',
+                                 parents=[CheckOptions.parser()]).parse_args()
   script = CheckTexinfo(args)
   status = script.main()
   sys.exit(status)
diff --git a/test/ConfirmTests.py b/test/ConfirmTests.py
index 0dc2b9f5..54187130 100755
--- a/test/ConfirmTests.py
+++ b/test/ConfirmTests.py
@@ -3,18 +3,22 @@
 # This script confirms both that the register report "adds up", and that its
 # final balance is the same as what the balance report shows.
 
+import argparse
+import pathlib
 import sys
 import os
 import re
 
 from LedgerHarness import LedgerHarness
 
-harness = LedgerHarness(sys.argv)
-tests   = sys.argv[3]
+parser = argparse.ArgumentParser(prog='ConfirmTests', parents=[LedgerHarness.parser()])
+parser.add_argument('tests', type=pathlib.Path)
+args = parser.parse_args()
+harness = LedgerHarness(args.ledger, args.sourcepath, args.verify, args.gmalloc, args.python)
 
-if not os.path.isdir(tests) and not os.path.isfile(tests):
-    sys.stderr.write("'%s' is not a directory or file (cwd %s)" %
-                     (tests, os.getcwd()))
+if not os.path.isdir(args.tests) and not os.path.isfile(args.tests):
+    print(f'{args.tests} is not a directory or file (cwd: {os.getcwd()})'
+          , file=sys.stderr)
     sys.exit(1)
 
 commands = [
@@ -86,7 +90,7 @@ def confirm_report(command):
     return not failure
 
 for cmd in commands:
-    if confirm_report('$ledger --rounding $cmd ' + re.sub('\$tests', tests, cmd)):
+    if confirm_report('$ledger --rounding $cmd ' + re.sub('\$tests', str(args.tests), cmd)):
         harness.success()
     else:
         harness.failure()
diff --git a/test/GenerateTests.py b/test/GenerateTests.py
index 9bd4a7bc..1301bcd0 100755
--- a/test/GenerateTests.py
+++ b/test/GenerateTests.py
@@ -3,7 +3,10 @@
 # This script confirms both that the register report "adds up", and that its
 # final balance is the same as what the balance report shows.
 
+import argparse
+import pathlib
 import sys
+import os
 import re
 
 from difflib import ndiff
@@ -15,19 +18,21 @@ try:
 except:
     pass
 
-args = sys.argv
-jobs = 1
-match = re.match('-j([0-9]+)?', args[1])
-if match:
-    args = [args[0]] + args[2:]
-    if match.group(1):
-        jobs = int(match.group(1))
-if jobs == 1:
-    multiproc = False
-
 from LedgerHarness import LedgerHarness
 
-harness = LedgerHarness(args)
+parser = argparse.ArgumentParser(prog='GenerateTests', parents=[LedgerHarness.parser()])
+parser.add_argument('-j', '--jobs', type=int, default=1)
+parser.add_argument('tests', type=pathlib.Path)
+parser.add_argument('beg_range', nargs='?', type=int, default=1)
+parser.add_argument('end_range', nargs='?', type=int, default=20)
+args = parser.parse_args()
+multiproc &= (args.jobs >= 1)
+harness = LedgerHarness(args.ledger, args.sourcepath, args.verify, args.gmalloc, args.python)
+
+if not os.path.isdir(args.tests) and not os.path.isfile(args.tests):
+    print(f'{args.tests} is not a directory or file (cwd: {os.getcwd()})'
+          , file=sys.stderr)
+    sys.exit(1)
 
 #def normalize(line):
 #    match = re.match("((\s*)([A-Za-z]+)?(\s*)([-0-9.]+)(\s*)([A-Za-z]+)?)(  (.+))?$", line)
@@ -123,12 +128,6 @@ def generation_test(seed):
 
     return success
 
-beg_range = 1
-end_range = 20
-if len(args) > 4:
-    beg_range = int(args[3])
-    end_range = int(args[4])
-
 def run_gen_test(i):
     if generation_test(i):
         harness.success()
@@ -137,14 +136,14 @@ def run_gen_test(i):
     return harness.failed
 
 if multiproc:
-    pool = Pool(jobs*2)
+    pool = Pool(args.jobs*2)
 else:
     pool = None
 
 if pool:
-    pool.map(run_gen_test, range(beg_range, end_range))
+    pool.map(run_gen_test, range(args.beg_range, args.end_range))
 else:
-    for i in range(beg_range, end_range):
+    for i in range(args.beg_range, args.end_range):
         run_gen_test(i)
 
 if pool:
diff --git a/test/LedgerHarness.py b/test/LedgerHarness.py
index 0da32e8a..e2c96894 100755
--- a/test/LedgerHarness.py
+++ b/test/LedgerHarness.py
@@ -1,5 +1,8 @@
 #!/usr/bin/env python3
 
+import argparse
+import pathlib
+import shlex
 import sys
 import os
 import re
@@ -33,27 +36,36 @@ copyreg.pickle(types.MethodType, _pickle_method, _unpickle_method)
 class LedgerHarness:
     ledger     = None
     sourcepath = None
+    skipped    = 0
     succeeded  = 0
     failed     = 0
     verify     = False
     gmalloc    = False
     python     = False
 
-    def __init__(self, argv):
-        if not os.path.isfile(argv[1]):
-            print("Cannot find ledger at '%s'" % argv[1])
+    @staticmethod
+    def parser():
+      parser = argparse.ArgumentParser(add_help=False)
+      parser.add_argument('-l', '--ledger', type=pathlib.Path, required=True)
+      parser.add_argument('-s', '--sourcepath', type=pathlib.Path, required=True)
+      parser.add_argument('--verify', action='store_true')
+      parser.add_argument('--gmalloc', action='store_true')
+      parser.add_argument('--python', action='store_true')
+      return parser
+
+    def __init__(self, ledger, sourcepath, verify=False, gmalloc=False, python=False):
+        if not ledger.is_file():
+            print("Cannot find ledger at '{ledger}'", file=sys.stderr)
             sys.exit(1)
-        if not os.path.isdir(argv[2]):
-            print("Cannot find source path at '%s'" % argv[2])
+        if not sourcepath.is_dir():
+            print("Cannot find source path at '{sourcepath}'", file=sys.stderr)
             sys.exit(1)
 
-        self.ledger     = os.path.realpath(argv[1])
-        self.sourcepath = os.path.realpath(argv[2])
-        self.succeeded  = 0
-        self.failed     = 0
-        self.verify     = '--verify' in argv
-        self.gmalloc    = '--gmalloc' in argv
-        self.python     = '--python' in argv
+        self.ledger     = ledger.resolve()
+        self.sourcepath = sourcepath.resolve()
+        self.verify     = verify
+        self.gmalloc    = gmalloc
+        self.python     = python
 
     def run(self, command, verify=None, gmalloc=None, columns=True):
         env = os.environ.copy()
@@ -70,34 +82,29 @@ class LedgerHarness:
             env['MALLOC_FILL_SPACE']     = '1'
             env['MALLOC_STRICT_SIZE']    = '1'
 
+        cmd = [str(self.ledger), '--args-only']
         if (verify is not None and verify) or \
            (verify is None and self.verify):
-            insert = ' --verify'
-        else:
-            insert = ''
-
+            cmd.append('--verify')
         if columns:
-            insert += ' --columns=80'
-
-        command = command.replace('$ledger', '"%s"%s %s' % \
-                         (self.ledger, insert, '--args-only'))
+            cmd.append('--columns=80')
+        command = command.replace('$ledger', shlex.join(cmd))
 
         valgrind = '/usr/bin/valgrind'
         if not os.path.isfile(valgrind):
             valgrind = '/opt/local/bin/valgrind'
 
-        if os.path.isfile(valgrind) and '--verify' in insert:
-            command = valgrind + ' -q ' + command
+        if os.path.isfile(valgrind) and '--verify' in cmd:
+            command = shlex.join([valgrind, '-q', command])
 
-        # If we are running under msys2, use bash to execute the test commands
-        if 'MSYSTEM' in os.environ:
+        ismsys2 = 'MSYSTEM' in os.environ
+        if ismsys2:
+            # If we are running under msys2, use bash to execute the test commands
             bash_path = os.environ['MINGW_PREFIX'] + '/../usr/bin/bash.exe'
-            return Popen([bash_path, '-c', command], shell=False,
-                         close_fds=False, env=env, stdin=PIPE, stdout=PIPE,
-                         stderr=PIPE, cwd=self.sourcepath)
+            command = shlex.join([bash_path, '-c', command])
 
-        return Popen(command, shell=True, close_fds=True, env=env,
-                     stdin=PIPE, stdout=PIPE, stderr=PIPE, 
+        return Popen(command, shell=not ismsys2, close_fds=not ismsys2,
+                     env=env, stdin=PIPE, stdout=PIPE, stderr=PIPE,
                      cwd=self.sourcepath)
 
     def read(self, fd):
@@ -112,10 +119,7 @@ class LedgerHarness:
     def readlines(self, fd):
         lines = []
         for line in fd.readlines():
-            if sys.version_info.major == 2:
-                line = unicode(line, 'utf-8')
-            else:
-                line = line.decode('utf-8')
+            line = line.decode('utf-8')
             if not line.startswith('GuardMalloc'):
                 lines.append(line)
         return lines
@@ -133,6 +137,11 @@ class LedgerHarness:
         sys.stdout.flush()
         self.succeeded += 1
 
+    def skip(self):
+        sys.stdout.write("S")
+        sys.stdout.flush()
+        self.skipped += 1
+
     def failure(self, name=None):
         sys.stdout.write("E")
         if name:
@@ -143,16 +152,20 @@ class LedgerHarness:
     def exit(self):
         print()
         if self.succeeded > 0:
-            print("OK (%d) " % self.succeeded,)
+            print(f"OK ({self.succeeded})")
+        if self.skipped > 0:
+            print(f"SKIPPED ({self.skipped})")
         if self.failed > 0:
-            print("FAILED (%d)" % self.failed,)
+            print(f"FAILED ({self.failed})")
         print()
 
         sys.exit(self.failed)
 
 if __name__ == '__main__':
-    harness = LedgerHarness(sys.argv)
-    proc = harness.run('$ledger -f doc/sample.dat reg')
+    parser = argparse.ArgumentParser(prog='LedgerHarness', parents=[LedgerHarness.parser()])
+    args = LedgerHarness.parser().parse_args()
+    harness = LedgerHarness(args.ledger, args.sourcepath, args.verify, args.gmalloc, args.python)
+    proc = harness.run('$ledger -f test/input/sample.dat reg')
     print('STDOUT:')
     print(proc.stdout.read())
     print('STDERR:')
diff --git a/test/RegressTests.py b/test/RegressTests.py
index a3998f1f..47abc3d0 100755
--- a/test/RegressTests.py
+++ b/test/RegressTests.py
@@ -2,6 +2,8 @@
 
 from io import open
 
+import argparse
+import pathlib
 import sys
 import os
 import re
@@ -18,22 +20,16 @@ from difflib import unified_diff
 
 from LedgerHarness import LedgerHarness
 
-args = sys.argv
-jobs = 1
-match = re.match('-j([0-9]+)?', args[1])
-if match:
-    args = [args[0]] + args[2:]
-    if match.group(1):
-        jobs = int(match.group(1))
-if jobs == 1:
-    multiproc = False
-
-harness = LedgerHarness(args)
-tests   = args[3]
-
-if not os.path.isdir(tests) and not os.path.isfile(tests):
-    sys.stderr.write("'%s' is not a directory or file (cwd %s)" %
-                     (tests, os.getcwd()))
+parser = argparse.ArgumentParser(prog='RegressTests', parents=[LedgerHarness.parser()])
+parser.add_argument('-j', '--jobs', type=int, default=1)
+parser.add_argument('tests', type=pathlib.Path)
+args = parser.parse_args()
+multiproc &= (args.jobs >= 1)
+harness = LedgerHarness(args.ledger, args.sourcepath, args.verify, args.gmalloc, args.python)
+
+if not args.tests.is_dir() and not args.tests.is_file():
+    print(f'{args.tests} is not a directory or file (cwd: {os.getcwd()})'
+          , file=sys.stderr)
     sys.exit(1)
 
 class RegressFile(object):
@@ -42,33 +38,31 @@ class RegressFile(object):
         self.fd = open(self.filename, encoding='utf-8')
 
     def transform_line(self, line):
-        line = line.replace('$sourcepath', harness.sourcepath)
-        line = line.replace('$FILE', os.path.realpath(self.filename))
-        return line
+        return line\
+            .replace('$sourcepath', str(harness.sourcepath))\
+            .replace('$FILE', str(self.filename.resolve()))
 
     def read_test(self):
-        test = {
-            'command':  None,
-            'output':   None,
-            'error':    None,
-            'exitcode': 0
-        }
+        class Test:
+            command =  None
+            output =   None
+            error =    None
+            exitcode = 0
 
         in_output = False
         in_error  = False
 
         line = self.fd.readline()
-        if sys.version_info.major == 2 and type(line) is str:
-            line = unicode(line, 'utf-8')
+        test = Test()
         while line:
             if line.startswith("test "):
                 command = line[5:]
                 match = re.match('(.*) -> ([0-9]+)', command)
                 if match:
-                    test['command'] = self.transform_line(match.group(1))
-                    test['exitcode'] = int(match.group(2))
+                    test.command = self.transform_line(match.group(1))
+                    test.exitcode = int(match.group(2))
                 else:
-                    test['command'] = command
+                    test.command = command
                 in_output = True
 
             elif in_output:
@@ -76,57 +70,50 @@ class RegressFile(object):
                     in_output = in_error = False
                     break
                 elif in_error:
-                    if test['error'] is None:
-                        test['error'] = []
-                    test['error'].append(self.transform_line(line))
+                    if test.error is None:
+                        test.error = []
+                    test.error.append(self.transform_line(line))
                 else:
                     if line.startswith("__ERROR__"):
                         in_error = True
                     else:
-                        if test['output'] is None:
-                            test['output'] = []
-                        test['output'].append(self.transform_line(line))
-
+                        if test.output is None:
+                            test.output = []
+                        test.output.append(self.transform_line(line))
             line = self.fd.readline()
-            if sys.version_info.major == 2 and type(line) is str:
-               line = unicode(line, 'utf-8')
             #print("line =", line)
 
-        return test['command'] and test
+        return test.command and test
 
     def notify_user(self, msg, test):
         print(msg)
         print("--")
-        print(self.transform_line(test['command']),)
+        print(self.transform_line(test.command),)
         print("--")
 
     def run_test(self, test):
         use_stdin = False
         if sys.platform == 'win32':
-            test['command'] = test['command'].replace('/dev/null', 'nul')
+            test.command = test.command.replace('/dev/null', 'nul')
             # There is no equivalent to /dev/stdout, /dev/stderr, /dev/stdin
             # on Windows, so skip tests that require them.
-            if '/dev/std' in test['command']:
-                harness.success()
+            if '/dev/std' in test.command:
+                harness.skipped()
                 return
-        if test['command'].find("-f ") != -1:
-            test['command'] = '$ledger ' + test['command']
-            if re.search("-f (-|/dev/stdin)(\s|$)", test['command']):
+        if test.command.find('-f ') != -1:
+            test.command = '$ledger ' + test.command
+            if re.search('-f (-|/dev/stdin)(\s|$)', test.command):
                 use_stdin = True
         else:
-            test['command'] = (('$ledger -f "%s" ' % 
-                                os.path.realpath(self.filename)) +
-                               test['command'])
+            test.command = f'$ledger -f "{str(self.filename.resolve())}" {test.command}'
 
-        p = harness.run(test['command'],
-                        columns=(not re.search('--columns', test['command'])))
+        p = harness.run(test.command,
+                        columns=(not re.search('--columns', test.command)))
 
         if use_stdin:
             fd = open(self.filename, encoding='utf-8')
             try:
-                stdin = fd.read()
-                if sys.version_info.major > 2:
-                    stdin = stdin.encode('utf-8')
+                stdin = fd.read().encode('utf-8')
                 p.stdin.write(stdin)
             finally:
                 fd.close()
@@ -135,9 +122,9 @@ class RegressFile(object):
         success = True
         printed = False
         index   = 0
-        if test['output'] is not None:
+        if test.output is not None:
             process_output = harness.readlines(p.stdout)
-            expected_output = test['output']
+            expected_output = test.output
             if sys.platform == 'win32':
                 process_output = [l.replace('\r\n', '\n').replace('\\', '/')
                                   for l in process_output]
@@ -154,21 +141,19 @@ class RegressFile(object):
                     self.notify_user("FAILURE in output from %s:" % self.filename, test)
                     success = False
                     printed = True
-                if sys.version_info.major == 2 and type(line) is str:
-                    line = unicode(line, 'utf-8')
                 print(' ', line,)
 
         printed = False
         index   = 0
         process_error = harness.readlines(p.stderr)
-        if test['error'] is not None or process_error is not None:
-            if test['error'] is None:
-                test['error'] = []
+        if test.error is not None or process_error is not None:
+            if test.error is None:
+                test.error = []
             if sys.platform == 'win32':
                 process_error = [l.replace('\r\n', '\n').replace('\\', '/')
                                  for l in process_error]
-                test['error'] = [l.replace('\\', '/') for l in test['error']]
-            for line in unified_diff(test['error'], process_error):
+                test.error = [l.replace('\\', '/') for l in test.error]
+            for line in unified_diff(test.error, process_error):
                 index += 1
                 if index < 3:
                     continue
@@ -180,24 +165,24 @@ class RegressFile(object):
                     printed = True
                 print(" ", line,)
 
-        if test['exitcode'] == p.wait():
+        if test.exitcode == p.wait():
             if success:
                 harness.success()
             else:
-                harness.failure(os.path.basename(self.filename))
+                harness.failure(self.filename.name)
                 print("STDERR:")
                 print(p.stderr.read())
         else:
             if success: print
             self.notify_user("FAILURE in exit code (%d != %d) from %s:"
-                             % (test['exitcode'], p.returncode, self.filename),
+                             % (test.exitcode, p.returncode, self.filename),
                              test)
-            harness.failure(os.path.basename(self.filename))
+            harness.failure(self.filename.name)
 
     def run_tests(self):
         if os.path.getsize(self.filename) == 0:
           print("WARNING: Empty testfile detected: %s" % (self.filename), file=sys.stderr)
-          harness.failure(os.path.basename(self.filename))
+          harness.failure(self.filename.name)
           return False
         test = self.read_test()
         while test:
@@ -214,22 +199,21 @@ def do_test(path):
 
 if __name__ == '__main__':
     if multiproc:
-        pool = Pool(jobs*2)
+        pool = Pool(args.jobs*2)
     else:
         pool = None
 
-    if os.path.isdir(tests):
-        tests = [os.path.join(tests, x)
-                 for x in os.listdir(tests) 
-                 if (x.endswith('.test') and 
-                     (not '_py.test' in x or (harness.python and
-                                              not harness.verify)))]
+    if args.tests.is_dir():
+        tests = [p for p in args.tests.iterdir()
+                if (p.suffix == '.test' and
+                    (not p.match('*_py.test') or (harness.python and
+                                             not harness.verify)))]
         if pool:
             pool.map(do_test, tests, 1)
         else:
             map(do_test, tests)
     else:
-        entry = RegressFile(tests)
+        entry = RegressFile(args.tests)
         entry.run_tests()
         entry.close()