test(dry-run): strengthen no-mutation coverage and refresh docs

Extend _snapshot_tree to record mode, inode, xattrs, directories and
special nodes, and add coverage proving a server-contacting --dry-run
leaves the destination structurally identical for --delay-updates,
--backup, symlinks, hardlinks, FIFOs, and daemon modules (including a
read-only module).  Add a regression test for the --read-batch --dry-run
refusal and for a missing/non-directory receive root failing a dry-run
exactly like a real run.

Fix stale version comments (2.20.0/633 -> 2.21.0/637) and RSYNC_COMPAT's
current --protocol value, and add a unit assertion that
--server-port/--port (and --server-host) set the dry-run routing bit.
This commit is contained in:
2026-09-13 12:57:37 +02:00
parent 07f7555c1d
commit 6269ae54e5
5 changed files with 258 additions and 18 deletions
+30
View File
@@ -43,6 +43,9 @@ from common import (
_find_free_port,
_wait_for_port,
)
# The dry-run no-mutation contract is asserted with the same structural snapshot
# (mode/inode/mtime/xattr/content) the feature suite uses.
from test_features import _snapshot_tree
SOURCE_DIR = os.path.join(TEST_DATA_DIR, "daemon_source")
MODULE_ROOT = os.path.join(TEST_DATA_DIR, "daemon_modules")
@@ -355,6 +358,33 @@ class TestDaemonRejection:
assert result.returncode != 0
assert self._tree_files() == before, "read-only rejection wrote under the module root"
@pytest.mark.ci
def test_read_only_module_allows_dry_run(self, daemon):
"""A server-contacting --dry-run IS a read-only wire operation, so a
`read only = yes` module is the safest dry-run target and must accept it
while writing nothing."""
result, _ = run_client(SOURCE_DIR, "127.0.0.1::readonly", flags=["--dry-run"],
port=daemon.port)
assert result.returncode == 0, (result.stderr or result.stdout)[:300]
assert "Dry run:" in result.stdout, result.stdout[:200]
assert _tree_file_count(READONLY_MODULE) == 0, "read-only dry-run wrote a file"
@pytest.mark.ci
def test_module_dry_run_mutates_nothing(self, daemon):
"""A daemon-module dry-run reports would-transfer entries but leaves the
module tree structurally identical (mode/inode/mtime/xattr/content)."""
result = _push("127.0.0.1::files", daemon.port)
assert result.returncode == 0, result.stderr or result.stdout
before = _snapshot_tree(FILES_MODULE)
# --ignore-times forces every regular file to be reported as
# would-transfer, so the dry-run exercises the receiver decision rather
# than an all-skip shortcut -- while still mutating nothing.
result, _ = run_client(SOURCE_DIR, "127.0.0.1::files",
flags=["--dry-run", "--ignore-times"], port=daemon.port)
assert result.returncode == 0, (result.stderr or result.stdout)[:300]
assert "Dry run:" in result.stdout, result.stdout[:200]
assert _snapshot_tree(FILES_MODULE) == before, "daemon dry-run mutated the module root"
def test_unknown_module_rejected(self, daemon):
result = _push("127.0.0.1::no-such-module", daemon.port)
assert result.returncode != 0
+196 -14
View File
@@ -370,26 +370,55 @@ class TestDryRun:
assert not mismatches, f"Mismatch: {mismatches}"
def _snapshot_tree(root):
"""Return {relpath: (size, mtime_ns, content_bytes)} for a directory tree.
def _snapshot_xattrs(path):
"""Return a stable, comparable tuple of (name, value) xattr pairs.
Used to prove a dry-run left the destination byte-for-byte and
timestamp-for-timestamp unchanged. Returns an empty dict for a missing
root so "nothing was created" is also observable."""
Returns None when the platform/filesystem does not expose xattrs so both
snapshots agree on "unavailable" instead of one being treated as changed."""
try:
names = os.listxattr(path, follow_symlinks=False)
except (AttributeError, OSError):
return None
if not names:
return ()
pairs = []
for name in sorted(names):
try:
value = os.getxattr(path, name, follow_symlinks=False)
except OSError:
value = None
pairs.append((name, value))
return tuple(pairs)
def _snapshot_tree(root):
"""Return a structural snapshot of a directory tree.
Every entry (including directories) is recorded as
(inode, mtime_ns, mode, xattrs, kind-specific payload) so a dry-run that
touched a mode, inode, mtime, xattr, or content is observable. Regular
files carry their size+bytes, symlinks their target, and special entries
(FIFO/socket/device) their size only -- opening a special file could block.
Returns an empty dict for a missing root so "nothing was created" is also
observable."""
snapshot = {}
if not os.path.exists(root):
return snapshot
for dirpath, _dirnames, filenames in os.walk(root):
for name in filenames:
for dirpath, dirnames, filenames in os.walk(root):
for name in list(dirnames) + filenames:
path = os.path.join(dirpath, name)
rel = os.path.relpath(path, root)
st = os.lstat(path)
entry = [st.st_ino, st.st_mtime_ns, stat.S_IMODE(st.st_mode), _snapshot_xattrs(path)]
if stat.S_ISLNK(st.st_mode):
snapshot[rel] = ("symlink", os.readlink(path), st.st_mtime_ns)
continue
with open(path, "rb") as fh:
data = fh.read()
snapshot[rel] = (st.st_size, st.st_mtime_ns, data)
entry.append(("symlink", os.readlink(path)))
elif stat.S_ISREG(st.st_mode):
with open(path, "rb") as fh:
data = fh.read()
entry += [st.st_size, data]
else:
entry.append(st.st_size)
snapshot[rel] = tuple(entry)
return snapshot
@@ -461,6 +490,10 @@ class TestRemoteDryRun:
@pytest.mark.ci
def test_remote_dry_run_mkpath_does_not_create_root(self, shared_server):
"""A wire dry_run cannot make --mkpath create anything, and it cannot
relax the precondition either: a nonexistent root is rejected (a real
run without the created root is impossible in dry-run) while nothing is
created."""
source = os.path.join(TEST_DATA_DIR, "remote_dry_mk_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_mk_dst")
self._seed(source)
@@ -469,8 +502,7 @@ class TestRemoteDryRun:
result, _ = run_client(source, dest, flags=["--dry-run", "--mkpath"],
port=shared_server.port)
assert result.returncode == 0, f"exit {result.returncode}: {result.stderr[:300]}"
assert "changed.txt" in result.stdout
assert result.returncode != 0, "dry-run --mkpath accepted a nonexistent receive root"
assert not os.path.exists(dest), "dry-run --mkpath created the destination root"
@pytest.mark.ci
@@ -534,6 +566,156 @@ class TestRemoteDryRun:
with open(os.path.join(received, "changed.txt"), "rb") as f:
assert f.read() == b"updated payload for the real transfer\n"
@pytest.mark.ci
def test_remote_dry_run_delay_updates_mutates_nothing(self, shared_server):
"""--delay-updates stages under the receive root; a dry-run must neither
create that staging tree nor publish anything (mode/inode/mtime intact)."""
source = os.path.join(TEST_DATA_DIR, "remote_dry_delay_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_delay_dst")
self._seed(source)
clean_dir(dest)
result, _ = run_client(source, dest, flags=["--delay-updates"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:200]
with open(os.path.join(source, "changed.txt"), "wb") as f:
f.write(b"changed for delay-updates dry-run\n")
before = _snapshot_tree(dest)
result, _ = run_client(source, dest, flags=["--dry-run", "--delay-updates"],
port=shared_server.port)
assert result.returncode == 0, result.stderr[:300]
assert "changed.txt" in result.stdout, result.stdout
assert _snapshot_tree(dest) == before, "delay-updates dry-run mutated the destination"
@pytest.mark.ci
def test_remote_dry_run_backup_mutates_nothing(self, shared_server):
"""--backup would rename the old file aside; a dry-run must not."""
source = os.path.join(TEST_DATA_DIR, "remote_dry_backup_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_backup_dst")
self._seed(source)
clean_dir(dest)
result, _ = run_client(source, dest, port=shared_server.port)
assert result.returncode == 0, result.stderr[:200]
with open(os.path.join(source, "changed.txt"), "wb") as f:
f.write(b"changed for backup dry-run\n")
before = _snapshot_tree(dest)
result, _ = run_client(source, dest, flags=["--dry-run", "--backup"],
port=shared_server.port)
assert result.returncode == 0, result.stderr[:300]
assert "changed.txt" in result.stdout, result.stdout
assert _snapshot_tree(dest) == before, "--backup dry-run mutated the destination"
@pytest.mark.ci
def test_remote_dry_run_symlink_mutates_nothing(self, shared_server):
source = os.path.join(TEST_DATA_DIR, "remote_dry_symlink_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_symlink_dst")
self._seed(source)
os.symlink("changed.txt", os.path.join(source, "link"))
clean_dir(dest)
result, _ = run_client(source, dest, flags=["-a"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:200]
received = get_dest_received_dir(dest, source)
assert os.path.islink(os.path.join(received, "link"))
# Re-point the source link so the entry is genuinely stale, then prove a
# dry-run leaves the destination link target, inode, and mtime untouched.
os.unlink(os.path.join(source, "link"))
os.symlink("keep.txt", os.path.join(source, "link"))
before = _snapshot_tree(dest)
result, _ = run_client(source, dest, flags=["-a", "--dry-run"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:300]
assert _snapshot_tree(dest) == before, "symlink dry-run mutated the destination"
assert os.readlink(os.path.join(received, "link")) == "changed.txt"
@pytest.mark.ci
def test_remote_dry_run_hardlink_mutates_nothing(self, shared_server):
source = os.path.join(TEST_DATA_DIR, "remote_dry_hardlink_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_hardlink_dst")
clean_dir(source)
clean_dir(dest)
with open(os.path.join(source, "h1.txt"), "wb") as f:
f.write(b"hardlinked payload\n")
os.link(os.path.join(source, "h1.txt"), os.path.join(source, "h2.txt"))
result, _ = run_client(source, dest, flags=["-H"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:200]
received = get_dest_received_dir(dest, source)
assert os.stat(os.path.join(received, "h1.txt")).st_ino == \
os.stat(os.path.join(received, "h2.txt")).st_ino
# Change the shared inode; both names are now stale in the destination.
with open(os.path.join(source, "h1.txt"), "wb") as f:
f.write(b"changed hardlinked payload\n")
before = _snapshot_tree(dest)
result, _ = run_client(source, dest, flags=["-H", "--dry-run"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:300]
assert _snapshot_tree(dest) == before, "hardlink dry-run mutated the destination"
@pytest.mark.ci
def test_remote_dry_run_fifo_special_mutates_nothing(self, shared_server):
source = os.path.join(TEST_DATA_DIR, "remote_dry_fifo_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_fifo_dst")
clean_dir(source)
clean_dir(dest)
with open(os.path.join(source, "plain.txt"), "wb") as f:
f.write(b"plain\n")
os.mkfifo(os.path.join(source, "existing.fifo"))
result, _ = run_client(source, dest, flags=["--specials"], port=shared_server.port)
assert result.returncode == 0, result.stderr[:200]
received = get_dest_received_dir(dest, source)
assert stat.S_ISFIFO(os.lstat(os.path.join(received, "existing.fifo")).st_mode)
os.mkfifo(os.path.join(source, "new.fifo"))
before = _snapshot_tree(dest)
result, _ = run_client(source, dest, flags=["--specials", "--dry-run"],
port=shared_server.port)
assert result.returncode == 0, result.stderr[:300]
assert not os.path.exists(os.path.join(received, "new.fifo")), \
"dry-run created a FIFO on the receiver"
assert _snapshot_tree(dest) == before, "special-node dry-run mutated the destination"
@pytest.mark.ci
def test_read_batch_with_dry_run_is_refused(self, shared_server):
"""A dry-run of a local batch apply is meaningless (and must not become a
mutation escape hatch): the CLI rejects the combination up front."""
source = os.path.join(TEST_DATA_DIR, "remote_dry_batch_src")
dest = os.path.join(TEST_DATA_DIR, "remote_dry_batch_dst")
self._seed(source)
clean_dir(dest)
result, _ = run_client(source, dest, flags=["--read-batch=/nonexistent.batch", "--dry-run"],
port=shared_server.port)
assert result.returncode != 0, "read-batch + dry-run was accepted"
combined = (result.stderr or "") + (result.stdout or "")
assert "cannot be combined" in combined or "--dry-run" in combined, combined[:300]
@pytest.mark.ci
def test_remote_dry_run_bad_root_fails_like_real_run(self, shared_server):
"""A wire dry_run must not relax the destination-root precondition: a
missing or non-directory root that fails a real run fails a dry-run too,
and the dry-run must not create/replace anything."""
source = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_src")
self._seed(source)
missing = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_missing")
shutil.rmtree(missing, ignore_errors=True)
real, _ = run_client(source, missing, port=shared_server.port)
assert real.returncode != 0, "real run accepted a missing receive root"
assert not os.path.exists(missing), "real run created the missing root"
dry, _ = run_client(source, missing, flags=["--dry-run"], port=shared_server.port)
assert dry.returncode != 0, "dry-run accepted a missing receive root a real run rejects"
assert not os.path.exists(missing), "dry-run created the missing receive root"
fileroot = os.path.join(TEST_DATA_DIR, "remote_dry_badroot_file")
shutil.rmtree(fileroot, ignore_errors=True)
with open(fileroot, "wb") as f:
f.write(b"i am a regular file, not a directory\n")
real, _ = run_client(source, fileroot, port=shared_server.port)
assert real.returncode != 0, "real run accepted a regular-file receive root"
dry, _ = run_client(source, fileroot, flags=["--dry-run"], port=shared_server.port)
assert dry.returncode != 0, "dry-run accepted a regular-file receive root a real run rejects"
with open(fileroot, "rb") as f:
assert f.read() == b"i am a regular file, not a directory\n", \
"dry-run clobbered a regular-file receive root"
class TestRemoveSourceFiles:
def test_removes_only_transferred_regular_files(self, shared_server):
+28
View File
@@ -590,6 +590,9 @@ static void test_parse_args_port_alias() {
int positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 5, argv_space, positional_args, &positional_count), 0);
EXPECT_EQ_INT(cfg->server_port, 9000);
/* The default port is 8080; the explicit bit is what lets --dry-run tell an
explicit remote target from the default and route to the server. */
EXPECT_TRUE(cfg->server_port_set);
config_delete(cfg);
cfg = config_create();
@@ -597,6 +600,7 @@ static void test_parse_args_port_alias() {
positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 4, argv_inline, positional_args, &positional_count), 0);
EXPECT_EQ_INT(cfg->server_port, 9001);
EXPECT_TRUE(cfg->server_port_set);
config_delete(cfg);
cfg = config_create();
@@ -604,6 +608,29 @@ static void test_parse_args_port_alias() {
positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 4, argv_long, positional_args, &positional_count), 0);
EXPECT_EQ_INT(cfg->server_port, 9002);
EXPECT_TRUE(cfg->server_port_set);
config_delete(cfg);
}
/* An explicit --server-host must set its own routing bit (the field itself
* defaults to 127.0.0.1, so a value check cannot distinguish an explicit host
* from the default); --dry-run uses it to route to the server. */
static void test_parse_args_server_host_sets_routing_bit() {
Config* cfg = config_create();
int positional_args[2];
int positional_count = 0;
EXPECT_FALSE(cfg->server_host_set);
char* argv_space[] = {"fastsync", "--server-host", "example.test", "/src", "/dst"};
EXPECT_EQ_INT(parse_args(cfg, 5, argv_space, positional_args, &positional_count), 0);
EXPECT_EQ_STR(cfg->server_host, "example.test");
EXPECT_TRUE(cfg->server_host_set);
config_delete(cfg);
cfg = config_create();
char* argv_inline[] = {"fastsync", "--server-host=example.test", "/src", "/dst"};
positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 4, argv_inline, positional_args, &positional_count), 0);
EXPECT_TRUE(cfg->server_host_set);
config_delete(cfg);
}
@@ -3301,6 +3328,7 @@ void test_client_cli() {
test_parse_args_non_numeric_port();
test_parse_args_invalid_server_port();
test_parse_args_port_alias();
test_parse_args_server_host_sets_routing_bit();
test_parse_args_threads();
test_client_abort_flag();
test_parse_args_invalid_compression_level();
+2 -2
View File
@@ -2399,7 +2399,7 @@ static void golden_config_populate(Config* c) {
c->modify_window = 3;
c->compress_choice = str_dup("zstd");
/* "u=rwx,go=rx" is the same 11 bytes as the original "u=rwX,go=rX" (so the
* frame stays 633 bytes) but X is not in FastSync's chmod grammar, and the
* frame stays 637 bytes) but X is not in FastSync's chmod grammar, and the
* receive-side golden validates the frame. */
c->chmod_spec = str_dup("u=rwx,go=rx");
c->skip_compress_set = true;
@@ -2531,7 +2531,7 @@ static unsigned long long capture_wire_hash(const Config* cfg, size_t* out_len)
return h;
}
/* Byte-for-byte wire compatibility guard (protocol 2.20.0). The expected hash
/* Byte-for-byte wire compatibility guard (protocol 2.21.0). The expected hash
* pins the pre-X-macro byte stream; the refactor MUST NOT change it. */
static void test_config_wire_golden() {
if (is_running_under_valgrind())