From: Andrew Tridgell <andrew@tridgell.net>
Date: Sun, 24 May 2026 22:12:03 +0000 (+1000)
Subject: testsuite: verify destination content/listings in daemon tests
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1687230672ad4ad23f63d76d6d7a2fdc520de6ed;p=thirdparty%2Frsync.git

testsuite: verify destination content/listings in daemon tests

These daemon tests confirmed refusals/exclusions but accepted the allowed
transfers on exit status alone, so a transfer that exited cleanly while moving
nothing would pass:

  daemon-refuse  allowed() imported verify_dirs but never called it; now it
                 confirms the allowed push/pull actually populated the dest.
  daemon-filter  pull()/the incoming push ignored their exit status, and the
                 outgoing-chmod loop iterated only files that exist -- a
                 zero-file pull passed vacuously. Check the codes and require
                 at least one file to have been mode-checked.
  daemon         run_and_check's unused `expected` param is dropped; the
                 hidden-module and glob listings now compare the exact set of
                 listed paths (catching a leaked extra path), replacing the
                 per-path containment check and the dead normalise() helper
                 whose regex never matched the -r listing format anyway.

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

diff --git a/testsuite/daemon-filter_test.py b/testsuite/daemon-filter_test.py
index cac387e8..59566068 100644
--- a/testsuite/daemon-filter_test.py
+++ b/testsuite/daemon-filter_test.py
@@ -40,8 +40,11 @@ url = start_test_daemon(conf, DAEMON_PORT)
 def pull(mod, dest):
     rmtree(dest)
     makepath(dest)
-    subprocess.run(rsync_argv('-a', f'{url}{mod}/', f'{dest}/'),
-                   stdout=subprocess.DEVNULL)
+    proc = subprocess.run(rsync_argv('-a', f'{url}{mod}/', f'{dest}/'),
+                          stdout=subprocess.DEVNULL, stderr=subprocess.PIPE,
+                          text=True)
+    if proc.returncode not in (0, 23):
+        test_fail(f"pull from {mod} failed (rc={proc.returncode}): {proc.stderr}")
 
 
 # --- daemon exclude hides *.secret everywhere in the module -----------------
@@ -53,8 +56,11 @@ assert_same(fp / 'd1' / 'd2' / 'f2', src / 'd1' / 'd2' / 'f2',
             label='daemon exclude kept others')
 
 # --- incoming chmod rewrites pushed file modes at depth ---------------------
-subprocess.run(rsync_argv('-a', f'{src}/', f'{url}inc/'),
-               stdout=subprocess.DEVNULL)
+proc = subprocess.run(rsync_argv('-a', f'{src}/', f'{url}inc/'),
+                      stdout=subprocess.DEVNULL, stderr=subprocess.PIPE,
+                      text=True)
+if proc.returncode not in (0, 23):
+    test_fail(f"incoming push failed (rc={proc.returncode}): {proc.stderr}")
 checked = 0
 for rel in rels:
     p = incdir / rel
@@ -67,10 +73,16 @@ if checked == 0:
 # --- outgoing chmod rewrites pulled file modes at depth ---------------------
 op = SCRATCHDIR / 'outpull'
 pull('out', op)
+checked = 0
 for rel in rels:
     p = op / rel
-    if p.is_file() and (os.stat(p).st_mode & 0o044):
+    if not p.is_file():
+        continue
+    checked += 1
+    if os.stat(p).st_mode & 0o044:
         test_fail(f"outgoing chmod did not clear group/other read on {rel}: "
                   f"{oct(os.stat(p).st_mode & 0o777)}")
+if checked == 0:
+    test_fail("outgoing chmod test pulled no files (loop was vacuous)")
 
 print("daemon-filter: exclude / incoming chmod / outgoing chmod verified at depth")
diff --git a/testsuite/daemon-refuse_test.py b/testsuite/daemon-refuse_test.py
index 770b36ba..054f2809 100644
--- a/testsuite/daemon-refuse_test.py
+++ b/testsuite/daemon-refuse_test.py
@@ -44,19 +44,23 @@ def refused(args, what):
     return proc.stderr
 
 
