]> git.ipfire.org Git - thirdparty/rsync.git/commitdiff
testsuite: harden output-options checks
authorAndrew Tridgell <andrew@tridgell.net>
Sun, 24 May 2026 22:04:56 +0000 (08:04 +1000)
committerAndrew Tridgell <andrew@tridgell.net>
Mon, 25 May 2026 21:43:00 +0000 (07:43 +1000)
Several subcases ran rsync without checking the exit status, so a silent
failure could pass as the expected (often empty) output -- most notably -q,
which only asserted empty stdout. Route every expected-success run through a
helper that asserts the exit status, and verify -q actually transferred the
tree. Replace the "-h/-8 didn't break the transfer" check with positive format
assertions: -h must render byte counts with a K/M/G suffix (and the default
must not), and -8 must leave a high-bit filename byte unescaped (\#371 absent)
where the default escapes it -- best-effort, self-skipping where the platform
can't store the raw byte.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
testsuite/output-options_test.py

index 3a836e1cd5d386ab3c59c0a2bedec43634051418..1d57c086aa430029b542b71af4a0483e0601fec4 100644 (file)
@@ -5,25 +5,40 @@ These options control rsync's OUTPUT, not its path handling, so they are
 checked for the documented output shape rather than at depth:
   --version, --help, --itemize-changes (-i), --dry-run (-n), --stats,
   --out-format, --list-only, --quiet (-q), --progress, -h, -8.
+
+Every rsync run that is expected to succeed has its exit status checked (a
+silent failure must not pass as "no output"), and the format-changing options
+(-h, -8) assert the documented difference rather than merely "didn't break".
 """
 
+import os
+import re
 import subprocess
 
 from rsyncfns import (
     FROMDIR, TODIR,
-    assert_not_exists, make_tree, rmtree, rsync_argv, test_fail,
+    assert_not_exists, make_data_file, make_tree, makepath, rmtree, rsync_argv,
+    test_fail, verify_dirs,
 )
 
 src = FROMDIR
 
 
-def out(*args):
-    return subprocess.run(rsync_argv(*args), capture_output=True, text=True)
+def out(*args, want_rc=0, env=None, text=True):
+    """Run rsync capturing output. Unless want_rc is None, fail the test if the
+    exit status isn't want_rc -- so a broken transfer can't masquerade as the
+    expected (often empty) output."""
+    p = subprocess.run(rsync_argv(*args), capture_output=True, text=text, env=env)
+    if want_rc is not None and p.returncode != want_rc:
+        err = p.stderr if text else p.stderr.decode('latin-1', 'replace')
+        test_fail(f"rsync {' '.join(args)} exited {p.returncode}, "
+                  f"expected {want_rc}:\n{err}")
+    return p
 
 
 # --- --version / --help -----------------------------------------------------
 p = out('--version')
-if p.returncode != 0 or 'protocol version' not in p.stdout:
+if 'protocol version' not in p.stdout:
     test_fail(f"--version output unexpected:\n{p.stdout}")
 p = out('--help')
 help_txt = p.stdout + p.stderr
@@ -68,11 +83,14 @@ if 'f0' not in p.stdout:
     test_fail(f"--list-only did not list files:\n{p.stdout}")
 assert_not_exists(TODIR / 'f0', label='--list-only copied a file')
 
-# --- --quiet suppresses normal stdout ---------------------------------------
+# --- --quiet suppresses normal stdout BUT still transfers -------------------
+# Checking only for empty stdout would also pass if the transfer silently
+# failed, so confirm the destination actually received the tree.
 rmtree(TODIR)
 p = out('-a', '-q', f'{src}/', f'{TODIR}/')
 if p.stdout.strip() != '':
     test_fail(f"--quiet produced stdout: {p.stdout!r}")
+verify_dirs(src, TODIR, label='--quiet still transferred')
 
 # --- --progress shows a percentage ------------------------------------------
 rmtree(TODIR)
@@ -80,11 +98,49 @@ p = out('-a', '--progress', f'{src}/', f'{TODIR}/')
 if '100%' not in p.stdout:
     test_fail(f"--progress did not show a percentage:\n{p.stdout}")
 
-# --- -h / -8 do not break a transfer ----------------------------------------
+# --- -h / --human-readable formats byte counts with a unit suffix -----------
+# Without -h, --stats prints grouped digits ("50,000 bytes"); with -h it uses a
+# K/M/G suffix ("50.00K"). Use a file big enough that the two forms differ.
+rmtree(src)
+rmtree(TODIR)
+makepath(src)
+make_data_file(src / 'big', 50_000)
+plain = out('-a', '--stats', f'{src}/', f'{TODIR}/').stdout
+rmtree(TODIR)
+human = out('-a', '-h', '--stats', f'{src}/', f'{TODIR}/').stdout
+suffix_re = r'Total file size: [\d.]+[KMG]'
+if not re.search(suffix_re, human):
+    test_fail(f"-h did not use a human-readable unit suffix:\n{human}")
+if re.search(suffix_re, plain):
+    test_fail(f"--stats without -h unexpectedly used a unit suffix:\n{plain}")
+
+# --- -8 / --8-bit-output leaves high-bit filename bytes unescaped ------------
+# rsync escapes non-printable name bytes as \#NNN; -8 prints 8-bit bytes raw.
+# This needs a filename containing a high-bit byte and a C locale (where such a
+# byte is non-printable). Best-effort: skip silently where the filesystem can't
+# store the raw byte (macOS/Cygwin may reject or normalise it).
+rmtree(src)
 rmtree(TODIR)
-p = out('-a', '-h', '-8', '--stats', f'{src}/', f'{TODIR}/')
-if p.returncode != 0:
-    test_fail(f"-h/-8 broke the transfer:\n{p.stderr}")
+makepath(src)
+weird = os.fsencode(str(src)) + b'/hi\xf9name'   # 0xf9 -> octal 371
+try:
+    with open(weird, 'wb') as f:
+        f.write(b"x\n")
+    weird_ok = True
+except OSError:
+    weird_ok = False
+
+if weird_ok:
+    cenv = dict(os.environ, LC_ALL='C')
+    makepath(TODIR)
+    noesc = out('-ai', f'{src}/', f'{TODIR}/', env=cenv, text=False)
+    if rb'\#371' in noesc.stdout:        # FS preserved the raw byte as expected
+        rmtree(TODIR)
+        makepath(TODIR)
+        esc = out('-ai', '-8', f'{src}/', f'{TODIR}/', env=cenv, text=False)
+        if rb'\#371' in esc.stdout:
+            test_fail("-8 should leave the high-bit name byte unescaped, but "
+                      f"the \\#371 escape was still present:\n{esc.stdout!r}")
 
 print("output-options: version/help/-i/-n/--stats/--out-format/--list-only/"
       "-q/--progress/-h/-8 verified")