testsuite: tighten metadata-precision and symlink-target assertions

Replace loose/partial oracles with exact ones:

  omit-times      under -O, require EVERY directory mtime to be omitted, not
                  just one (the old "at least one differs" missed partial bugs).
  dir-sgid        assert the created dirs' actual gid: a setgid parent makes
                  them inherit its group (set to a secondary group to be
                  discriminating), while the non-setgid case gets the process's.
  relative-implied pin a deterministic umask and assert the exact default mode
                  (0o755) for --no-implied-dirs, not merely "not the source's".
  safe-links /    compare the preserved symlink TARGET strings via readlink,
  unsafe-links    not just that a symlink exists.
  preallocate     verify do_punch_hole via st_blocks on the --inplace --sparse
                  case (guarded by a sparse-capability probe).

Note: --preallocate --sparse leaves the file fully allocated on a fresh write
(the zero run is not punched), so that case stays content-only rather than
asserting hole-punching -- see the test comment; rsync.1's claim that the
combination yields sparse blocks does not hold for the fresh-write path.

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

diff --git a/testsuite/dir-sgid_test.py b/testsuite/dir-sgid_test.py
index 44845aad3e43727f6e92b2dcf84127e96c3cd3a4..bfa735ae81644e5b33f85cb010c717a8914c5cc6 100644 (file)
--- a/testsuite/dir-sgid_test.py
+++ b/testsuite/dir-sgid_test.py
@@ -10,17 +10,28 @@ import shutil
 import subprocess
 
 from rsyncfns import (
-    SCRATCHDIR, check_perms, run_rsync, test_skipped,
+    SCRATCHDIR, check_perms, run_rsync, test_fail, test_skipped,
 )
 
 
 old_umask = os.umask(0o077)
 
+# A secondary group (if the user has one) lets the gid checks below be
+# discriminating: a setgid parent makes new dirs inherit the PARENT's group,
+# whereas without setgid they get the process's own group.
+prim = os.getgid()
+alt_gid = next((g for g in os.getgroups() if g != prim), None)
 
-def testit(dirname, dirperms, file_expected, prog_expected, dir_expected):
+
+def testit(dirname, dirperms, file_expected, prog_expected, dir_expected, setgid):
     """Mirror shell `testit dirname dirperms file_expected prog_expected dir_expected`."""
     todir = SCRATCHDIR / dirname
     todir.mkdir()
+    # For the setgid case, give the parent a distinct group (when available) so
+    # the inherited gid differs from the process's; chmod is applied after the
+    # chown so the setgid bit survives.
+    if setgid and alt_gid is not None:
+        os.chown(todir, -1, alt_gid)
     # dirperms is either an octal int or the symbolic shell form we translate.
     if isinstance(dirperms, int):
         os.chmod(todir, dirperms)
@@ -37,6 +48,20 @@ def testit(dirname, dirperms, file_expected, prog_expected, dir_expected):
     check_perms(todir / 'to' / 'file', file_expected)
     check_perms(todir / 'to' / 'program', prog_expected)
 
+    # With setgid set, new dirs must inherit the PARENT's group. We only assert
+    # the setgid case: it holds on both SysV/Linux and BSD, and (because the
+    # parent is given a secondary group above) still proves setgid took effect
+    # on Linux. The no-setgid case is deliberately not checked -- the inherited
+    # gid is OS-defined there (the process's group on Linux, but always the
+    # parent's on BSD), so it's not a portable assertion.
+    if setgid:
+        expect_gid = os.stat(todir).st_gid
+        for sub in ('to', 'to/dir'):
+            g = os.stat(todir / sub).st_gid
+            if g != expect_gid:
+                test_fail(f"{dirname}: {sub} gid is {g}, expected {expect_gid} "
+                          "(setgid inheritance)")
+
 
 # Cygwin's default dir ACL ruins this test; mimic the shell's getfacl skip.
 src_dir = SCRATCHDIR / 'dir'
@@ -66,7 +91,8 @@ if not (os.stat(src_dir).st_mode & 0o2000):
 if not (os.stat(src_dir / 'blah').st_mode & 0o2000):
     test_skipped("Your filesystem doesn't use directory setgid; maybe it's BSD.")
 
