rewrite test suite in python#903
Merged
Merged
Conversation
Replace the entire shell-based testsuite with Python. runtests.py
already drove the suite (it had replaced runtests.sh earlier); this
converts all 60 test scripts from *.test shell to *_test.py and adds
testsuite/rsyncfns.py as the shared helper module -- the Python
counterpart of the now-removed rsync.fns.
runtests.py:
* Discovers and runs both *.test and *_test.py; dispatches the
Python tests via the same python3 that runs the harness.
* Extends PYTHONPATH so tests can `import rsyncfns`.
testsuite/rsyncfns.py provides everything the ports need:
* environment wiring (scratchdir / srcdir / TOOLDIR / RSYNC /
TLS_ARGS, and HOME pointed at the per-test scratch dir);
* result reporting -- test_fail / test_skipped / test_xfail mapping
to the 0 / 1 / 77 / 78 exit-code convention;
* the transfer-and-verify helpers checkit, checkdiff, verify_dirs,
rsync_ls_lR, check_perms and the v_filt output filter;
* fixture builders hands_setup, build_symlinks, build_rsyncd_conf,
make_data_file, cp_p / cp_touch, makepath / rmtree.
All 60 tests are converted, including the four split-variant tests
that share one source via a Makefile-built symlink (chown/chown-fake,
devices/devices-fake, xattrs/xattrs-hlink, exclude/exclude-lsh);
Makefile.in's CHECK_SYMLINKS now points at the *_test.py names.
The dead rsync.fns shell library is removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rsyncfns.claim_ports(*ports) takes exclusive POSIX byte-range locks on /tmp/rsync_test.lck (offset = port number) so any number of test processes can run concurrently without colliding on a TCP port: a test asking for a port already held blocks until the holder exits. The kernel drops the locks automatically when the holding process dies, so a crashed test releases its ports with no manual cleanup. Ports are claimed in sorted order so two callers requesting the same set in different orders can't deadlock. The lock file is forced to mode 0o666 after creation (the umask would otherwise trim it and lock out a second user on a shared CI runner; EPERM when we're not the owner is fine). proxy-response-line-too-long is the first user: it switches from an ephemeral port to a claimed fixed port (12873). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Daemon-mode tests default to the stdio-pipe transport (RSYNC_CONNECT_PROG), which opens no listening socket -- so `make check` never exposes a network service. Real TCP is opt-in via `runtests.py --use-tcp`, with the daemon bound to loopback (127.0.0.1) on a claim_ports()-reserved port; CI runs the suite both ways. start_test_daemon() is the single seam every daemon test uses: the secure pipe by default, a real rsyncd on a claimed loopback port under --use-tcp. Tests with no pipe equivalent (the fake-proxy listener and the reverse-DNS hostname-ACL daemon test) are gated behind require_tcp(). `make check` also now runs the suite in parallel by default (CHECK_J=8); the claim_ports() byte-range locks make that safe across concurrent runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
socketpair_tcp() fakes a connected socket pair via a loopback TCP self-connect (socket -> bind 127.0.0.1:0 -> listen -> connect -> accept), used by sock_exec() for RSYNC_CONNECT_PROG. Its comment has long promised that "nobody else can attach to the socket, or if they do that this function fails", but nothing actually verified it: the code accept()ed whatever connection arrived first without checking it was the one our own connect() made. Between listen() and accept() the ephemeral loopback port is connectable by any local user. With backlog 1 a same-host attacker who races a connection in before our connect() lands could have their socket returned by accept(), handing them one end of the rsync protocol stream. The exposure is small (loopback only, random ephemeral port, sub-millisecond window, local users only), but the promised guarantee was simply not enforced. Enforce it: after the connection is established, require that the peer address of the accepted end (fd[0]) equals the local address of our connecting end (fd[1]), and that both are 127.0.0.1. A hijacked connection has a different source port and is rejected (errno EPERM, fail closed). The legitimate self-connect always matches, so there is no behaviour change for the normal path. Verified: rebuilds clean with -Wall -W; the full testsuite still passes in both transports (pipe `make check` 57/3, `runtests.py --use-tcp` 59/1) -- the pipe transport exercises this code path on every daemon test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chmod-option: pin umask to the suite-wide 022 baseline (mirroring the
old rsync.fns) so rsync's --chmod `D+w` is computed and applied under
the same umask -- fixes failures under a different ambient umask (077).
* daemon module-list test: assert the `list = no` module does NOT leak
into the listing (the substring check alone missed regressions).
* claim_ports() lock file: open with O_NOFOLLOW and only fchmod a file we
O_EXCL-created, rejecting a symlink OR hard link planted at the
well-known /tmp path -- which, with the TCP tests running under sudo in
CI, could otherwise chmod an arbitrary root-owned target. Require a
pristine (regular, nlink==1) file.
* CI: extend the Linux/Cygwin expected-skip lists for the gated tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Python rewrite had gated the xattr / fake-super tests (xattrs,
xattrs-hlink, chown-fake, devices-fake) to Linux because it used the
Linux-only os.*xattr. Restore them on macOS, FreeBSD, Cygwin and Solaris
via a per-OS xattr surface in rsyncfns.py (xattrs_supported / xattr_set /
xattr_dump):
* Linux -- os.*xattr
* macOS -- xattr
* FreeBSD -- setextattr / lsextattr / getextattr
* Cygwin -- getfattr / setfattr (from the `attr` package; CPython on
Cygwin has no os.*xattr)
* Solaris -- runat(1), with the script on stdin and the attr name/value
passed via the environment (the runat -c form mangles args)
Test attribute names are logical; the "user." namespace prefix is added
only on the Linux-style platforms (Linux, Cygwin). RSYNC_PREFIX/RUSR vary
per OS (macOS and Solaris use rsync.nonuser to avoid rsync's reserved
rsync.* space). The macOS and Cygwin workflows no longer skip these tests;
the FreeBSD/Solaris jobs use IGNORE skip-checking so need no change.
Verified on real Linux, macOS, FreeBSD, Cygwin and Solaris hosts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This rewrites all existing tests to be python based, which has better error handling and allows for better TCP port handling