testsuite/runtests: count XFAIL (exit 78) as expected, not a failure

The regression tests use test_xfail() (exit 78) to assert a known, documented
residual on platforms where the fix can't apply -- e.g. link-dest-relative-basis
XFAILs where the receiver has no openat2/O_RESOLVE_BENEATH and the portable
resolver rejects the '..' for safety.  runtests.py counted exit 78 in the
generic else->failed branch, so a bare XFAIL failed the whole suite; tally it
separately ('N xfailed (expected)') and exclude it from the failure exit code.
Also add --race-timeout plumbing (race_timeout env) for race tests.

diff --git a/runtests.py b/runtests.py
index dc19d87835777f9f39a03df00ada17b1d60dc79b..3a1c7485271c1318d327713e5d7bfdb0eb8f3eae 100755 (executable)
--- a/runtests.py
+++ b/runtests.py
@@ -31,6 +31,11 @@ import subprocess
 import sys
 import threading
 
+# Share the test exit-code enum with the test helpers. exitcodes.py lives in
+# testsuite/ (next to this script); it has no import-time side effects.
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), 'testsuite'))
+from exitcodes import Exit
+
 
 def parse_args():
     p = argparse.ArgumentParser(description='Run rsync test suite')
@@ -58,6 +63,9 @@ def parse_args():
                    help='Stop after first test failure')
     p.add_argument('--timeout', type=int, default=300, metavar='SECS',
                    help='Per-test timeout in seconds (default: 300)')
+    p.add_argument('--race-timeout', type=float, default=5.0, metavar='SECS',
+                   help='Budget (seconds) a TOCTOU symlink-race test may spend '
+                        'trying to win its race before concluding (default: 5)')
     p.add_argument('--rsync-bin', default=None, metavar='PATH',
                    help='Path to rsync binary (default: ./rsync)')
     p.add_argument('--rsync-bin2', default=None, metavar='PATH',
@@ -242,18 +250,18 @@ def parse_expect_result(path):
                     f"{path}:{lineno}: expected '<testname> "
                     f"<{'|'.join(_VALID_OUTCOMES)}>', got: {raw.rstrip()}\n"
                 )
-                sys.exit(2)
+                sys.exit(Exit.ERROR)
             expect[fields[0]] = fields[1]
     return expect
 
 
 def outcome_of(result):
     """Map a per-test exit code to an outcome string."""
-    if result == 0:
+    if result == Exit.PASS:
         return 'pass'
-    if result == 77:
+    if result == Exit.SKIP:
         return 'skip'
-    if result == 78:
+    if result == Exit.XFAIL:
         return 'xfail'
     return 'fail'
 
@@ -321,7 +329,7 @@ def run_one_test(testscript, testbase, scratchdir, base_env, timeout,
     # Build output text
     output_parts = []
 
-    show_log = always_log or (result not in (0, 77, 78))
+    show_log = always_log or (result not in (Exit.PASS, Exit.SKIP, Exit.XFAIL))
     if show_log:
         output_parts.append(f'----- {testbase} log follows')
         try:
@@ -338,9 +346,9 @@ def run_one_test(testscript, testbase, scratchdir, base_env, timeout,
             output_parts.append(f'----- {testbase} rsyncd.log ends')
 
     skipped_reason = ''
-    if result == 0:
+    if result == Exit.PASS:
         output_parts.append(f'PASS    {testbase}')
-    elif result == 77:
+    elif result == Exit.SKIP:
         whyfile = os.path.join(scratchdir, 'whyskipped')
         try:
             with open(whyfile) as f:
@@ -348,7 +356,7 @@ def run_one_test(testscript, testbase, scratchdir, base_env, timeout,
         except FileNotFoundError:
             pass
         output_parts.append(f'SKIP    {testbase} ({skipped_reason})')
-    elif result == 78:
+    elif result == Exit.XFAIL:
         output_parts.append(f'XFAIL   {testbase}')
     else:
         output_parts.append(f'FAIL    {testbase}')
@@ -407,13 +415,13 @@ def main():
 
     if not os.path.isfile(rsync_bin):
         sys.stderr.write(f"rsync_bin {rsync_bin} is not a file\n")
-        sys.exit(2)
+        sys.exit(Exit.ERROR)
     if not os.path.isfile(rsync_bin2):
         sys.stderr.write(f"rsync_bin2 {rsync_bin2} is not a file\n")
-        sys.exit(2)
+        sys.exit(Exit.ERROR)
     if not os.path.isdir(srcdir):
         sys.stderr.write(f"srcdir {srcdir} is not a directory\n")
-        sys.exit(2)
+        sys.exit(Exit.ERROR)
 
     # Helper programs the test scripts invoke directly. Missing any of these
     # would cause many tests to fail with confusing "not found" errors, so
@@ -430,7 +438,7 @@ def main():
             f"Build them with: make {' '.join(missing)}\n"
             f"or run the full test target: make check\n"
         )
-        sys.exit(2)
+        sys.exit(Exit.ERROR)
 
     testuser = get_testuser()
 
@@ -475,6 +483,7 @@ def main():
         'scratchbase': scratchbase,
         'suitedir': suitedir,
         'TESTRUN_TIMEOUT': str(args.timeout),
+        'race_timeout': str(args.race_timeout),
         'HOME': scratchbase,
         'PYTHONPATH': pythonpath,
     })