-testit('setgid-off', 0o700, 'rw-------', 'rwx------', 'rwx------')
-testit('setgid-on', 'u=rwx,g=rw,g+s,o-rwx', 'rw-------', 'rwx------', 'rwx--S---')
+testit('setgid-off', 0o700, 'rw-------', 'rwx------', 'rwx------', setgid=False)
+testit('setgid-on', 'u=rwx,g=rw,g+s,o-rwx', 'rw-------', 'rwx------', 'rwx--S---',
+       setgid=True)
 
 os.umask(old_umask)

diff --git a/testsuite/omit-times_test.py b/testsuite/omit-times_test.py
index a532c85f9640719640721c0731f98dad6d3d22e2..b994b9d631aceca8fce6c73a56136863e457645a 100644 (file)
--- a/testsuite/omit-times_test.py
+++ b/testsuite/omit-times_test.py
@@ -31,13 +31,13 @@ seed()
 run_rsync('-rlt', '-O', f'{src}/', f'{TODIR}/')
 for f in walk_files(src):
     assert_mtime_close(TODIR / f.relative_to(src), OLD, label=f'-O file {f.name}')
-omitted = False
+# Every directory mtime must be omitted (left at ~now), not just one of them:
+# the old "at least one differs" check would miss a bug that preserved some.
 for d in walk_dirs(src):
-    if abs(os.stat(TODIR / d.relative_to(src)).st_mtime - OLD) > 1:
-        omitted = True            # at least one dir keeps a fresh (now) mtime
-if not omitted:
-    test_fail("-O did not omit directory times -- every dir mtime matched the "
-              "source")
+    rel = d.relative_to(src)
+    if abs(os.stat(TODIR / rel).st_mtime - OLD) <= 1:
+        test_fail(f"-O preserved the mtime of directory {rel} instead of "
+                  "omitting it")
 
 # --- -J: symlink mtime omitted (where the platform records symlink mtimes) --
 seed()

diff --git a/testsuite/preallocate_test.py b/testsuite/preallocate_test.py
index db7e5e37e2da1961db9eea8006a6538aa35fc67f..47a995feab65bfb1bec73a7878601f3673885484 100644 (file)
--- a/testsuite/preallocate_test.py
+++ b/testsuite/preallocate_test.py
@@ -119,6 +119,10 @@ st = os.stat(src / deep)
 os.utime(src / deep, (st.st_atime, st.st_mtime + 100))   # force a delta update
 run_rsync('-a', '--inplace', '--sparse', '--no-whole-file', f'{src}/', f'{TODIR}/')
 assert_same(TODIR / deep, src / deep, label='--inplace --sparse content')
+if can_punch and allocated(TODIR / deep) >= os.path.getsize(TODIR / deep):
+    test_fail(f"--inplace --sparse did not punch the zero run: allocated "
+              f"{allocated(TODIR / deep)} for a {os.path.getsize(TODIR / deep)}"
+              "-byte file")
 
 print("preallocate: --preallocate (do_fallocate) + sparse hole-punching "
       "(do_punch_hole) verified at depth")