-def allowed(args, what):
+def allowed(args, what, expected=None, actual=None):
     proc = subprocess.run(rsync_argv(*args),
                           stdout=subprocess.DEVNULL, stderr=subprocess.PIPE,
                           text=True)
     if proc.returncode not in (0, 23):
         test_fail(f"{what} was unexpectedly refused: {proc.stderr}")
+    # A 0/23 exit alone doesn't prove the transfer happened; verify the data
+    # actually landed for the allowed cases.
+    if expected is not None:
+        verify_dirs(expected, actual, label=what)
 
 
 # --- a named refused option (delete) ----------------------------------------
 refused(['-a', '--delete', f'{src}/', f'{url}refuse-delete/'],
         "--delete on a refuse=delete module")
 allowed(['-a', f'{src}/', f'{url}refuse-delete/'],
-        "plain push to a refuse=delete module")
+        "plain push to a refuse=delete module", src, deldir)
 
 # --- a wildcard refused option (checksum*) ----------------------------------
 dest = SCRATCHDIR / 'wilddest'
@@ -67,7 +71,8 @@ refused(['-a', '--checksum', f'{url}refuse-wild/', f'{dest}/'],
 # --- the "* !a !v" allow-list: -av allowed, -z refused ----------------------
 rmtree(dest)
 makepath(dest)
-allowed(['-av', f'{url}only-av/', f'{dest}/'], "-av on an allow-list module")
+allowed(['-av', f'{url}only-av/', f'{dest}/'], "-av on an allow-list module",
+        src, dest)
 refused(['-avz', f'{url}only-av/', f'{dest}/'],
         "-z on an allow-list module")
 
diff --git a/testsuite/daemon_test.py b/testsuite/daemon_test.py
index 9643e941..4e022fa5 100644
--- a/testsuite/daemon_test.py
+++ b/testsuite/daemon_test.py
@@ -7,7 +7,6 @@
 # by using RSYNC_CONNECT_PROG to spawn the daemon as a child of rsync.
 
 import os
-import re
 import subprocess
 
 from rsyncfns import (
@@ -22,20 +21,21 @@ DAEMON_PORT = 12877
 
 SSH = f"{SRCDIR / 'support' / 'lsh.sh'} --no-cd"
 
-# Replacements that hide the variable parts of `rsync -r` listings: tabs/
-# columns for file vs directory, and the date/time stamp.
-_FILE_RE = re.compile(r'^([^d][^ ]*) *(\.{10}[0-9]) ', flags=re.MULTILINE)
-_DIR_RE = re.compile(r'^(d[^ ]*)  *[0-9][.,0-9]* ', flags=re.MULTILINE)
-_LS_RE = re.compile(
-    r'[0-9]{4}/[0-9]{2}/[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}'
-)
-
 
-def normalise(text: str) -> str:
-    out = _FILE_RE.sub(r'\1 \2 ', text)
-    out = _DIR_RE.sub(r'\1         DIR ', out)
-    out = _LS_RE.sub('####/##/## ##:##:##', out)
-    return out
+def listed_paths(text: str) -> set:
+    """The set of path names in an `rsync -r` listing. Each listing line is
+    "<mode> <size> <date> <time> <path>"; pull out the trailing path. Comparing
+    the whole set (not just checking individual paths are present) catches a
+    listing that leaks EXTRA paths, not only one that omits expected ones."""
+    paths = set()
+    for line in text.splitlines():
+        parts = line.split()
+        # A listing line starts with a 10-char mode string and ends with the
+        # path; -U adds extra date columns, so take the last token (the test's
+        # paths contain no spaces).
+        if len(parts) >= 5 and len(parts[0]) == 10 and parts[0][0] in '-dlbcps':
+            paths.add(parts[-1])
+    return paths
 
 
 conf = build_rsyncd_conf()
@@ -62,7 +62,7 @@ expected_modules = (
 )
 
 
-def run_and_check(args, expected, label, capture_stderr=False):
+def run_and_check(args, label, capture_stderr=False):
     proc = subprocess.run(
         rsync_argv(*args),
         capture_output=True, text=True,
@@ -81,7 +81,7 @@ def run_and_check(args, expected, label, capture_stderr=False):
 rsync_path = f"{RSYNC}{(' ' + ' '.join(confopt)) if confopt else ''}"
 out = run_and_check(
     ['-ve', SSH, f'--rsync-path={rsync_path}', 'localhost::'],
-    expected_modules, "module list via lsh.sh",
+    "module list via lsh.sh",
 )
 if expected_modules not in out:
     test_fail("module list via lsh.sh did not contain the expected modules")
@@ -95,7 +95,7 @@ print('====')
 # loopback rsyncd under --use-tcp).
 daemon_url = start_test_daemon(conf, DAEMON_PORT).rstrip('/')
 
-out = run_and_check(['-v', f'{daemon_url}/'], expected_modules, "module list via daemon")
+out = run_and_check(['-v', f'{daemon_url}/'], "module list via daemon")
 if expected_modules not in out:
     test_fail("module list via daemon did not contain the expected modules")
 # test-hidden is `list = no`; it must NOT appear in the module listing.
@@ -104,43 +104,29 @@ if 'test-hidden' in out:
     test_fail("module list via daemon leaked the `list = no` test-hidden module")
 print('====')
 
-# test-hidden: a recursive listing of the module, with file/dir/date
-# columns normalised so the diff is content-only.
-out = run_and_check(['-r', f'{daemon_url}/test-hidden'], "", "test-hidden listing")
-normalised = normalise(out)
-expected_hidden = """\
-drwxr-xr-x         DIR ####/##/## ##:##:## .
-drwxr-xr-x         DIR ####/##/## ##:##:## bar
--rw-r--r-- ........1 ####/##/## ##:##:## bar/two
-drwxr-xr-x         DIR ####/##/## ##:##:## bar/baz
--rw-r--r-- ........1 ####/##/## ##:##:## bar/baz/three
-drwxr-xr-x         DIR ####/##/## ##:##:## foo
--rw-r--r-- ........1 ####/##/## ##:##:## foo/one
-"""
-# The exact byte sizes vary by locale ("4" vs "          4"); just check that
-# every expected path appears in the normalised output.
-for path in ('bar', 'bar/two', 'bar/baz', 'bar/baz/three', 'foo', 'foo/one'):
-    if path not in normalised:
-        print(normalised)
-        test_fail(f"test-hidden listing missing path {path!r}")
-
-# test-from/f* glob: only the foo subtree.
-out = run_and_check(['-r', f'{daemon_url}/test-from/f*'], "", "test-from glob")
-normalised = normalise(out)
-for path in ('foo', 'foo/one'):
-    if path not in normalised:
-        print(normalised)
-        test_fail(f"test-from glob listing missing path {path!r}")
-if 'bar' in normalised:
-    print(normalised)
-    test_fail("test-from glob listing leaked the bar subtree")
+# test-hidden: a recursive listing of the whole module. Compare the exact set
+# of listed paths so an unexpected/leaked extra path is caught, not only a
+# missing one.
+out = run_and_check(['-r', f'{daemon_url}/test-hidden'], "test-hidden listing")
+got = listed_paths(out)
+want = {'.', 'bar', 'bar/two', 'bar/baz', 'bar/baz/three', 'foo', 'foo/one'}
+if got != want:
+    print(out)
+    test_fail(f"test-hidden listing paths {sorted(got)} != expected {sorted(want)}")
+
+# test-from/f* glob: only the foo subtree, nothing from bar.
+out = run_and_check(['-r', f'{daemon_url}/test-from/f*'], "test-from glob")
+got = listed_paths(out)
+want = {'foo', 'foo/one'}
+if got != want:
+    print(out)
+    test_fail(f"test-from glob paths {sorted(got)} != expected {sorted(want)}")
 
 # atimes-format variant -- only if rsync was built with atimes support.
 vv = run_rsync('-VV', check=True, capture_output=True)
 if '"atimes": true' in vv.stdout:
-    out = run_and_check(['-rU', f'{daemon_url}/test-from/f*'], "", "test-from glob with -U")
-    normalised = normalise(out)
-    for path in ('foo', 'foo/one'):
-        if path not in normalised:
-            print(normalised)
-            test_fail(f"-U glob listing missing path {path!r}")
+    out = run_and_check(['-rU', f'{daemon_url}/test-from/f*'], "test-from glob with -U")
+    got = listed_paths(out)
+    if got != want:
+        print(out)
+        test_fail(f"-U glob paths {sorted(got)} != expected {sorted(want)}")