@@ -535,34 +544,40 @@ def main():
     passed = 0
     failed = 0
     skipped = 0
+    xfailed = 0
     skipped_list = []
     outcomes = {}  # testbase -> actual outcome string ('pass'/'skip'/'fail'/'xfail')
 
     def process_result(tr):
         """Process a TestResult and update counters. Returns True if the test
         should count as a failure for --stop-on-fail purposes."""
-        nonlocal passed, failed, skipped
+        nonlocal passed, failed, skipped, xfailed
         with _print_lock:
             if tr.output:
                 print(tr.output)
         scratchdir = os.path.join(scratchbase, tr.testbase)
         oc = outcome_of(tr.result)
         outcomes[tr.testbase] = oc
-        if tr.result == 0:
+        if tr.result == Exit.PASS:
             passed += 1
-        elif tr.result == 77:
+        elif tr.result == Exit.SKIP:
             skipped_list.append(tr.testbase)
             skipped += 1
+        elif tr.result == Exit.XFAIL:
+            # XFAIL: an expected failure (a known, documented residual the test
+            # asserts against). Reported distinctly but does NOT fail the suite;
+            # when the underlying issue is fixed the test returns 0 instead.
+            xfailed += 1
         else:
             failed += 1
-        if tr.result in (0, 77) and not args.preserve_scratch \
+        if tr.result in (Exit.PASS, Exit.SKIP, Exit.XFAIL) and not args.preserve_scratch \
                 and os.path.isdir(scratchdir):
             subprocess.run(['rm', '-rf', scratchdir], capture_output=True)
         # With a manifest, only a mismatch is a "failure" (an expected fail is
-        # fine); without one, any non-pass/non-skip result is a failure.
+        # fine); without one, any non-pass/non-skip/non-xfail result is a failure.
         if expect is not None:
             return mismatch(tr.testbase, oc)
-        return tr.result not in (0, 77)
+        return tr.result not in (Exit.PASS, Exit.SKIP, Exit.XFAIL)
 
     if args.parallel > 1:
         # Parallel execution
@@ -624,6 +639,8 @@ def main():
     print(f'      {passed} passed')
     if failed > 0:
         print(f'      {failed} failed')
+    if xfailed > 0:
+        print(f'      {xfailed} xfailed (expected)')
     if skipped > 0:
         print(f'      {skipped} skipped')
     if vg_errors > 0:

diff --git a/testsuite/exitcodes.py b/testsuite/exitcodes.py
new file mode 100644 (file)
index 0000000..faf3efc
--- /dev/null
+++ b/testsuite/exitcodes.py
@@ -0,0 +1,17 @@
+"""Exit codes a test reports to runtests.py (autotools test convention).
+
+Shared by runtests.py (the harness, which reads these from each test) and
+rsyncfns.py (the helpers, which exit with them) so the 0/1/2/77/78 values are
+named in exactly one place.  This module has no import-time side effects, so
+runtests.py can import it without pulling in rsyncfns's environment checks.
+"""
+
+import enum
+
+
+class Exit(enum.IntEnum):
+    PASS = 0
+    FAIL = 1
+    ERROR = 2     # the test could not run (e.g. missing environment)
+    SKIP = 77
+    XFAIL = 78    # expected failure: a known, documented residual

diff --git a/testsuite/rsyncfns.py b/testsuite/rsyncfns.py
index d4c06606030fd81f69db98dca45838c7334a0998..9a96c2b058e18c8dbaf089d40a8440d5042231fe 100644 (file)
--- a/testsuite/rsyncfns.py
+++ b/testsuite/rsyncfns.py
@@ -5,7 +5,7 @@ the Python-rewritten tests actually need; grow it as more shell tests are
 ported.
 
 Conventions matching the shell harness:
-  * Exit 0 = pass, 1 = fail, 77 = skip, 78 = xfail.
+  * Exit codes (see the Exit enum): 0=pass, 1=fail, 2=error, 77=skip, 78=xfail.
   * The runner sets these environment variables before invoking each test:
       scratchdir   per-test scratch directory
       srcdir       rsync source directory
@@ -31,6 +31,8 @@ import sys
 import time
 from pathlib import Path
 
+from exitcodes import Exit   # re-exported: tests may `from rsyncfns import Exit`
+
 
 # --- environment -----------------------------------------------------------
 
@@ -41,7 +43,7 @@ def _required(name: str) -> str:
             f"rsyncfns: required environment variable {name} is not set; "
             "run this test via runtests.py rather than directly.\n"
         )
-        sys.exit(2)
+        sys.exit(Exit.ERROR)
     return v
 
 
@@ -105,18 +107,18 @@ OUTFILE = SCRATCHDIR / 'rsync.out'
 
 def test_fail(msg: str) -> 'None':
     sys.stderr.write(msg.rstrip() + '\n')
-    sys.exit(1)
+    sys.exit(Exit.FAIL)
 
 
 def test_skipped(msg: str) -> 'None':
     sys.stderr.write(msg.rstrip() + '\n')
     (TMPDIR / 'whyskipped').write_text(msg.rstrip() + '\n')
-    sys.exit(77)
+    sys.exit(Exit.SKIP)
 
 
 def test_xfail(msg: str) -> 'None':
     sys.stderr.write(msg.rstrip() + '\n')
-    sys.exit(78)
+    sys.exit(Exit.XFAIL)
 
 
 # --- rsync invocation ------------------------------------------------------