Skip to content

Add smoke tests for binaries and platform lookup helpers#1697

Merged
oferchen merged 1 commit into
masterfrom
increase-test-coverage-to-96%
Oct 30, 2025
Merged

Add smoke tests for binaries and platform lookup helpers#1697
oferchen merged 1 commit into
masterfrom
increase-test-coverage-to-96%

Conversation

@oferchen
Copy link
Copy Markdown
Owner

Summary

  • refactor the oc-rsync and oc-rsyncd binaries to expose a reusable run_with helper for tests
  • add smoke tests that exercise the --version flows for both binaries
  • cover the user and group lookup helpers on Unix and non-Unix platforms

Testing

  • cargo fmt --all
  • cargo test -p oc-rsync-bin
  • cargo test -p oc-rsyncd-bin
  • cargo test -p rsync-cli --lib platform::tests::support_flags_match_platform
  • cargo test -p rsync-cli --lib platform::tests::user_lookup_round_trip_matches_current_identity
  • cargo test -p rsync-cli --lib platform::tests::group_lookup_round_trip_matches_current_identity

https://chatgpt.com/codex/tasks/task_e_69037ce06ca08323a648703efb321e64

@oferchen oferchen merged commit 5c767f0 into master Oct 30, 2025
@oferchen oferchen deleted the increase-test-coverage-to-96% branch October 30, 2025 15:09
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +110 to +114
if cfg!(unix) {
let uid = get_current_uid();
let user = get_user_by_uid(uid).expect("current user should exist");
let name = user.name().to_string_lossy().into_owned();
assert_eq!(lookup_user_by_name(&name), Some(uid));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard Unix-only test helpers

The new user lookup test calls get_current_uid and get_user_by_uid unconditionally while the users imports are compiled only when cfg(unix) holds. On non-Unix targets the import is removed, these names are undefined, and cargo test -p rsync-cli fails to build even though the code path is gated at runtime by if cfg!(unix). The test (and the analogous group test below) should be #[cfg(unix)] or otherwise gate the Unix-only calls so Windows builds continue to compile.

Useful? React with 👍 / 👎.

oferchen added a commit that referenced this pull request May 1, 2026
)

Document that daemon-side filter rules (filter/exclude/include
directives in oc-rsyncd.conf) are never transmitted on the wire during
a pull, in both upstream rsync 3.4.1 and oc-rsync. Cite upstream
clientserver.c:874-893 (rsync_module loads daemon_filter_list) and
exclude.c:1589/1645/1658/1661 (send_rules walks &filter_list, never
&daemon_filter_list). Cross-reference oc-rsync sources at
crates/protocol/src/filters/wire.rs, crates/transfer/src/receiver/transfer.rs,
and crates/daemon/src/daemon/sections/module_access/helpers.rs. Provide
an operator-facing tcpdump reproduction recipe with decoded bytes for
the 4-byte LE filter-list zero terminator. Note an unrelated comment
discrepancy in tools/ci/known_failures.conf (rsync 3.0.9 advertises
PROTOCOL_VERSION 30, not 28) for future cleanup.
oferchen added a commit that referenced this pull request May 1, 2026
…1697) (#3516)

Adds wire-byte audit for server-side filter rules (filter / exclude /
include / exclude from / include from in oc-rsyncd.conf module sections)
during a pull. Headline finding: no active divergence today; daemon
rules are applied process-locally and never serialised. Documents the
load-bearing invariants and divergence sentinels.

Companion to docs/audits/tcpdump-daemon-proto28-29.md (PR #3513) and
docs/daemon/filter-precedence.md (PR #1888).
oferchen added a commit that referenced this pull request May 5, 2026
)

Document that daemon-side filter rules (filter/exclude/include
directives in oc-rsyncd.conf) are never transmitted on the wire during
a pull, in both upstream rsync 3.4.1 and oc-rsync. Cite upstream
clientserver.c:874-893 (rsync_module loads daemon_filter_list) and
exclude.c:1589/1645/1658/1661 (send_rules walks &filter_list, never
&daemon_filter_list). Cross-reference oc-rsync sources at
crates/protocol/src/filters/wire.rs, crates/transfer/src/receiver/transfer.rs,
and crates/daemon/src/daemon/sections/module_access/helpers.rs. Provide
an operator-facing tcpdump reproduction recipe with decoded bytes for
the 4-byte LE filter-list zero terminator. Note an unrelated comment
discrepancy in tools/ci/known_failures.conf (rsync 3.0.9 advertises
PROTOCOL_VERSION 30, not 28) for future cleanup.
oferchen added a commit that referenced this pull request May 5, 2026
…1697) (#3516)

Adds wire-byte audit for server-side filter rules (filter / exclude /
include / exclude from / include from in oc-rsyncd.conf module sections)
during a pull. Headline finding: no active divergence today; daemon
rules are applied process-locally and never serialised. Documents the
load-bearing invariants and divergence sentinels.

Companion to docs/audits/tcpdump-daemon-proto28-29.md (PR #3513) and
docs/daemon/filter-precedence.md (PR #1888).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant