greatly expand testsuite coverage and add coverage analysis with gcov#913
Merged
Conversation
Add helpers for the option-coverage expansion (the path-handling restructure
changes parent-component resolution, so options must be exercised at depth and
across directory boundaries):
* make_tree() builds a tree with a regular file at every level so a property
can be checked at the tree root and >=3 levels deep;
* walk_files()/walk_dirs() iterate entries for per-level assertions;
* assert_same/assert_mode/assert_mtime_close/assert_is_symlink/
assert_hardlinked/assert_not_hardlinked/assert_exists/assert_not_exists
assert the concrete property an option controls (not just dest == src);
* write_daemon_conf() writes an arbitrary rsyncd.conf (globals + modules)
for daemon-parameter tests, beyond build_rsyncd_conf's fixed four modules;
* forced_protocol() lets protocol-sensitive tests gate sub-cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fill the highest-restructure-risk gap: options that do two-directory / rename /
outside-tree work, asserted at >=3 levels deep with the aux tree kept outside
the main tree, and asserting the option's specific property rather than just
tree equality (which the ported tests already cover).
alt-dest-deep --link-dest hardlinks unchanged files (same inode), --copy-dest
copies (never links), --compare-dest omits unchanged files;
ref tree outside both src and dest.
temp-dir cross-dir temp->final rename at depth; temp dir left clean; a
missing --temp-dir fails (so the option is proven consulted).
partial --partial keeps the partial in the dest file; relative
--partial-dir stages per-directory at depth (pre-seed +
interrupt/resume); absolute --partial-dir writes the partial
outside the tree.
inplace --inplace keeps the destination inode across a delta update;
the default temp+rename path replaces it.
append --append completes truncated files tail-only; --append-verify
repairs a corrupted prefix (protocol >= 30).
backup-deep --suffix saves <name>S beside the new file; --backup-dir
relocates old files to a parallel deep tree outside the dest
and captures deletions under --delete.
All green on master and under --protocol=29/30.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover the structure and link options at >=3 levels and across directories,
asserting each option's specific effect:
links -l keeps a symlink, -L dereferences it, -k follows a
directory symlink -- all on a symlink several levels deep.
dirs -d copies the top layer (file + empty dir) without recursing.
prune-empty-dirs -m drops empty chains and chains emptied by an exclude,
keeps populated ones.
hardlinks-deep -H preserves a hard link whose names live in different
directories at depth; without -H they become separate inodes.
delete-deep --delete removes a deep extraneous file/subtree; the four
delete-timing variants agree; --max-delete caps deletions;
--existing / --ignore-existing select/skip correctly.
relative-implied -R mirrors an implied directory's mode at depth;
--no-implied-dirs does not (proto 30+).
Green on master and under --protocol=29/30 (the --no-implied-dirs sub-case is
gated to protocol >= 30, where multi-component sender paths are accepted).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Set each attribute distinctively on a file AND a directory at every level of a
>=3-deep tree and verify it per entry after transfer (metadata is applied as a
single-component op on an entry whose parent chain the resolver restructure
rewrites):
metadata-depth -p preserves exact file/dir modes; -t preserves file
mtimes; --chmod=D710,F600 rewrites them.
omit-times -O omits directory times (files still preserved); -J omits
symlink times.
sparse -S preserves a deep file's hole (allocated << size);
--no-sparse fills it.
xattrs-depth -X reproduces a user xattr on every entry (gated on xattr
support).
acls-depth -A reproduces a POSIX ACL on every entry (gated on ACL
support + setfacl/getfacl).
ownership-depth --groupmap and --chown=:GROUP remap the group of every
entry (non-root, to a secondary group); -o/--usermap gated
on root.
All green on master and under --protocol=29/30.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Assert exactly which entries are/aren't transferred, deep in the tree:
filter-depth --exclude/--include precedence on files at every level, and
a -F per-directory .rsync-filter loaded from a deep dir that
applies to that subtree only (not above it).
cvs-exclude -C built-in cruft patterns (*.o, *~) at every level plus a
deep per-directory .cvsignore scoped to its subtree.
size-filter --max-size / --min-size select the right files all the way
down.
files-from-depth --files-from selects only the listed deep paths (implied
parents created); --from0 NUL-delimited; --exclude-from /
--include-from filter at depth.
(--existing / --ignore-existing are covered in delete-deep_test.py.)
Green on master and under --protocol=29/30.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drive a loopback daemon (secure stdio-pipe transport by default, also green
under --use-tcp) via the new write_daemon_conf helper and assert the behaviour
of the security-relevant rsyncd.conf parameters, transferring >=3-deep trees:
daemon-access path / read only / write only / list, incl. a deep sub-path
pull and that a list=no module is hidden yet usable by name.
daemon-filter daemon exclude hides matching files everywhere; incoming /
outgoing chmod rewrite modes of every transferred file.
daemon-auth auth users + secrets file accept the right password, reject a
wrong one and an unauthenticated request; strict modes rejects
a world-readable secrets file.
daemon-exec pre-/post-xfer exec run with RSYNC_MODULE_NAME /
RSYNC_EXIT_STATUS; a failing pre-xfer exec aborts the transfer
(marker files polled for, since post-xfer exec runs after the
client disconnects under TCP).
daemon-munge munge symlinks stores incoming links with the /rsyncd-munged/
prefix and strips it on the way out.
daemon-refuse refuse options: a named option, a wildcard, and the '* !a !v'
allow-list idiom.
Green on master under pipe and --use-tcp transports and under --protocol=29.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Breadth pass for options not yet exercised:
output-options output shape of --version/--help/-i/-n/--stats/
--out-format/--list-only/-q/--progress/-h/-8 (these control
output, not path handling, so they're checked for shape).
compare -c and -I catch a stealth change (same size+mtime, new
content) deep in the tree; --size-only skips a same-size
change; --modify-window absorbs a 1s mtime difference.
compress-options --compress-choice for every advertised compressor,
--compress-level, --skip-compress, --checksum-choice for
every advertised checksum, and --checksum-seed -- each a
clean byte-identical transfer at depth.
Green on master and under --protocol=29/30.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oject#715 test Add resolve_beneath_supported() to rsyncfns: it functionally probes whether the rsync binary can follow an in-tree directory symlink under its secure resolver (an initial transfer plus a delta update through a dir-symlink, the operation issue RsyncProject#715 is about). This tracks the actual binary instead of a platform name. Use it in symlink-dirlink-basis_test.py in place of the SunOS/OpenBSD/NetBSD/ Cygwin name check: it skips on those platforms too, and additionally on Linux < 5.6, a seccomp-blocked openat2, and the new --disable-openat2 build, where the portable O_NOFOLLOW fallback rejects the in-tree symlink. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two test-coverage build knobs (both behaviour-neutral by default):
--enable-coverage appends '--coverage -fprofile-update=atomic -O0' and adds
a 'make coverage' target (whole suite, run serially, then
gcovr HTML with branch + decision coverage). rsync forks
and its children exit without running the gcov atexit
flush -- the generator via its SIGUSR1 handler
(_exit_cleanup) and the receiver via the SIGUSR2 handler
-- so under GCOV_COVERAGE we call __gcov_dump() at both, or
receiver.c/generator.c record no coverage at all.
--disable-openat2 gates the Linux openat2(RESOLVE_BENEATH) sites in syscall.c
on HAVE_OPENAT2 (defined by default), so disabling it forces
the portable per-component O_NOFOLLOW resolver to run as the
primary on ordinary Linux -- exercising and
coverage-counting that fallback tier without a pre-5.6
kernel. NOTE: coordinate with the parallel syscall.c
path-resolution restructure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
COVERAGE.md is the living checklist mapping every CLI option (~142) and daemon parameter (~54) to its test(s), with depth / cross-dir status and remaining gaps, so the path-resolution restructure can see exactly what is guarded. update_test.py closes two of the documented gaps: -u/--update (keep a newer destination, update an older one) and --force (replace a non-empty destination directory with a file), both at depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A delta (--no-whole-file) resume whose basis is an absolute --partial-dir
looped forever on exit code 23 ("failed verification -- update put into
partial-dir"), stranding the correct data in the partial-dir and never
populating the destination.
Cause: an absolute --partial-dir makes the basis path absolute, but the
receiver opened it with secure_relative_open(NULL, fnamecmp, ...), which by
design rejects an absolute relpath (EINVAL). The basis fd was then -1, so
receive_data() mapped no basis and (because the matched-block sum_update() is
guarded by "if (mapbuf)") computed the whole-file verification checksum over
the literal data only -> a spurious mismatch every run. (The data itself was
correct, since the in-place update leaves the matched basis bytes in place.)
Under a non-chroot daemon the in-place write went through the same call and
failed outright.
Fix: add secure_basis_open(), which treats an operator-trusted absolute basis
path as (trusted directory + confined leaf) -- the same way secure_relative_open
already trusts an absolute basedir while keeping O_NOFOLLOW on the leaf -- and
use it for both the basis read and the inplace-partial write. The strict
"reject absolute relpath" contract of secure_relative_open is left intact.
Defense-in-depth: receive_data() now treats a block-match token with no mapped
basis as a protocol inconsistency (it can only arise from a basis that the
generator opened but the receiver could not), failing cleanly instead of
silently dropping those bytes from the verify checksum or the output.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
partial_test.py sub-test 5 deterministically asserts a delta (--no-whole-file) resume from an absolute, outside-tree --partial-dir reproduces the source and consumes the basis -- the regression guard for the receiver fix. Sub-test 4 keeps asserting the cross-directory partial WRITE on interrupt. Drop the --whole-file workaround and the 'broken on master' notes in the docstring and COVERAGE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
coverage-tcp reuses the coverage recipe with --use-tcp (daemon tests over a real loopback rsyncd, which also runs the require_tcp-only tests) and a separate report directory, via COVERAGE_RUNFLAGS / COVERAGE_DIR. Verified end to end: pipe run reports 63.9% lines, the TCP run 64.5% (it exercises more code). Also drop gcovr's --branches flag: it is deprecated in gcovr 8 and branch + decision coverage still appear in --print-summary and the HTML without it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Builds with --enable-coverage and runs the suite under both transports (make coverage, then make coverage-tcp). gcovr's line/branch/decision totals are printed to the step log and also written to the GitHub step summary, so the coverage numbers are visible directly in the CI output; the HTML reports are uploaded as an artifact. make coverage exits with the suite's status, so a test regression fails the job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
acls-depth skips where ACLs/setfacl are unavailable (macOS, Cygwin) like the existing acls tests, and sparse skips on APFS (macOS), where a seek-written hole isn't allocated sparsely. Add them to the per-platform RSYNC_EXPECT_SKIPPED lists so the skip-set assertion stays accurate. 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.
in preparation for some restructuring work this expands the testsuite a lot and add html coverage reports