diff --git a/testsuite/relative-implied_test.py b/testsuite/relative-implied_test.py
index 167d79dfee50f110cc7691eae1c0b4f6b5af16eb..81d423b5cc40b21f5268a186f5cb5319fbc9462b 100644 (file)
--- a/testsuite/relative-implied_test.py
+++ b/testsuite/relative-implied_test.py
@@ -9,14 +9,14 @@ implied directory carrying a non-default mode, several levels deep.
 """
 
 import os
-import stat
 
 from rsyncfns import (
     SCRATCHDIR, TODIR,
     assert_mode, assert_same, forced_protocol, makepath, rmtree, run_rsync,
-    test_fail,
 )
 
+os.umask(0o022)        # make the default implied-dir mode deterministic (0o755)
+
 base = SCRATCHDIR / 'rbase'
 rmtree(base)
 rmtree(TODIR)
@@ -43,10 +43,11 @@ if proto is not None and proto < 30:
 else:
     rmtree(TODIR)
     run_rsync('-aR', '--no-implied-dirs', 'b/c/file', f'{TODIR}/')
-    m = stat.S_IMODE(os.stat(TODIR / 'b').st_mode)
-    if m == 0o750:
-        test_fail("--no-implied-dirs unexpectedly mirrored the source mode "
-                  "0750 onto the implied directory")
+    # The implied dir must get the default mode (0o777 & ~umask == 0o755),
+    # not the source's distinctive 0750 -- assert the exact mode, not merely
+    # "different from 0750".
+    assert_mode(TODIR / 'b', 0o755,
+                label='--no-implied-dirs uses the default mode, not source 0750')
     assert_same(TODIR / 'b' / 'c' / 'file', base / 'a' / 'b' / 'c' / 'file',
                 label='--no-implied-dirs deep file')
 

diff --git a/testsuite/safe-links_test.py b/testsuite/safe-links_test.py
index 8e1aad2f96c30a52b2d6ff7980f719779d9a18b3..9a74095cdd80045699eaaf968f151d01f39074bf 100644 (file)
--- a/testsuite/safe-links_test.py
+++ b/testsuite/safe-links_test.py
@@ -10,9 +10,12 @@ import os
 from rsyncfns import TMPDIR, is_a_link, run_rsync, test_fail
 
 
-def assert_symlink(path):
+def assert_symlink(path, target):
     if not is_a_link(path):
         test_fail(f"File {path} is not a symlink")
+    actual = os.readlink(path)
+    if actual != target:
+        test_fail(f"symlink {path} target is {actual!r}, expected {target!r}")
 
 
 def assert_notexist(path):
@@ -42,7 +45,7 @@ os.symlink("a/a/a/../../../unsafe2", "from/safe/links/unsafe2")
 print("rsync with relative path and --safe-links")
 run_rsync('-avv', '--safe-links', 'from/safe/', 'to')
 
-assert_symlink("to/links/file1")
-assert_symlink("to/links/file2")
+assert_symlink("to/links/file1", "../files/file1")
+assert_symlink("to/links/file2", "../files/file2")
 assert_notexist("to/links/unsafefile")
 assert_notexist("to/links/unsafe2")

diff --git a/testsuite/unsafe-links_test.py b/testsuite/unsafe-links_test.py
index 3f60528155a8bb150ae827cffd6e16a51022e38d..311982f88ee5229167e71e4398c46feae3bb588b 100644 (file)
--- a/testsuite/unsafe-links_test.py
+++ b/testsuite/unsafe-links_test.py
@@ -12,9 +12,12 @@ import os
 from rsyncfns import TMPDIR, is_a_link, rmtree, run_rsync, test_fail
 
 
-def assert_symlink(path):
+def assert_symlink(path, target):
     if not is_a_link(path):
         test_fail(f"File {path} is not a symlink")
+    actual = os.readlink(path)
+    if actual != target:
+        test_fail(f"symlink {path} target is {actual!r}, expected {target!r}")
 
 
 def assert_regular(path):
@@ -41,9 +44,9 @@ os.symlink("../../unsafe/unsafefile", "from/safe/links/unsafefile")
 
 print("rsync with relative path and just -a")
 run_rsync('-avv', 'from/safe/', 'to')
-assert_symlink("to/links/file1")
-assert_symlink("to/links/file2")
-assert_symlink("to/links/unsafefile")
+assert_symlink("to/links/file1", "../files/file1")
+assert_symlink("to/links/file2", "../files/file2")
+assert_symlink("to/links/unsafefile", "../../unsafe/unsafefile")
 
 print("rsync with relative path and -a --copy-links")
 run_rsync('-avv', '--copy-links', 'from/safe/', 'to')
@@ -53,8 +56,8 @@ assert_regular("to/links/unsafefile")
 
 print("rsync with relative path and --copy-unsafe-links")
 run_rsync('-avv', '--copy-unsafe-links', 'from/safe/', 'to')
-assert_symlink("to/links/file1")
-assert_symlink("to/links/file2")
+assert_symlink("to/links/file1", "../files/file1")
+assert_symlink("to/links/file2", "../files/file2")
 assert_regular("to/links/unsafefile")
 
 rmtree("to")
@@ -67,13 +70,13 @@ try:
     run_rsync('-avv', '--copy-unsafe-links', 'safe/', '../to')
 finally:
     os.chdir(saved)
-assert_symlink("to/links/file1")
-assert_symlink("to/links/file2")
+assert_symlink("to/links/file1", "../files/file1")
+assert_symlink("to/links/file2", "../files/file2")
 assert_regular("to/links/unsafefile")
 
 rmtree("to")
 print("rsync with absolute path")
 run_rsync('-avv', '--copy-unsafe-links', f'{os.getcwd()}/from/safe/', 'to')
-assert_symlink("to/links/file1")
-assert_symlink("to/links/file2")
+assert_symlink("to/links/file1", "../files/file1")
+assert_symlink("to/links/file2", "../files/file2")
 assert_regular("to/links/unsafefile")